fix(release): default to embedded SwiftPM packaging and restore portable local/PR packages#406
Conversation
…ble local/PR packages The slim SPM-based packages from #405 were unusable outside the release pipeline: local builds stamped an absolute path into the checkout (machine-locked tgz) and PR artifacts pinned an ios-spm tag that is never created. Embedded packaging is now the default everywhere; the slim shape is an explicit opt-in used only by the release workflow. - build_npm_ios/vision gain --spm-mode <embedded|remote> (default embedded): the xcframework zips from build_spm_artifacts.sh are embedded at framework/internal/local-spm with a committed Package.swift template (spm-templates/), referenced by relative path — the tgz is self-contained and portable (zips because npm strips symlinks; SwiftPM extracts local zip binary targets). remote keeps the deploy shape pinning ios-spm; the release workflow builds with `npm run build-ios -- --spm-mode=remote`. The CI env-var heuristic and NS_SPM_LOCAL are gone. - pull_request.yml artifacts are usable again (embedded default) and the SwiftPM artifact zips are uploaded too. - release workflow cleanup: version/tag/matrix resolution moves to scripts/resolve-release.mjs; the checksum-arg loop and curl|grep manifest assertion fold into generate-spm-manifest.mjs (--checksums-dir, --channel) and generate-spm-probe.mjs (--assert-release-manifest); the 4x copy-pasted deps-install block becomes the setup-build-env composite action + scripts/install-ci-deps.sh. - all touched scripts take real CLI arguments (util.parseArgs / while-case parsers, both --flag value and --flag=value, --help) instead of env flags and hand-rolled argv loops; no heredoc-generated files. - README documents both modes and local linking via --framework-path (and the dangling SPM_DISTRIBUTION_PLAN.md reference is removed).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe PR centralizes CI setup, adds release metadata resolution, introduces embedded and remote SwiftPM packaging modes for iOS and visionOS, adds local SwiftPM manifests, and consolidates release manifest generation and verification. ChangesRelease and packaging flow
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
scripts/install-ci-deps.sh (3)
33-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
npm cifor reliable CI builds.Using
npm ciinstead ofnpm installin CI environments guarantees a clean, reproducible installation strictly matching thepackage-lock.jsonand prevents accidental lockfile modifications during the build.♻️ Proposed refactor
-npm install +npm ci🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/install-ci-deps.sh` at line 33, Replace the npm install command in the CI dependency installation step with npm ci so installs remain clean and strictly follow package-lock.json without modifying it.
46-49: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDisable Homebrew auto-update to speed up CI.
Running
brew installin a CI environment can trigger a lengthy auto-update process. PrependingHOMEBREW_NO_AUTO_UPDATE=1prevents this delay.♻️ Proposed refactor
if [ "$TEST_TOOLS" = "1" ]; then - brew install chargepoint/xcparse/xcparse + HOMEBREW_NO_AUTO_UPDATE=1 brew install chargepoint/xcparse/xcparse npm install -g `@edusperoni/junit-cli-report-viewer` verify-junit-xml fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/install-ci-deps.sh` around lines 46 - 49, Update the Homebrew installation command in the TEST_TOOLS block to set HOMEBREW_NO_AUTO_UPDATE=1 for that invocation, while leaving the npm installation unchanged.
36-39: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDisable Homebrew auto-update to speed up CI.
Running
brew installin a CI environment can trigger a lengthy auto-update process. PrependingHOMEBREW_NO_AUTO_UPDATE=1prevents this delay.♻️ Proposed refactor
# Ensure CMake is available without conflicting with pinned Homebrew formula if ! command -v cmake >/dev/null; then - brew list cmake || brew install cmake + brew list cmake || HOMEBREW_NO_AUTO_UPDATE=1 brew install cmake fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/install-ci-deps.sh` around lines 36 - 39, Update the brew install fallback in the CMake availability check to set HOMEBREW_NO_AUTO_UPDATE=1 for the install command, while preserving the existing brew list fallback and command-not-found condition.build_npm_ios.sh (1)
5-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNear-duplicate
usage()/arg-parsing block withbuild_npm_vision.sh.Lines 5-59 here are essentially identical to
build_npm_vision.shlines 5-58 (only wording/tool names differ). Consider extracting the shared--spm-modeparsing into a small sourced helper (e.g.scripts/parse-spm-mode.sh) to avoid maintaining two copies that can drift.♻️ Suggested shared helper sketch
# scripts/parse-spm-mode.sh (sourced) parse_spm_mode_args() { SPM_MODE="embedded" while [ $# -gt 0 ]; do case "$1" in --spm-mode) SPM_MODE="$2"; shift 2 ;; --spm-mode=*) SPM_MODE="${1#*=}"; shift ;; -h|--help) usage; exit 0 ;; *) echo "Unknown argument: $1" >&2; usage >&2; exit 1 ;; esac done case "$SPM_MODE" in embedded|remote) ;; *) echo "Invalid --spm-mode '$SPM_MODE'" >&2; usage >&2; exit 1 ;; esac }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@build_npm_ios.sh` around lines 5 - 59, Extract the duplicated --spm-mode argument parsing and validation from the usage/argument-handling blocks in build_npm_ios.sh and build_npm_vision.sh into a shared sourced helper. Keep each script’s tool-specific usage output, ensure the helper preserves defaulting to embedded, both option syntaxes, missing-value handling, help behavior, unknown-argument errors, and embedded/remote validation, then invoke the helper from both scripts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/npm_release.yml:
- Around line 107-111: Update the “Build (${{ matrix.target }})” step to pass
the matrix script through an environment variable, following the existing
INPUT_VERSION pattern, and invoke npm using that variable instead of
interpolating matrix.script directly in the shell command. Preserve the remote
--spm-mode argument.
In `@README.md`:
- Around line 270-273: Update the shell command fenced block in README.md to
include the shell language identifier on its opening fence, preserving the
existing commands and formatting.
In `@scripts/stamp-template-version.mjs`:
- Around line 18-19: Remove the duplicate let declaration for values and
positionals in the stamp-template-version.mjs initialization near the try block,
ensuring each variable is declared only once while preserving their existing use
in remote-mode stamping.
---
Nitpick comments:
In `@build_npm_ios.sh`:
- Around line 5-59: Extract the duplicated --spm-mode argument parsing and
validation from the usage/argument-handling blocks in build_npm_ios.sh and
build_npm_vision.sh into a shared sourced helper. Keep each script’s
tool-specific usage output, ensure the helper preserves defaulting to embedded,
both option syntaxes, missing-value handling, help behavior, unknown-argument
errors, and embedded/remote validation, then invoke the helper from both
scripts.
In `@scripts/install-ci-deps.sh`:
- Line 33: Replace the npm install command in the CI dependency installation
step with npm ci so installs remain clean and strictly follow package-lock.json
without modifying it.
- Around line 46-49: Update the Homebrew installation command in the TEST_TOOLS
block to set HOMEBREW_NO_AUTO_UPDATE=1 for that invocation, while leaving the
npm installation unchanged.
- Around line 36-39: Update the brew install fallback in the CMake availability
check to set HOMEBREW_NO_AUTO_UPDATE=1 for the install command, while preserving
the existing brew list fallback and command-not-found condition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: adb7dd29-2ec0-458f-8790-83e6a4717a2e
📒 Files selected for processing (18)
.github/actions/setup-build-env/action.yml.github/workflows/npm_release.yml.github/workflows/pull_request.ymlREADME.mdbuild_all_ios.shbuild_all_vision.shbuild_npm_ios.shbuild_npm_vision.shscripts/generate-spm-manifest.mjsscripts/generate-spm-probe.mjsscripts/get-next-version.jsscripts/get-npm-tag.jsscripts/install-ci-deps.shscripts/resolve-release.mjsscripts/stamp-template-local-spm.mjsscripts/stamp-template-version.mjsspm-templates/local-spm-ios/Package.swiftspm-templates/local-spm-visionos/Package.swift
…pm-mode parser Address CodeRabbit review on #406: - pass matrix.script to the build step through an env var (consistent with INPUT_VERSION; silences zizmor's template-injection warning — the values are hard-coded in resolve-release.mjs, so this is hardening, not a fix) - add the bash language tag to the README linking snippet (MD040) - extract the duplicated --spm-mode parsing from build_npm_ios.sh / build_npm_vision.sh into parse_spm_mode_args in build_utils.sh (already sourced by both; usage() stays per-script)
Problem
After #405, packages built outside the release pipeline are unusable:
XCLocalSwiftPackageReferencewith an absolute path into the developer's checkout (dist/local-spm) — the tgz only works on that machine, at that path, and can't be shared with teammates or an app's CI.ios-spmat a PR version whose tag is never created — thenpm-packageartifact could never resolve, and the xcframework zips weren't uploaded either.npm run build, which doesn't exist), andstamp-template-local-spm.mjscitedSPM_DISTRIBUTION_PLAN.md, which was never committed.npm_release.ymlhand-built JSON matrices in bash, looped checksum args,curl | grep'd manifests, and copy-pasted the deps-install block 4×.Change
Embedded packaging is now the default; the slim deploy shape is an explicit opt-in used only by the release workflow.
build_npm_ios.sh/build_npm_vision.sh(and thebuild_all_*wrappers) take--spm-mode <embedded|remote>:embedded(default): the xcframework zips frombuild_spm_artifacts.share embedded atframework/internal/local-spmwith a committedPackage.swifttemplate (spm-templates/), referenced by relative path. The tgz is fully self-contained and portable — zipped because npm strips symlinks (the Catalyst slices contain them); SwiftPM extracts local zip binary targets itself.remote: the deploy shape pinningios-spmat the package version; the release workflow builds withnpm run build-ios -- --spm-mode=remote. TheCIenv heuristic andNS_SPM_LOCALare removed.ns platform add ios --framework-path=<tgz>works on the downloaded artifact); the SwiftPM artifact zips are uploaded as well.scripts/resolve-release.mjs(matrix built withJSON.stringify, tag-vs-package.json assertion preserved; the dispatch input still flows through env to avoid shell interpolation of untrusted input)generate-spm-manifest.mjs --checksums-dir --channel(non-nextchannels still hard-require the visionOS checksums)generate-spm-probe.mjs --assert-release-manifest.github/actions/setup-build-envcomposite +scripts/install-ci-deps.sh(PR jobs move from Node 20 → 22, matching release builds)util.parseArgs(strict) in Node,while/caseparsers in bash; both--flag valueand--flag=valueforms,--helpeverywhere, hard failure on unknown args. No env-flag behavior switching, no heredoc-generated files (the localPackage.swiftis a committed template).Verification
./build_spm_artifacts.sh ios && ./build_npm_ios.sh→ extracted tgz hasrelativePath = "internal/local-spm", both zips + manifest inside, zero absolute paths,__NS_RUNTIME_VERSION__gone.__PROJECT_NAME__→TestApp) and ranxcodebuild -resolvePackageDependencies: resolvesNativeScriptSDK @ localfrom the embedded package.--spm-mode=remote→ template pinsios-spmexactVersion 9.0.3, no binaries in the tgz. Space/=flag forms, invalid mode, and unknown args all behave.resolve-release.mjsreproduces the old bash logic for all three trigger shapes (dispatch version, tag push incl. mismatch failure, main push →next+ iOS-only matrix).generate-spm-manifest.mjs --checksums-dirshapesnextmanifests iOS-only and failslatestwithout visionOS checksums;generate-spm-probe.mjs --assert-release-manifestexits non-zero on a 404/stale manifest.bash -nclean on every touched shell script.Not in scope (flagged separately): the
git tag -f/push -fon ios-spm release tags inspm-update.Summary by CodeRabbit
--spm-modecommand-line support (with help) and improved argument forwarding in build scripts.