Repository navigation
Swift GitHub Actions Workflows Unification. - #316
Conversation
Bring together the best aspects of the swiftlang GitHub Actions workflows in this repository and the SwiftNIO ones at apple/swift-nio, through a new reusable workflow that sits beside the existing one and a hierarchy of supporting infrastructure. Key design features: 1) GitHub-release-based versioning 2) No skipped jobs 3) Scripts are cloned not curled 4) Custom matrix builds 5) Minimum Swift version detection `package_test.yml` runs a range of common test and build configurations on a variety of platforms against multiple Swift versions. It sits atop a matrix generation layer which takes inputs and produces a canonical work definition in YAML or JSON; that definition is then expanded into one job per entry, each executing one slice of the work. Benchmarking is a further consumer of the same layer rather than part of the recommended test workflow. The design is layered, so an adopter may use the whole stack or customize portions beyond the level the workflow allows: * Use `package_test.yml` for the full suite of conveniences. * Use a custom workflow which calls `toolchain_matrix.yml` for the matrix and `execute_matrix.yml` to run its own command across it. * Generate the work definition by some other means, or hard-code it into a workflow, and pass it to `execute_matrix.yml`. The matrix generation layer is a Swift script, `generate-matrix.swift`, run by the matrix job on `ubuntu-latest`, which comes with the Swift, jq and yq it needs. It reads the workflow's inputs from the environment, applies the defaults and the minimum version filter, and writes the work definition to standard output. In SwiftNIO it was a bash script, which executes faster, but the unified version is much more complex, so Swift seemed like the more maintainable option. A test package under `tests/MatrixGeneratorValidator` covers the inputs, their defaults and failure modes. Execution times are reduced by defaulting to running natively on the runner rather than inside a container. The toolchain is installed with swiftly, which was previously used only for macOS toolchains, so using it on Linux is new to both upstreams. Containers remain available for a package that needs a particular distribution or system dependencies, through `linux_use_docker` or by naming a distribution in `linux_os`. The existing workflows are untouched, so a repository migrates when it suits it. The most visible change on migrating is that the `*_exclude_swift_versions` inputs are gone, replaced by explicit version lists and minimum-version detection which drops anything the package manifest cannot support. A repository that was only excluding old versions can delete those inputs entirely. Migrating from the SwiftNIO workflows is a change in spelling rather than a reduction in flexibility: per-version inputs become one map, which removes the churn of adding an input per Swift release. Functionality new to this repository, some inspired by SwiftNIO and some new to both: * Simulator testing. iOS was build-only; xcodebuild now builds and tests on simulators, with tvOS, watchOS and visionOS alongside it. Mac Catalyst is new to both upstreams. * Benchmarks across the matrix. `benchmarks.yml` checks committed thresholds under `Thresholds/<swift-version>/` on every platform and Swift version, failing the job and printing a diff when one moves. This differs from the existing `performance_test.yml`, which compares a pull request against its merge base in one container and posts the table as a comment: that reports what a change did to performance, this holds a package to a standard it has written down. * A C++ interoperability check. A release-configuration build needs no dedicated input: it is a second labeled command, so `Release-Build Linux Swift 6.3` runs alongside `Debug-Test Linux Swift 6.3` from one call. * A semantic version pull request label check, `pull_request_label.yml`. * An importable Swift version matrix, `toolchain_matrix.yml`, which gives a custom workflow the current supported matrix as plain YAML, so it can be filtered or extended with yq before it is executed. * Per-version overrides, so one Swift version can take extra arguments or a different command without splitting the matrix by hand. Known limitation: a pinned tag still does not pin the scripts. This is inherited behavior. `swift_package_test.yml` and `soundness.yml` check this repository out with no `ref:`, so they resolve to its default branch, and one variant hardcodes `ref: main`. A caller on `@0.0.14` has therefore always run that tag's YAML against the default branch's scripts, and the unified workflows inherit that default. Resolving it is possible but would require stamping versions as part of the release process.
|
Overall this looks good. Thanks for working on it. I left some comments inline. One thing I would like to see added is the definition of the JSON/YML used by the matrix job so if somebody wants to hand-roll a matrix they have a reference to write it against.
We should create an issue to track this since we ran into problems with this in the past. |
| @@ -0,0 +1,47 @@ | |||
| # Swift Package Test Workflow Architecture | |||
|
|
|||
| ## Aims | |||
There was a problem hiding this comment.
Nit:
| ## Aims | |
| ## Goals |
|
|
||
| The Swift Package Test workflow is intended to give maintainers of open-source Swift packages a low-friction way to give contributors confidence in the code they write. This means making testing against a range of Swift versions and platforms as simple as possible, and making the results as accessible as possible. | ||
|
|
||
| This workflow builds on the GitHub Actions workflows from [apple/swift-nio](https://github.com/apple/swift-nio/tree/main/.github/workflows) and [swiftlang/github-workflows](https://github.com/swiftlang/github-workflows/tree/main/.github/workflows), taking from both repositories the key features listed below, which the unified architecture treats as requirements. |
There was a problem hiding this comment.
I would drop this since it won't stand the test of time once NIO removes its workflows.
| windows_job_timeout: | ||
| type: number | ||
| description: "Timeout in minutes for Windows jobs. 0 uses job_timeout. Windows installs a toolchain and Visual Studio Build Tools before building, so it often needs longer." | ||
| default: 0 | ||
| freebsd_job_timeout: |
There was a problem hiding this comment.
Why do we need different timeouts per platform? If we do should this be configured in the matrix instead?
There was a problem hiding this comment.
Different platforms I think will have different performance characteristics because of different virtualization strategies/levels and/or prerequisites. I think it could go into the matrix but I haven't made that change yet.
There was a problem hiding this comment.
I think adding this to the matrix seems sensible to me
| benchmark_package_path: | ||
| type: string | ||
| description: "Path to the directory containing the benchmarking package. Used only when benchmark_package_paths is empty." | ||
| default: "." | ||
| benchmark_package_paths: |
There was a problem hiding this comment.
Do we need both or would just the plural be enough?
There was a problem hiding this comment.
Plural would be enough, just matching existing ergonomics for people.
There was a problem hiding this comment.
Since this is new can we just go with the plural spelling please
| android_sdk_versions: | ||
| type: string | ||
| description: "Android Swift SDK version list (JSON array)." | ||
| default: '["6.3", "nightly-release", "nightly-main"]' |
There was a problem hiding this comment.
Better to make the first one "release" to signify the current release, then let people add 6.1 or whatever else they want also?
There was a problem hiding this comment.
Hey @finagolfin, thanks for taking a look at the PR. I'm happy to be led by the consensus here, but I have discussed this with many Swift server repo admins and the (split) majority opinion was to only have sliding nightly versions. That way they can control when new Swift versions appear in their supported matrix. That said, I think nothing stops us from having both ways of spelling this so users could opt-in, but I think that's an enhancement.
The workflows' version-list inputs now default to empty, which the generator reads as its own list, so a new release is added in one place.
* Add a reference for the matrix YAML to architecture.md * Rename Aims to Goals and drop the progenitor paragraph * Explain why only macOS checks out linked PRs in execute_matrix.yml
An SDK build takes its own `*_sdk_version_overrides`, and falls back to `linux_version_overrides` when it has none.
Jobs run their setup command from the repository root, then enter `package_path` for the command, and minimum Swift version detection reads that directory's manifest.
Forks do not run Actions until the owner enables them, so the input guarded only the case of a fork that had. Its generator check and the inputs that forwarded it are gone.
FranzBusch
left a comment
There was a problem hiding this comment.
One small comment and then the 2-3 open comments where I left a new reply. Otherwise this looks really good to me
The singular input duplicated the plural, which covers one path with a one-element list and defaults to the current directory when empty.
The generator emits `timeout_minutes` on Windows and FreeBSD entries from `windows_job_timeout` and `freebsd_job_timeout`, and `execute_matrix.yml` reads it from the entry, falling back to `job_timeout`. The executor no longer has a timeout input per platform, and a hand-written matrix can set one per entry.
Job names are required status checks, and a new Swift release renamed them. The default version window also lived in the generator, which a pinned workflow version does not pin, so a release moved the versions under every adopter. `current`, `current-1` and `current-2` are now version labels, resolved by `SwiftReleases`, and a job is named for the label the caller wrote. The releases they count back from come from new `swift_current_release` and `swift_nightly_release` inputs whose defaults live in each workflow, so the generator holds none of its own and a pinned workflow keeps the versions it was released with. The `macos_swiftly_toolchains` default moved back into the workflow for the same reason, and a test checks that the three workflows declare the same releases.
`SwiftReleases` parsed a label from its string shape and re-derived facts `SwiftVersion` already held, which needed `components` widened to `fileprivate`. `VersionLabel` is now the only place that rejects a malformed relative label, `SwiftVersion` exposes `major`, `minor` and `init(majorMinor:)`, and `SwiftReleases.ordered` resolves and orders a label in one call, so `components` is private again. The releases-agree test now compares the other workflows against the first.
|
After discussion with @FranzBusch I've pushed some commits which default the job naming/definition to floating |
An override that applies to every version had to be written out once per version, so a change of release meant editing a block of identical lines. A `default` key in a `*_version_overrides` input now sets the arguments for every version whose own key does not name some, and a version key replaces it for the fields it names. Arguments written alone, with no key, are the same as `default`. A command under `default` is an error, since it would give every command label the same one.
An unreadable directory was read as having no `Package.swift`, which raises no floor, so minimum-version detection could let a too-old Swift version through. `MinimumVersion.detected` now stops with the directory and the error, as reading a manifest already does. The test support's cleanup and scalar decoding no longer swallow errors silently: the cleanup prints a warning, and the decode says why a failed shape is not an error.
A version label, the version it resolves to, and the release a relative label counts back from were all `version`, `current` or `releaseToken`, and the test runner shared the generator's type name. `Combination.version` is `versionLabel`, as are the parameters of the override lookups, `Variant.swiftVersions` is `versionLabels`, `SwiftReleases.version(for:)` is `resolvedVersion(for:)`, and `SwiftReleases.current` is `currentRelease`. `Toolchain` and `Configuration` both name the nightly branch `nightlyBranch`, and the script's global is `configuration`. In the test support, `Generator` is `GeneratorRunner`, `Generated` is `GeneratorRun`, and the entry's FreeBSD configuration is `FreeBSDBuild`. The JSON keys are unchanged.
Whether a name is a nightly was decided by a string prefix at five sites, the Android build was three optional fields that had to be set together, and an unset release was an empty string checked where it was used. `VersionLabel.isNightly` is the one definition of a nightly. `SwiftBuildJobs.android` holds the triples, NDK releases and emulator flag of the Android build as one value, so a half-configured one cannot be represented. `SwiftReleases` holds the releases as optionals, with the empty input turned into `nil` where it is read, and the messages for an unset release are unchanged.
Wrapped two long messages, reflowed two comments, put a blank line after multi-line blocks and between the cases of switches with a long case, replaced guard-not with if, and dropped redundant comparisons on Bool.
The checks every swiftlang package is expected to pass (macOS, macOS with swiftly, the Static Linux, Wasm, Embedded Wasm and Android SDK builds, the Android emulator tests, and the Cxx interoperability check) were opt-in, so a package adopting `package_test.yml` ran none of them unless it asked. `package_test.yml` and the generator now enable each of them by default, and a package opts out with `enable_x: false`. FreeBSD stays opt-in. `benchmarks.yml` and `toolchain_matrix.yml` set the groups they do not use to false so their matrices are unchanged, and the self-tests that were about Linux and Windows name what they leave out. The test runner disables every job group before applying a test's own environment.
shahmishal
left a comment
There was a problem hiding this comment.
Please hold off from merging this, this is a major change. I would like to review it before it's merged.
@rnro can you look into this? |
|
trying a fix here #329 |
A step's `shell:` cannot read the matrix, so GitHub rejected the whole `pull_request.yml` with "Unrecognized named-value: 'matrix'" and ran none of its jobs. The job now sets `defaults.run.shell` from the matrix, where the context is available.
The soundness check flags `hung`, and a comment in `tests/execute-matrix-tests.sh` used it for a VM that stops responding. It says `stuck` now.
`swift-format` rewrites a script that begins with a shebang and a comment, joining the two onto one line and dropping the executable bit, and the formatting check rejects the result. The generator is now an ordinary source file that the workflows and the test runner run with `swift`, as `cross-pr-checkout.swift` already is.
A Windows container runs its commands under cmd, and `cd "tests/TestPackage"` fails there with "The system cannot find the path specified" when the path is quoted and written with forward slashes. A Windows entry now enters the package with backslashes, which cmd and PowerShell both accept.
A Windows container receives its commands as one command line for cmd, and a double quote in it arrives as a literal backslash and quote, so the quoted `cd` could not find the path whatever separator it used. A Windows entry that runs in a container now enters the package with a bare, backslash-separated path and refuses one with whitespace, a Windows entry on the runner keeps the quoted PowerShell form, and the container self-test builds with `swift build`, since `Invoke-Program` does not exist under cmd.
|
Thanks @shahmishal . I think the remaining CI issues are pre-existing now |
| android_ndk_versions: | ||
| type: string | ||
| description: "Android NDK version list (JSON array)." | ||
| default: '["r27d", "r28c"]' |
There was a problem hiding this comment.
We should bump this to r30. Overall, let's match the other workflow defaults.
…imeout `swift_package_test.yml` now defaults the Android NDK to `r30` alone and bounds its Linux, Static SDK, Wasm and Android steps at 90 minutes, while `package_test.yml` still defaulted to `r27d` and `r28c` and 60 minutes, so a package moving between them would have changed what it builds and how long it may take. `package_test.yml` and the generator default `android_ndk_versions` to `r30`, and `job_timeout` defaults to 90 in `package_test.yml` and `execute_matrix.yml`.
|
Swift GitHub Actions Workflows Unification.
This PR aims to bring the best aspects of the swiftlang GitHub Actions workflows (this repository) and the SwiftNIO (apple/swift-nio) workflows together. It does this chiefly through a new re-usable workflow which sits beside the existing workflow, and a hierarchy of supporting infrastructure.
Some key design features:
The unified design provides one workflow which can be adopted to run a range of common test and build configurations on a variety of platforms against multiple Swift versions. This workflow sits atop a "matrix generation" layer which takes inputs and produces a canonical work definition (in YAML or JSON). That definition is then expanded into one job per entry, each executing one slice of the work. Benchmarking is a further consumer of the same layer rather than part of the recommended test workflow.
The design is intended to be layered, so that adopters may use the whole stack, or customize portions beyond the level which the workflow allows. Adopters may:
package_test.ymlworkflow for the full suite of conveniencestoolchain_matrix.ymlfor the matrix andexecute_matrix.ymlto run its own command across itexecute_matrix.ymlThe matrix generation layer is a Swift script,
generate-matrix.swift, run by the matrix job onubuntu-latest, which comes with the Swift, jq and yq it needs. It reads the workflow's inputs from the environment, applies the defaults and the minimum version filter, and writes the work definition to standard output. In SwiftNIO it was a bash script, which executes faster, but the unified version is much more complex, so Swift seemed like the more maintainable option. A test package undertests/MatrixGeneratorValidatorcovers the inputs, their defaults and failure modes.As part of this work an effort was also made to reduce execution times, defaulting to running natively on the runner rather than inside a container where possible. The unified workflows install the toolchain with swiftly on the runner rather than pulling a full Docker image. Containers remain available for a package that needs a particular distribution or system dependencies, either through
linux_use_dockeror by naming a distribution inlinux_os. swiftly was previously used only for macOS toolchains, so using it on Linux is new to both upstreams.Migrating from this repository's existing workflow
The new workflow is currently available alongside existing infrastructure so a repository can control when they perform the migration.
When adopting the new workflow the most visible change is that the
*_exclude_swift_versionsinputs are gone, replaced with explicit supported Swift version lists and minimum-version detection which drops anything the package manifest cannot support. A repository that was only excluding old versions based on its Swift tools version can simplify its config.Before:
After:
The new architecture means that jobs which are not intended to be run will no longer appear as skipped:
Before:

After:

Migrating from the SwiftNIO workflows
Changing from the SwiftNIO workflows means a change in spelling but no reduction in flexibility. Per-version inputs are now supplied in a map which reduces churn and synchronization problems when a new Swift version is released.
Before:
After:
Functionality new to this repository
The unification also brings some new functionality to this repository. Some inspired by SwiftNIO, some entirely new:
xcodebuildnow builds and tests on simulators, and tvOS, watchOS and visionOS join it. Mac Catalyst is new to both upstreams.benchmarks.ymlmeasures performance with Swift Package Benchmark, checking committed thresholds underThresholds/<swift-version>/on every platform and Swift version in the matrix, failing the job and printing a diff when one moves. This differs from the existingperformance_test.yml, which compares a PR against its merge base in one container and posts the table as a PR comment. The existing workflow tells you what a change did to performance; the new one holds a package to a standard it has written down.Release-Build Linux Swift 6.3runs alongsideDebug-Test Linux Swift 6.3from one call.pull_request_label.yml), which fails a PR that carries no SemVer label.toolchain_matrix.yml, provides the current supported Swift version matrix for custom workflows to utilize. The output is plain YAML, so it can be filtered or extended withyqbefore it is executed.Known limitations
swift_package_test.ymlandsoundness.ymlcheck this repository out with noref:, so they resolve to its default branch, and one variant hardcodesref: main; this means a caller on a tag e.g.@0.0.14has always run that tag's YAML against the default branch's scripts. The unified workflows inherit that default. Resolving this should be possible but would require stamping versions as part of the release process.Updates
Changes made in response to review.
Default job names are now relative to the current Swift release.
current-2,current-1andcurrent, so a default job isLinux Swift current-1rather thanLinux Swift 6.3. Job names are the required status checks in adopting repositories, and naming them for a version meant every Swift release renamed them and sent someone to branch protection. The nightlies keep their names. A version list that writes versions out (["6.2", "6.3"]) getsLinux Swift 6.3as before, and the two spellings can be mixed in one list, for example["6.0", "current"]. Anything older thancurrent-2has to be written as a version. The job names in the examples and screenshots above are what explicit spelling produces.linux_swift_versions: '["6.2", "6.3", "6.4", "nightly-release", "nightly-main"]', and the jobs are named for those versions.currentnames comes from the workflow, not the generator.swift_current_releaseandswift_nightly_releaseare inputs with defaults in each workflow file, so a workflow pinned to a tag keeps the versions it was released with, even though the scripts still come from the default branch (see "Known limitations"). A new Swift release is a change to those defaults.swift_current_releaseholds a repository at an earlier release without renaming its checks.Other changes
package_pathruns a package that is not at the repository root. Jobs run the setup command from the root, then enter the directory for the command, and minimum Swift version detection reads that directory's manifest.linux_static_sdk_version_overrides,wasm_sdk_version_overrides,embedded_wasm_sdk_version_overridesandandroid_sdk_version_overrides. A build with none of its own useslinux_version_overrides.timeout_minutes, fromwindows_job_timeoutandfreebsd_job_timeout, andexecute_matrix.ymlreads it from the entry, falling back tojob_timeout. A hand-written matrix can set one per entry.benchmarks.ymltakes onlybenchmark_package_paths. The singular input is gone.macos_repository_owneris gone. Forks do not run Actions until the owner enables them, so the input guarded only the case of a fork that had.architecture.md, for anyone writing a matrix by hand to pass toexecute_matrix.yml.