feat(cua-driver): add Intel Mac (x86_64) support via cross-compilation - #1469
feat(cua-driver): add Intel Mac (x86_64) support via cross-compilation#1469r33drichards wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds cross-compilation support to the Swift CUA driver release pipeline. The build script now detects native architecture and respects an ChangesCross-Architecture Build and Release
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/cd-swift-cua-driver.yml:
- Around line 175-176: The workflow overwrites .release/CuaDriver.app when
cross-compiling x86_64 (ARCH=x86_64) which later causes the bare-binary
packaging to pick up the wrong-arch app using native `uname -m`; update the
build/packaging steps so each arch writes an arch-suffixed app directory (e.g.
.release/CuaDriver-x86_64.app and .release/CuaDriver-arm64.app) or make the
bare-binary step explicitly point to the correct arched app (e.g. use
.release/CuaDriver-arm64.app when packaging the native runner) instead of
relying on the unsuffixed .release/CuaDriver.app; adjust calls around
build-release-notarized.sh, the ARCH variable usage, and the bare-binary
packaging invocation to reference the suffixed app name.
In `@libs/cua-driver/scripts/build/build-release-notarized.sh`:
- Around line 69-73: The script runs swift build for ARCH-specific and native
builds without failing immediately on build errors or missing artifacts; update
the blocks that invoke swift build (the conditional using ARCH and NATIVE_ARCH
and the later similar block at 83-91) to check the exit status of the swift
build command and verify that the expected $BINARY_PATH exists and is non-empty
right after each build, and if either check fails call echo with a descriptive
error mentioning the ARCH and exit with a non-zero status so the script
hard-fails instead of producing a broken payload.
- Around line 236-237: The cleanup step currently uses a wildcard that removes
artifacts from other-architecture runs; modify the cleanup to only remove
archives for the current architecture by using the OS_IDENTIFIER (set to
"darwin-${ARCH}") or ARCH variable when matching archive filenames in
build-release-notarized.sh so each invocation only deletes its own artifacts
(scope the glob/pattern to ${OS_IDENTIFIER} or ${ARCH} instead of a broad
wildcard).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 416be182-e536-4c52-ae36-2818da3dbc88
📒 Files selected for processing (2)
.github/workflows/cd-swift-cua-driver.ymllibs/cua-driver/scripts/build/build-release-notarized.sh
| # Cross-compile for Intel Mac (x86_64) on this arm64 runner | ||
| ARCH=x86_64 LOG_LEVEL=minimal ./build-release-notarized.sh |
There was a problem hiding this comment.
Second pass overwrites .release/CuaDriver.app, which can mislabel the bare-binary artifact.
After Line 176, .release/CuaDriver.app is x86_64, but the later bare-binary step names output using native uname -m (arm64 on this runner). That can publish a wrongly labeled binary tarball.
One practical approach
LOG_LEVEL=minimal ./build-release-notarized.sh
+cp -R .release/CuaDriver.app .release/CuaDriver-arm64.app
# Cross-compile for Intel Mac (x86_64) on this arm64 runner
ARCH=x86_64 LOG_LEVEL=minimal ./build-release-notarized.shThen point the bare-binary packaging step to .release/CuaDriver-arm64.app/... for the native artifact (or produce explicitly arch-suffixed bare binaries from each app).
🤖 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 @.github/workflows/cd-swift-cua-driver.yml around lines 175 - 176, The
workflow overwrites .release/CuaDriver.app when cross-compiling x86_64
(ARCH=x86_64) which later causes the bare-binary packaging to pick up the
wrong-arch app using native `uname -m`; update the build/packaging steps so each
arch writes an arch-suffixed app directory (e.g. .release/CuaDriver-x86_64.app
and .release/CuaDriver-arm64.app) or make the bare-binary step explicitly point
to the correct arched app (e.g. use .release/CuaDriver-arm64.app when packaging
the native runner) instead of relying on the unsuffixed .release/CuaDriver.app;
adjust calls around build-release-notarized.sh, the ARCH variable usage, and the
bare-binary packaging invocation to reference the suffixed app name.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| if [ "$ARCH" != "$NATIVE_ARCH" ]; then | ||
| swift build -c release --product cua-driver --arch "$ARCH" > /dev/null | ||
| else | ||
| swift build -c release --product cua-driver > /dev/null | ||
| fi |
There was a problem hiding this comment.
Fail fast if ARCH-specific build output is missing.
Right now, a failed swift build or missing $BINARY_PATH won’t stop the script immediately, which can produce a signed/packageable but broken app payload.
Suggested hardening
log "essential" "Building release version (arch: $ARCH)..."
if [ "$ARCH" != "$NATIVE_ARCH" ]; then
- swift build -c release --product cua-driver --arch "$ARCH" > /dev/null
+ if ! swift build -c release --product cua-driver --arch "$ARCH" > /dev/null; then
+ log "error" "swift build failed for ARCH=$ARCH"
+ exit 1
+ fi
else
- swift build -c release --product cua-driver > /dev/null
+ if ! swift build -c release --product cua-driver > /dev/null; then
+ log "error" "swift build failed for native ARCH=$NATIVE_ARCH"
+ exit 1
+ fi
fi
@@
if [ "$ARCH" != "$NATIVE_ARCH" ]; then
BINARY_PATH=".build/${ARCH}-apple-macosx/release/cua-driver"
else
BINARY_PATH=".build/release/cua-driver"
fi
+if [ ! -f "$BINARY_PATH" ]; then
+ log "error" "Expected binary not found at $BINARY_PATH"
+ exit 1
+fi
cp -f "$BINARY_PATH" "$APP_BUNDLE/Contents/MacOS/cua-driver"Also applies to: 83-91
🤖 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 `@libs/cua-driver/scripts/build/build-release-notarized.sh` around lines 69 -
73, The script runs swift build for ARCH-specific and native builds without
failing immediately on build errors or missing artifacts; update the blocks that
invoke swift build (the conditional using ARCH and NATIVE_ARCH and the later
similar block at 83-91) to check the exit status of the swift build command and
verify that the expected $BINARY_PATH exists and is non-empty right after each
build, and if either check fails call echo with a descriptive error mentioning
the ARCH and exit with a non-zero status so the script hard-fails instead of
producing a broken payload.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| # ARCH is already set above (env override or native); derive OS identifier | ||
| OS_IDENTIFIER="darwin-${ARCH}" |
There was a problem hiding this comment.
Scope archive cleanup to the current architecture.
With dual invocations, the wildcard cleanup on Line 244 deletes artifacts from the first pass (native) before the x86_64 pass finishes, so native upload paths can disappear.
Suggested fix
-# Clean up any existing artifacts first to avoid conflicts
-rm -f cua-driver-*.tar.gz cua-driver-*.pkg.tar.gz
+# Clean up only this arch's artifacts to keep outputs from other arch passes
+rm -f "cua-driver-${VERSION}-${OS_IDENTIFIER}.tar.gz" \
+ "cua-driver-${VERSION}-${OS_IDENTIFIER}.pkg.tar.gz"🤖 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 `@libs/cua-driver/scripts/build/build-release-notarized.sh` around lines 236 -
237, The cleanup step currently uses a wildcard that removes artifacts from
other-architecture runs; modify the cleanup to only remove archives for the
current architecture by using the OS_IDENTIFIER (set to "darwin-${ARCH}") or
ARCH variable when matching archive filenames in build-release-notarized.sh so
each invocation only deletes its own artifacts (scope the glob/pattern to
${OS_IDENTIFIER} or ${ARCH} instead of a broad wildcard).
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Builds unsigned darwin-arm64 and darwin-x86_64 binaries on every PR touching libs/cua-driver, then posts download links as a PR comment. This lets Intel Mac users test the binary before the PR is merged without requiring Apple notarization credentials. Binaries are unsigned/unnotarized — users need: xattr -d com.apple.quarantine ./cua-driver
🔧 Pre-release binaries (unsigned)Built from commit dfc590c · CI run
|
Three bugs flagged by CodeRabbit: 1. (Critical) Wildcard cleanup clobbers arm64 artifacts on x86_64 pass: rm -f cua-driver-*.tar.gz wiped the arm64 tarballs before the x86_64 pass finished. Fixed by scoping the rm to the current arch's files only. 2. (Critical) swift build failures silently produced broken app bundles: Added 'if ! swift build ...' guards and binary path existence check so a failed build exits immediately instead of packaging a broken payload. 3. (Major) Bare-binary step used uname -m (always arm64 on runner) but CuaDriver.app was x86_64 after the second build pass, causing a mislabeled bare binary. Fixed by: - Writing arch-suffixed bundles: CuaDriver-arm64.app / CuaDriver-x86_64.app - Packaging each via a temporary CuaDriver.app symlink (install.sh compat) - Iterating both arches explicitly in the bare-binary packaging step
|
this is pending testing on a physical intel macos device |
| # Cross-compile for Intel Mac (x86_64) on this arm64 runner | ||
| ARCH=x86_64 LOG_LEVEL=minimal ./build-release-notarized.sh |
| if [ "$ARCH" != "$NATIVE_ARCH" ]; then | ||
| swift build -c release --product cua-driver --arch "$ARCH" > /dev/null | ||
| else | ||
| swift build -c release --product cua-driver > /dev/null | ||
| fi |
| # ARCH is already set above (env override or native); derive OS identifier | ||
| OS_IDENTIFIER="darwin-${ARCH}" |
|
Thanks for tackling this! Closing as superseded by #1490 (merged), which added Intel Mac support via a different mechanism — matrix builds across If you want to verify, grab the latest cua-driver release and Going forward, we're consolidating on the Rust port ( |
3 participants
Summary
Fixes Intel Mac users being unable to install
cua-driverviainstall.sh. The installer already detectsuname -mand constructscua-driver-{VERSION}-darwin-{ARCH}.tar.gz— thedarwin-x86_64artifact just never existed because the CD pipeline only ran on an Apple Silicon (macos-15) runner.Changes
libs/cua-driver/scripts/build/build-release-notarized.shARCH=$(uname -m)→ARCH=${ARCH:-$(uname -m)}— allows arch override via env var--arch $ARCHwhen cross-compiling (skipped for native builds to preserve current behavior).build/x86_64-apple-macosx/release/cua-driverinstead of the.build/release/symlink.github/workflows/cd-swift-cua-driver.ymlARCH=x86_64 LOG_LEVEL=minimal ./build-release-notarized.shdarwin-x86_64artifacts to theCreate ReleasestepResult
Each release now ships both
cua-driver-{VERSION}-darwin-arm64.tar.gzandcua-driver-{VERSION}-darwin-x86_64.tar.gz.install.shrequires no changes — it already picks the right one based onuname -m.Source code changes: none. The Swift codebase is fully architecture-neutral; all frameworks used (AppKit, CoreGraphics, ScreenCaptureKit, SkyLight) are available on Intel macOS 14+.
Closes linear issue CUA-504.
Summary by CodeRabbit