Drop the duplicate Swift compile in build_and_test_samples - #632
Merged
kieran-osgood-shopify merged 1 commit intoAug 14, 2026
Merged
Conversation
Contributor
Author
This was referenced Aug 13, 2026
Part of the macOS CI speed-up. Tracked on shop/issues-checkout-kit#1203, under shop/issues-checkout-kit#1202. ## Problem `build_and_test_samples` builds two apps out of one `Samples.xcworkspace`. Both resolve the root Swift package and Apollo iOS 2.0.6. The second `run_app` passed `clean`, which deletes the shared build products the first `run_app` just produced, inside a single CI step. ## Change `platforms/swift/Scripts/build_and_test_samples:31` drops `clean` from the second app. Line 30 is untouched, so this PR moves exactly one variable and the measurement stays attributable. ## Why this one is not a cache enabler The script has two `clean` calls, and they cost different things. Only the second one wastes time on CI today: | Line | App | Cost on a fresh runner | Removed by | | --- | --- | --- | --- | | 30 | `CheckoutKitSwiftDemo` | None. Nothing exists yet to clean. | #637, with the cache | | 31 | `ShopifyAcceleratedCheckoutsApp` | A full recompile of the shared products line 30's app just built. | this PR | `xcode_run` passes neither `-derivedDataPath` nor `-workspace`. Both `run_app` calls therefore run from `platforms/swift/Samples` against one workspace and one default DerivedData. The Swift package and Apollo iOS 2.0.6 sit in both schemes' dependency graphs, so cleaning the second scheme deletes products the first scheme produced seconds earlier, in the same step. Line 30 is the enabler-only one: it deletes nothing today and only starts to matter once a restored cache is there to delete. That is why it is folded into the DerivedData PR rather than this one, as agreed. ## Baseline to beat Last 30 successful `main` runs of `ci.yml`, measured with a local `gh api` script. | Job | n | median | p90 | min | max | | --- | --- | --- | --- | --- | --- | | `Swift / build-and-test-samples` | 11 | 6m38s | 8m10s | 5m06s | 9m57s | | ↳ step `Run Tests` | 11 | 6m04s | — | — | — | `Run Tests` is 91% of the job. **How the gate works.** A step time only exists on `main`, so this merges first and is measured after. Re-measure the same step over 10 `main` runs, then append the delta to shop/issues-checkout-kit#1202. If the delta does not appear, revert this PR and stop the stack there — the later phases assume this premise holds. ## Verification - `shadowenv exec -- ./scripts/test_ruby` — green. - Cold run of the script itself. `dev swift test sample` runs `test_samples`, which never calls `build_and_test_samples`, so it does not cover this change. 🟡 The saving is read off the script, not timed locally. The mechanism is not in doubt, but the size of it is. If you want a number before merging rather than after, say so and I will time the second `run_app` with and without `clean` from the same warm state. ## Open question for the author `platforms/swift/Scripts/build_samples:24` carries the identical `clean build` double-compile, on both apps. It is deliberately untouched here and nowhere else in this stack: it is reachable only from `dev.yml:377,384`, so it costs no CI time, and changing it would add a second variable to the measurement. Say the word and it becomes a one-line follow-up, so the local path matches CI.
kieran-osgood-shopify
force-pushed
the
kieran-osgood/macos-ci-1-swift-duplicate-compile
branch
from
August 13, 2026 09:22
39ad804 to
d20ea5f
Compare
kieran-osgood-shopify
marked this pull request as ready for review
August 13, 2026 11:00
Install this buildOpen Tophat, select your target device, then click Install. Links open on the Mac running Tophat.
Checkout Kit E2E results
|
markmur
approved these changes
Aug 14, 2026
Contributor
Author
Merge activity
|
kieran-osgood-shopify
deleted the
kieran-osgood/macos-ci-1-swift-duplicate-compile
branch
August 14, 2026 12:34
kieran-osgood-shopify
added a commit
that referenced
this pull request
Aug 14, 2026
Part of the macOS CI speed-up. Tracked on shop/issues-checkout-kit#1210, under shop/issues-checkout-kit#1202. ## The two halves only work together `clean` deletes nothing on a fresh runner, so removing it alone changes nothing. A DerivedData cache saves nothing while a `clean` runs before every build. Paired here on purpose, and they report one delta. ## Change **Stop cleaning** - `platforms/react-native/sample/scripts/build_ios` — `xcodebuild build`, was `xcodebuild clean build`. - `platforms/swift/Scripts/build_and_test_samples:30` — `"build test"`, was `"clean build test"`. This is the line the first PR in this stack deliberately left alone, so that its own measurement moved one variable. On a fresh runner it deletes nothing, so it belongs here, beside the cache it would otherwise wipe. Merge order matters: the #632 delta has to be read before this lands, or the two changes share one number. **Start caching** Two `restore-cache@3` / `save-cache@1` pairs over `~/Library/Developer/Xcode/DerivedData`, one per macOS build workflow, keyed on the resolved dependency graph: | Workflow | Cache key inputs | | --- | --- | | `ci-ios-swift-samples` | `Package.resolved` | | `ci-ios-react-native-build-ios` | `sample/ios/Podfile.lock` + `Package.resolved` | Separate keys, because a shared key would have each job overwrite the other's products. Both jobs spend most of their time compiling dependencies that do not change between pull requests — Apollo iOS and the root Swift package for the Swift samples, the 89 React Native New Architecture C++ pods for the sample app. A pin change rebuilds rather than reuses. Xcode's incremental build owns correctness for the source that did change, which is its normal mode locally. Each `restore-cache@3` has a matching `save-cache@1`. A restore with no save warms nothing: the key never gets written, so every build pays full price while looking cached. Developer-only entry points such as `platforms/swift/Scripts/build_samples` keep their `clean`. A stale local DerivedData tree is a real failure mode there, and the rebuild costs nobody CI time. ## Measure the *second* run after merge A cold cache shows nothing. The number that matters is the median over 10 `main` runs once the key is populated. | Job | baseline median | baseline slowest step | | --- | --- | --- | | `React Native / Build iOS Sample` | 11m59s | `Build iOS sample` 10m27s | | `Swift / build-and-test-samples` | 6m38s | `Run Tests` 6m04s | ## Verification - `shadowenv exec -- ./scripts/test_ruby` — green. - `shadowenv exec -- bitrise validate --config=e2e/bitrise.yml` — valid. - Cold and warm local builds of the React Native sample with `CI=true`. ## 🔴 Unrelated local breakage found while verifying `pnpm sample build:ios` fails on a developer machine that has `sccache` on `PATH`: ``` error: unable to spawn process 'sccache clang' (No such file or directory) ``` `build_ios:8-18` exports `CC="sccache clang"`, and Xcode cannot spawn a two-word command as a compiler. CI is unaffected — the `CI = true` branch skips sccache entirely. The next PR in this stack touches these lines, so it is recorded on shop/issues-checkout-kit#1211 rather than fixed here. ## Decision made without you The plan scoped this to the React Native sample. It is applied to `ci-ios-swift-samples` too, because that job's `clean` has the same effect on a restored cache and leaving it in would make the new invariant untrue for half the macOS fleet.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of the macOS CI speed-up
Epic
Issue
Problem
build_and_test_samplesbuilds two apps in a single xcworkspace - they can share DerivedData.Both apps are built with
xcodebuild clean buildresulting in the second built app to discard the entire cache that was just built by the first appRemoving the second clean results in a faster compilation
Baseline to beat
Last 30 successful
mainruns ofci.yml, measured with a localgh apiscript.
Swift / build-and-test-samplesRun TestsTesting
We didn't see an improvement on walltime specifically, but we can observe from the logs that the second app did not recompile sources from the first app
Apollo, ApolloAPI, ShopifyCheckoutKit, ShopifyAcceleratedCheckouts and EmbeddedCheckoutProtocol.Before you merge
Important
platforms/swift/README.mdand/orplatforms/android/README.md)Releasing a new Swift version?
ShopifyCheckoutKit.podspecplatforms/swift/Sources/ShopifyCheckoutKit/ShopifyCheckoutKit.swiftplatforms/swift/README.md(major version only)Releasing a new Embedded Checkout Protocol version?
embeddedCheckoutProtocolAndroidinplatforms/android/gradle/libs.versions.tomlprotocol/languages/kotlin/embedded-checkout-protocol/api/embedded-checkout-protocol.apiif the public API changedReleasing a new Android version?
checkoutKitAndroidinplatforms/android/gradle/libs.versions.tomlplatforms/android/README.mdTip
See the Contributing documentation for the full release process per platform.