Skip to content

Conversation

euanh
Copy link
Collaborator

@euanh euanh commented May 1, 2025

Motivation

Some minor documentation bugs have been fixed on main. We would like to release these changes and point the Swift Package Index documentation links back at the most recent release (#113), but some minor changes for v1.1 are also in main. This pull request cherry-picks only the patch changes.

Modifications

Cherry picks:
#111
#112
#114
#116
#117
#120
#121

Result

release/1.0 will hold only patch-level fixes suitable for a 1.0.1 release.

Test Plan

All existing tests continue to pass.

euanh added 7 commits May 1, 2025 08:43
…ple#111)

Motivation
----------

The DoCC documentation was restructured substantially between 0.5.0 and
1.0.0, so the links in `README.md` are no longer correct. The examples
should also point to the new release.

Modifications
-------------

* Update the links to the Swift Package Index documentation
* Update example project dependencies 
* Stop asking SPI to build the `containertool` documentation target 
 
Result
------

Documentation links will be correct once Swift Package Index builds the
new release.
 
Test Plan
---------

All existing tests continue to pass.
…e#112)

Motivation
----------

By default, Swift Package Index shows the documentation for the latest
release. This still has the `containertool` documentation target in
`.spi.yml`, which is causing the left-hand navigation bar (the curation)
to show the topics for `containertool` even though the main content is
the Swift Container Plugin Documentation.

This should be resolved when the next release is tagged, but in the
meantime linking to the `main` branch documentation will cause users to
see the most up-to-date documentation with the correct navigation bar.

Modifications
-------------

Links to the SPI documentation in `README.md` point to the `main`
branch.

Result
------

Users should see the latest documentation, with the correct navigation
bar.

Test Plan
---------

No functional change.
All existing tests continue to pass.
…geReference arguments (apple#114)

Motivation
----------

`ContainerRegistry` methods follow the distribution API closely.
`containertool` defines some extensions which build more convenient
operations over these primitive functions. These extensions should
accept `ImageReference` arguments. Currently they take separate
repository and reference arguments and the caller needs to extract these
fields from the appropriate references. This provides an opportunity for
errors, such as trying to use the a reference with the wrong repository.

Modifications
-------------

`getImageManifest()`, `getImageConfiguration()` and
`putImageConfiguration()` now accept `ImageReference` arguments.

Result
------

The caller no longer has to extract the underlying fields, and the risk
of mixing them up is reduced.

Test Plan
---------

All tests continue to pass.
…lishing (apple#116)

Motivation
----------

Most of `containertool`'s work is handled in its `run()` method.
* It starts by processing arguments and assigning defaults;
* It then creates `RegistryClient` instance;
* Finally it assembles the image and uploads it.

All of this work takes place inline, in a single method called by
`SwiftArgumentParser`. This complicates maintenance and makes it
difficult to test the image assembly process without writing full
end-to-end tests.

Modifications
-------------

* Extract the image building process into a standalone function.
* `run()` is only responsible for processing arguments and creating the
clients needed by the build function.

Result
------

The image build process will be easier to test and maintain.
It will be easier to add more complex argument handling, such as
additional environment variables or a configuration file.

Test Plan
---------

Existing tests continue to pass.
Future pull requests can test the image build process in integration or
unit tests, instead of requiring full end-to-end tests.
…le#117)

Motivation
----------

Many `containertool` options have default values set by environment
variables.
* if the option flag is set on the command line, the command line value
is used
* otherwise, if the option flag is not set, the environment variable
value is used
* otherwise, a hard-coded default is used

This pattern is quite convenient, but there are some drawbacks. For
example:
* the default value declarations complicate the definition of new
arguments.
* default values appear in the `--help` output: for some options, such
as default base image, this is desirable, but for others, such as
credentials, it is not.

Some options do not fit this pattern well. For instance, `containertool`
tries to detect the architecture of the executable it is packaging so it
can try to choose a compatible base image.
* if the architecture flag is set on the command line, that value is
used
* otherwise, if the flag is not set, the environment variable value is
used
* otherwise, if the executable is an ELF file, the architecture is read
from the file
* otherwise, a hard-coded default is used

The path to the executable is an option which is parsed by the argument
parser. It is not available when the parser sets up default argument
values, so it must be handled after the arguments have been parsed,
overwriting whatever the parser had set. This means that option handling
for this flag is split into two different places, which invites bugs.

Future changes to `containertool`'s configuration, such as supporting a
configuration file on disk, add further complications.

Modifications
-------------

* All command line arguments with corresponding environment variables
are handled uniformly, with defaults applied at the start of the `run()`
function

Result
------

No functional change, but maintenance and future enhancement become
easier.

Test Plan
---------

All existing tests continue to pass.
Handling defaults in this way allows future PRs to add unit or
integration tests which exercise argument parsing.
Motivation
----------

On first visit to the repository or the documentation, it can be
difficult to understand what the plugin does and how it fits into
the development process.   A flow diagram showing how the code is
built, packaged, uploaded and deployed is much easier to read at a
glance than a screen of example command output.   The diagram can
also provide a structure which can be referred to in the text.

Modifications
-------------

* Add a workflow diagram image
* Rewrite the overview section of README.md and the DocC documentation
entry point to include the image and expand on what happens in each
step.

Result
------

New visitors to the repository or the documentation will see a
diagram which summarises what the plugin does and where it fits in
the Swift Package Manager workflow.

Test Plan
---------

All existing tests continue to pass.
Manually tested all new URLs.
Motivation
----------

The help text for `--username` and `--password` states that these
options override `.netrc`. In fact they are used as defaults if no
suitable entry is found in `.netrc`.

Modifications
-------------

Correct the `--help` option descriptions and manual page.
 
Result
------

Help and documentation accurately describe what the options do.

Test Plan
---------

All existing tests continue to pass.   No functional change.
@euanh euanh added area/documentation Improvements or additions to documentation semver/patch No public API change. labels May 1, 2025
@euanh euanh merged commit 0375ff1 into apple:release/1.0 May 1, 2025
23 checks passed
@euanh euanh deleted the release-1.0-cherry-picks branch May 1, 2025 08:29
@euanh euanh added the kind/cherry-pick Back-port changes from one release or branch to another label May 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/documentation Improvements or additions to documentation kind/cherry-pick Back-port changes from one release or branch to another semver/patch No public API change.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant