Harden SPM integration test to guard visionOS slices - #3073
Conversation
Make visionOS device + simulator slices default-on in the SPM integration test and verify they are present in the built MSAL.xcframework, so the published binary can never silently ship out of sync with Package.swift's .visionOS() platform declaration (root cause of #2809). - spm-integration-test.sh: default INCLUDE_VISIONOS=true; add --skip-visionos opt-out (keep --include-visionos as a no-op alias); add plistlib verification that xros device + xros-simulator slices are present, failing the build if either is missing. - azure_pipelines/pr-validation.yml: pass --skip-visionos on the fast PR job, which does not install the visionOS SDK (visionOS is validated separately in visionos-validation.yml). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b06a8f53-b71a-4ce4-b94d-210d927e09fa
There was a problem hiding this comment.
Pull request overview
Hardens the Swift Package Manager (SPM) integration test so CI reliably detects regressions where MSAL.xcframework is missing required visionOS slices despite Package.swift declaring .visionOS.
Changes:
- Makes visionOS slices default-on in
spm-integration-test.sh, with a new--skip-visionosfast-path opt-out (and keeps--include-visionosas a backward-compatible alias). - Adds a post-build
Info.plistverification step (viaplistlib) to ensure bothxrosdevice andxrossimulator slices are present in the producedMSAL.xcframework. - Updates the fast PR validation pipeline job to run the SPM integration test with
--skip-visionos(visionOS validated elsewhere).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| spm-integration-test.sh | Defaults visionOS inclusion on, adds --skip-visionos, and validates the xcframework contains required visionOS slices. |
| azure_pipelines/pr-validation.yml | Skips visionOS slices in the fast PR job by passing --skip-visionos to the SPM integration test script. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Address review feedback on #3073: - Add an ERR trap that dumps build.log on any failure, so failing xcodebuild archive steps (which run with -quiet and redirect to build.log) surface an actionable error in the CI log. - Add an explicit existence check for MSAL.xcframework/Info.plist before parsing it, emitting a clear error instead of an unhandled Python stack trace if xcframework creation failed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b06a8f53-b71a-4ce4-b94d-210d927e09fa
Address high-severity review feedback on #3073: the script is invoked via `sh spm-integration-test.sh` and has no shebang, so the previous `trap ... ERR` (a bash-ism) is unreliable under a POSIX /bin/sh such as dash and could abort the script before any build step. Replace it with a POSIX-compatible EXIT trap that inspects the exit status and dumps build.log only on a non-zero exit. Validated with `dash -n` and a functional set -e test under sh. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b06a8f53-b71a-4ce4-b94d-210d927e09fa
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
spm-integration-test.sh:23
- Issue: The script treats the first positional argument as the branch name (
BRANCH_NAME="$1"), but it then parses flags by iterating over all of"$@", which still includes that branch name. If the branch name is missing (or accidentally starts with--), it can be misinterpreted as a flag, producing an invalid temporary publish (wrong branch name/URL) or unintentionally toggling visionOS behavior.
Impact: A malformed invocation (e.g., missing branch argument due to a pipeline variable issue) can silently publish to the wrong ref and/or skip the intended visionOS validation, making CI behavior brittle and harder to debug.
Recommendation: Validate that a branch name was provided, shift it off before parsing flags, then parse options from the remaining args only.
BRANCH_NAME="$1"
# visionOS device + simulator slices are included by default so the published
# xcframework never ships out of sync with Package.swift's .visionOS() platform.
# Use --skip-visionos on fast paths (e.g. regular PR validation) that do not have
# the visionOS SDK installed. --include-visionos is kept as a no-op alias for
# backward compatibility with existing callers.
INCLUDE_VISIONOS=true
SKIP_SAMPLE_APP=false
# Parse optional flags
for arg in "$@"; do
case $arg in
--include-visionos)
INCLUDE_VISIONOS=true
;;
--skip-visionos)
INCLUDE_VISIONOS=false
;;
--skip-sample-app)
SKIP_SAMPLE_APP=true
;;
esac
done
spm-integration-test.sh:71
- Issue: The error message says "See build.log above", but
build.logis emitted by theEXITtrap after the script exits, so it will appear after this line in CI output.
Impact: When Info.plist is missing, the CI log guidance is misleading and makes it slightly harder to quickly find the relevant xcodebuild output.
Recommendation: Adjust the message to point to the build.log dump that will be printed on exit.
if [ ! -f framework/MSAL.xcframework/Info.plist ]; then
echo "** ERROR: framework/MSAL.xcframework/Info.plist not found; xcframework creation likely failed. See build.log above. **"
exit 1
| # Use --skip-visionos on fast paths (e.g. regular PR validation) that do not have | ||
| # the visionOS SDK installed. --include-visionos is kept as a no-op alias for | ||
| # backward compatibility with existing callers. | ||
| INCLUDE_VISIONOS=true |
There was a problem hiding this comment.
Just curious about flipping the default here. Both callers in the repo already pass a flag explicitly - pr-validation.yml now passes --skip-visionos and visionos-validation.yml passes --include-visionos --skip-sample-app - so the new default doesn't add any coverage, but it does break anyone running the script locally without the visionOS SDK installed. I guess the real regression guard is the slice verification below, so keeping INCLUDE_VISIONOS=false would give us the same protection with less surprise? :)
Summary
Regression guard for #2809. Makes the visionOS device + simulator slices default-on in the SPM integration test and verifies they are actually present in the built
MSAL.xcframework, so the published binary can never again silently ship out of sync withPackage.swift's.visionOS(.v1)platform declaration.This is a complement to the actual binary fix in the internal OneBranch release pipeline — it protects the GitHub-side CI so the mismatch that caused #2809 cannot regress unnoticed.
Changes
spm-integration-test.shINCLUDE_VISIONOS=true(previouslyfalse).--skip-visionosopt-out for fast paths that don't install the visionOS SDK.--include-visionosis kept as a no-op alias for backward compatibility.plistlibthat bothxros(device) andxros-simulatorslices are present inMSAL.xcframework/Info.plist, failing the build if either is missing.azure_pipelines/pr-validation.yml--skip-visionoson the fast PR job, which does not install the visionOS SDK. visionOS is validated separately invisionos-validation.yml.Context
Package.swiftdeclares.visionOS(.v1), but the releasedMSAL.zipxcframework shipped no visionOS slice, so SPM resolved and then failed at link time (no library for this platform was found in MSAL.xcframework). The binary is produced by the internal release pipeline, which is fixed separately; this PR ensures the repo's own CI would catch a missing-slice regression.Testing
ruby/shell parse-checked.plistlibverification runs only when visionOS is included (i.e., not on the--skip-visionosfast PR job).Related: #2809, #2850