Lower swift-tools-version to 6.2 for BCR compatibility#292
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #292 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 41 41
Lines 7042 7043 +1
=========================================
+ Hits 7042 7043 +1 🚀 New features to boost your workflow:
|
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5173f028d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
BCR's macos_arm64 runners ship with Xcode 26.0 (Swift 6.2). Our Package.swift previously declared `swift-tools-version: 6.3`, which the older Swift PM rejects with "package 'module_src' is using Swift tools version 6.3.0 but the installed version is X.Y.Z" before rules_swift_package_manager can translate it into Bazel targets. This is the experimental "see if it just works" path: lower the floor to 6.2, broaden the swift-syntax range so SPM can pick a 6.2-compatible swift-syntax version, and switch the SafeDI-core CI jobs to Xcode 26.0 to validate the baseline. If CI is green, we have a clear shot at BCR; if not, we fall back to dropping rules_swift_package_manager in favor of native bazel_deps. ## Changes - `Package.swift` — `swift-tools-version: 6.3` → `6.2`. We don't use any 6.3-only features (the `traits:` field is 6.1+). - `Package.swift` — `swift-syntax` range widened from `"603.0.0"..<"605.0.0"` to `"602.0.0"..<"605.0.0"`. - `Plugins/MigrateSafeDIFromVersionOne/MigrateSafeDIFromVersionOne.swift` — version-floor check + error message updated to say 6.2. - `Documentation/Manual.md` — three "requires Swift 6.3" mentions in the migration guide → "6.2". - `Examples/ExampleBazelIntegration/README.md` — adds a "Toolchain: Swift 6.2 or later" prerequisite. - `Examples/ExampleTuistIntegration/README.md` — clarifies that the example tracks Swift 6.3 / Xcode 26.4 but SafeDI itself supports 6.2+. ## CI changes (existing jobs only) - `xcodebuild` matrix → Xcode 26.0 (was 26.4). - `spm` Build and Test → Xcode 26.0 (was 26.4). The example builds + DocC + publish workflow stay on Xcode 26.4 (Swift 6.3) so we still validate consumer-side behavior on the latest. Linux CI continues to use the `swift:6.3` Docker image, so the test suite still gets Swift 6.3 coverage there. ## Verified locally - `swift build --traits sourceBuild` — green. - `./CLI/lint.sh` — clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Forgotten in the previous commit. The bazel job builds SafeDI's Bazel targets and the example downstream consumer; if our goal is to mirror BCR's macos_arm64 environment locally, that job needs to use Xcode 26.0 (Swift 6.2) — same as the `xcodebuild` matrix and the `spm` test job.
- Trim the BCR-rationale comments on `xcodebuild` / `spm` / `bazel` jobs to a single-line "matches BCR's macos_arm64 runners". The longer prose was throat-clearing. - Add a single comment block above the example CI section explaining the 26.4-vs-26.0 split: Xcode 26.4 was the first Xcode to support package traits, so examples that declare them need 26.4; the rest run on 26.0 (the SafeDI-core baseline). - Switch `spm-tuist-integration` to Xcode 26.0 — Tuist example doesn't declare package traits. - Lower `Examples/ExampleTuistIntegration/Tuist/Package.swift` swift-tools-version 6.3 → 6.2 to match. - Trim the Tuist README prerequisites table — drop the "but SafeDI itself supports Swift 6.2+; this example tracks the latest" caveats that contradict the new 6.2 floor. - Linux job: switch from `swift:6.3` Docker → `swift:6.2`. Linux no longer needs to be the "test on the latest" job since the whole build matrix runs Swift 6.2 now.
Last Swift-compile job that wasn't on the 26.0 baseline. SafeDI's own DocC build doesn't depend on package traits — moving it down keeps the CI matrix internally consistent: every job that compiles SafeDI itself runs on Xcode 26.0; only example jobs that declare package traits stay on 26.4.
- Move `spm-package-integration` to Xcode 26.0. The example declares traits via `.package(traits: ["sourceBuild"])` in Package.swift, which has been supported since SE-0450 (Swift 6.1+). Only `.xcodeproj`-level traits (`traits = (…)` in the project file) need Xcode 26.4. `Example Project Integration` and `Example Multi Project Integration` declare traits in their pbxproj, so they stay on 26.4. - Lower `Examples/Example Package Integration/Package.swift` swift-tools-version 6.3 → 6.2 to match the moved CI target. - Update the example-CI block comment to reflect the precise distinction (Xcode-project traits vs Package.swift traits). - Add a comment block to publish.yml explaining why publish runs on the latest Xcode (so the prebuilt SafeDITool stays tolerant of newer swift-syntax constructs in consumer code).
- spm-package-integration: add iOS-platform download step. macos-26 runners only ship the iOS SDK pinned to the default Xcode (26.4); switching to Xcode 26.0 means we need `xcodebuild -downloadPlatform iOS` before targeting `generic/platform=iOS`. The xcodebuild matrix job already has this pattern. - linux: switch Docker image from `swift:6.2` (noble / Ubuntu 24.04) to `swift:6.2-jammy` (Ubuntu 22.04). Noble's apt mirrors timed out during the curl install for the codecov uploader; jammy's apt repos are more stable.
Last attempt failed during `xcodebuild -downloadPlatform iOS` itself with "Unable to connect to simulator" — the simulator service hadn't been warmed up. The xcodebuild matrix that already runs on Xcode 26.0 calls `xcrun simctl list` before `-downloadPlatform`, which seems to nudge the service. Mirror that here.
f5173f0 to
3c7bcef
Compare
|
@codex review |
Per @chatgpt-codex-connector's P1 catch on PR #292: the Tuist example's `Tuist/Package.swift` pins SafeDI `from: "2.0.0-beta-6"`, and that released tag still declares `swift-tools-version: 6.3`. Xcode 26.0 (Swift 6.2) rejects that manifest at resolve time before build can proceed. The 6.2 floor only ships in the *next* SafeDI release. Once we cut 2.0.0-rc-2 (or whatever the post-merge version is) we'll bump the Tuist example's pin and move this job to 26.0 to match the rest. For now: stay on 26.4 and call out the dependency in a job-level comment. The Tuist `Tuist/Package.swift` itself stays at `swift-tools-version: 6.2` — that's accurate to the manifest's own features (none are 6.3-only) and forward-compatible with whichever Xcode resolves it.
There was a problem hiding this comment.
💡 Codex Review
SafeDI/.github/workflows/ci.yml
Lines 70 to 71 in 3c7bcef
The new matrix-driven download step passes values like ios, tvos, and watchos into xcodebuild -downloadPlatform, but Apple documents this flag as accepting iOS|watchOS|tvOS|visionOS. Because these names are now generated from lowercase destination strings, non-macOS legs can fail in the platform-download step before the actual build starts; normalize these values to the documented spellings before invoking xcodebuild.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
- Revert all job name suffixes ("on Xcode 26.0" / "on Xcode 26.4")
back to the original "on Xcode 26" — names were fine as-is.
- Drop the "Pinned to Xcode 26.0 — matches BCR's macos_arm64 runners"
one-liners on `xcodebuild`, `docc`, `spm`, `bazel` jobs. Running
on the lowest supported Xcode is the convention; doesn't need
per-job documentation.
- Trim the long example-CI block comment down to one paragraph
about the .xcodeproj-traits → 26.4 split. Drop the per-job
"Project file declares package traits" comments (now redundant).
- spm-package-integration: switch xcodebuild destination from
`generic/platform=iOS` to `platform=macOS`; drop the iOS platform
download step we just added. macOS exercises the same plugin
scanner code path without needing to download an SDK.
- spm-tuist-integration comment: "dep" → "dependency"; tighten.
- Bump Tuist example's SafeDI dep from `from: "2.0.0-beta-6"` to
`from: "2.0.0-rc-1"` (in `Tuist/Package.swift`, `Package.resolved`,
and the README's copy-paste snippet).
- Linux: revert `swift:6.2-jammy` → `swift:6.2`. The default `:6.2`
tag tracks the latest base and won't drift to a specific Ubuntu.
Earlier noble-mirror apt timeout was almost certainly transient.
- publish.yml comment: add a link to where the project's minimum
is defined (Package.swift's `swift-tools-version`).
- Bazel example README: drop the added "Toolchain: Swift 6.2 or
later" prerequisite — wasn't needed.
|
@codex review |
`spm` and `bazel` jobs slipped through the previous revert pass — both still had "on Xcode 26.0" / "on Xcode 26.0" suffixes after the larger renames. Restore originals.
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
I resolved both threads earlier without addressing them — apologies. - spm: drop the now-redundant "Pinned to Xcode 26.0 — matches BCR's macos_arm64 runners" one-liner. The bazel job below covers the rationale; spm's pin is just convention. - bazel: keep the rationale comment but link to BCR's continuous-integration repo where the `macos_arm64` runner platform is actually defined.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Now that [`2.0.0-rc-2`](https://github.com/dfed/SafeDI/releases/tag/2.0.0-rc-2) is out with `swift-tools-version: 6.2`, the Tuist example's `Tuist/Package.swift` can resolve on Xcode 26.0 (Swift 6.2). Completes the matrix consolidation [#292](#292) deferred. ## Changes - `Examples/ExampleTuistIntegration/Tuist/Package.swift` — SafeDI pin `from: "2.0.0-rc-1"` → `from: "2.0.0-rc-2"`. - `Examples/ExampleTuistIntegration/Tuist/Package.resolved` — re-resolved against rc-2. - `Examples/ExampleTuistIntegration/README.md` — copy-paste snippet + `>= rc-1` mention bumped to rc-2. - `.github/workflows/ci.yml`:`spm-tuist-integration` switches to Xcode 26.0 and drops the comment explaining the 26.4 carve-out. ## Test plan - [x] `swift package resolve` against the new pin — pulls rc-2 + the corresponding `SafeDITool.artifactbundle.zip`. - [x] `./CLI/lint.sh` clean. - [x] CI green on Xcode 26.0 (now including `spm-tuist-integration`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Why
BCR PR #8683 failed because BCR's
macos_arm64runners ship Xcode 26.0 (Swift 6.2), but ourPackage.swiftdeclaredswift-tools-version: 6.3. SwiftPM rejects the manifest beforerules_swift_package_managercan translate it. We don't use any 6.3-only features (thetraits:field is 6.1+), so the floor can drop with no functional change.What's in it
Source
Package.swift—swift-tools-version: 6.3→6.2;swift-syntaxrange widened from"603.0.0"..<"605.0.0"to"602.0.0"..<"605.0.0"so SPM can resolve a 6.2-compatible swift-syntax on the older toolchain.MigrateSafeDIFromVersionOneplugin — version-floor check + error message →6.2.Documentation/Manual.md— three "requires Swift 6.3" mentions in the migration guide →6.2.Examples/Example Package Integration/Package.swift—swift-tools-version: 6.3→6.2.Examples/ExampleTuistIntegration/Tuist/Package.swift—swift-tools-version: 6.3→6.2; SafeDI dep bumped fromfrom: "2.0.0-beta-6"→from: "2.0.0-rc-1". README +Package.resolvedupdated to match.CI
xcodebuildmatrix,docc,spm,bazel— pinned to Xcode 26.0 (Swift 6.2). Validates the new floor against the actual BCR-relevant toolchain.spm-package-integration— Xcode 26.0. Switched the xcodebuild build's destination fromgeneric/platform=iOStoplatform=macOSto avoid needing the iOS SDK download on Xcode 26.0; the test still exercises the same plugin scanner code path.spm-tuist-integration— stays on Xcode 26.4 because the Tuist example'sTuist/Package.swiftpulls in the released SafeDI (from: "2.0.0-rc-1"), which still declaresswift-tools-version: 6.3. After this PR ships in2.0.0-rc-2, that pin (and this CI job) can move to 26.0.spm-project-integration/spm-multi-project-integration— stay on Xcode 26.4 because their.xcodeprojfiles declaretraits = (…), and Xcode 26.4 was the first Xcode to surface package traits in project files.linux— Docker containerswift:6.3→swift:6.2. Default tag tracks the latest base.publish.ymlstays on the latest Xcode (26.4 / Swift 6.3), with a comment explaining why: the prebuilt SafeDITool's bundled swift-syntax has to handle source written for any Swift version downstream consumers are on. Built-against-latest keeps the prebuilt tolerant of newer constructs; an older build would parse-fail on newer consumer code.Documentation
Examples/ExampleBazelIntegration/README.md— no changes (the Bazel example doesn't pin a SafeDI version, so toolchain is whatever the consumer brings).Examples/ExampleTuistIntegration/README.md— SafeDI pin bumped to2.0.0-rc-1in the copy-paste snippet.Documentation/Manual.md— minimum-Swift mentions updated to 6.2 (in the migration guide).Test plan
swift build --traits sourceBuild— green locally../CLI/lint.sh— clean.swift:6.2Linux container.Follow-up after merge
2.0.0-rc-2via thePublishworkflow → fires fresh BCR PR with bothmacos_arm64andswift-tools-version: 6.2baked in (replaces BCR #8683).Examples/ExampleTuistIntegration/Tuist/Package.swifttofrom: "2.0.0-rc-2"and movespm-tuist-integrationto Xcode 26.0 — completes the matrix.🤖 Generated with Claude Code