build: target aarch64 over armv7, update to macOS 15.0 SDK (Xcode 16.0), add verification step to setup-sdk#7184
Conversation
Since the container image could be run on different architectures we can't implicitly assume that the x86_64 compiler is available, it needs to be made explicit. Additionally, we stopped releasing ARMv7 (32-bit) binaries a long time ago, we should test against ARMv8 (64-bit) instead.
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
GitHub Actions buildhttps://github.com/kwvg/dash/actions/runs/22501928993 Checksums for ca6c6f5 |
|
Guix Automation has began to build this PR tagged as v23.1.0-devpr7184.ca6c6f5f. A new comment will be made when the image is pushed. |
WalkthroughThis pull request migrates CI and build targets from 32-bit ARM (arm-linux-gnueabihf / arm-linux) to 64-bit ARM (aarch64-linux-gnu / aarch64-linux) across GitHub Actions workflows, CI scripts, Dockerfiles, and container setup. It updates macOS/Xcode versions (15.0→16.0) and associated SDK identifiers and checksums, switches default SDK URLs to a Dash S3 bucket, and adds SHA-256 verification for the macOS SDK download. Darwin makefiles gain clang resource-dir awareness and adjusted include handling. Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
contrib/guix/README.md (1)
225-227:⚠️ Potential issue | 🟡 MinorUpdate default
HOSTSdocumentation to remove ARMv7.The documented default list still includes
arm-linux-gnueabihf, which conflicts with this PR’s architecture migration to aarch64-only for ARM Linux targets.Suggested doc fix
- _(defaults to "x86\_64-linux-gnu arm-linux-gnueabihf aarch64-linux-gnu + _(defaults to "x86\_64-linux-gnu aarch64-linux-gnu powerpc64-linux-gnu powerpc64le-linux-gnu x86\_64-w64-mingw32 x86\_64-apple-darwin arm64-apple-darwin")_🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contrib/guix/README.md` around lines 225 - 227, Update the README default HOSTS list to remove the ARMv7 entry "arm-linux-gnueabihf": locate the parentheses containing the defaults string (the line showing _(defaults to "x86_64-linux-gnu arm-linux-gnueabihf aarch64-linux-gnu ...")_) and delete the "arm-linux-gnueabihf" token so the documented defaults reflect the new aarch64-only ARM targets.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@contrib/guix/README.md`:
- Around line 225-227: Update the README default HOSTS list to remove the ARMv7
entry "arm-linux-gnueabihf": locate the parentheses containing the defaults
string (the line showing _(defaults to "x86_64-linux-gnu arm-linux-gnueabihf
aarch64-linux-gnu ...")_) and delete the "arm-linux-gnueabihf" token so the
documented defaults reflect the new aarch64-only ARM targets.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.github/workflows/build.yml.github/workflows/guix-build.ymlci/dash/matrix.shci/test/00_setup_env.shci/test/00_setup_env_aarch64.shci/test/00_setup_env_mac.shcontrib/containers/ci/ci.Dockerfilecontrib/containers/deploy/Dockerfile.GitHubActions.Dispatchcontrib/containers/guix/scripts/setup-sdkcontrib/guix/README.mdcontrib/guix/symbol-check.pycontrib/macdeploy/README.mddepends/hosts/darwin.mk
|
Guix Automation has failed with an unknown error for tag v23.1.0-devpr7184.ca6c6f5f |
…olution This is an issue with more recent SDKs (observed from Xcode 15.3 15E204a shipped SDKs onwards) and explicit inclusion resolves it. We have to use `:=` to force immediate resolution.
|
Guix Automation has began to build this PR tagged as v23.1.0-devpr7184.1f64bad3. A new comment will be made when the image is pushed. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
depends/hosts/darwin.mk (1)
36-36: Stale comment references-nostdlibincbut code now uses-nostdinc.The comment on line 36 still mentions
-nostdlibinc, but the actual flags on lines 57 and 62 now use-nostdinc(and-nostdinc++). Consider updating the comment to reflect the current implementation.📝 Suggested comment update
-# -isysroot$(OSX_SDK) -nostdlibinc +# -isysroot$(OSX_SDK) -nostdinc / -nostdinc++🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@depends/hosts/darwin.mk` at line 36, The commented-out example still mentions the old flag "-nostdlibinc" while the actual makefile now uses "-nostdinc" and "-nostdinc++"; update the comment text "# -isysroot$(OSX_SDK) -nostdlibinc" to reference the current flags (e.g., "-nostdinc" or "-nostdinc++") so the comment matches the real flags used elsewhere in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@depends/hosts/darwin.mk`:
- Line 36: The commented-out example still mentions the old flag "-nostdlibinc"
while the actual makefile now uses "-nostdinc" and "-nostdinc++"; update the
comment text "# -isysroot$(OSX_SDK) -nostdlibinc" to reference the current
flags (e.g., "-nostdinc" or "-nostdinc++") so the comment matches the real flags
used elsewhere in the file.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
ci/test/00_setup_env.shci/test/00_setup_env_mac.shcontrib/containers/guix/scripts/setup-sdkcontrib/guix/README.mdcontrib/guix/symbol-check.pycontrib/macdeploy/README.mddepends/hosts/darwin.mk
🚧 Files skipped from review as they are similar to previous changes (5)
- contrib/containers/guix/scripts/setup-sdk
- contrib/guix/symbol-check.py
- ci/test/00_setup_env.sh
- contrib/macdeploy/README.md
- ci/test/00_setup_env_mac.sh
|
Guix Automation has failed with an unknown error for tag v23.1.0-devpr7184.1f64bad3 |
| export BASE_BUILD_DIR=${BASE_BUILD_DIR:-$BASE_SCRATCH_DIR/build-ci} | ||
| export PREVIOUS_RELEASES_DIR=${PREVIOUS_RELEASES_DIR:-$BASE_ROOT_DIR/releases/$HOST} | ||
| export SDK_URL=${SDK_URL:-https://bitcoincore.org/depends-sources/sdks} | ||
| export SDK_URL=${SDK_URL:-https://s3.us-west-2.amazonaws.com/dash-depends-sources} |
There was a problem hiding this comment.
NACK. Who is owner of this file? What is a procedure to update version? where are docs for it? @ktechmidas are you aware of this file?
…ode-26.1.1-17B100 2c779e3 docs: TODO to reapply reverted part of bitcoin#30451 (Konstantin Akimov) 63e9f7c Merge bitcoin#32584: depends: hard-code necessary c(xx)flags rather than setting them per-host (Ava Chow) e4ad957 fix: minimum sdk & minumum xcode are not the same (Konstantin Akimov) 3dc4624 fix: hash of archive and extra debug output (Konstantin Akimov) 153959e Merge bitcoin#32009: contrib: turn off compression of macOS SDK to fix determinism (across distros) (merge-script) b68d7a3 Merge bitcoin#34036: contrib: update macOS SDK to Xcode-26.1.1-17B100 (merge-script) eda7eb8 Revert "fix(build): include resource directory in search path for include resolution" (Konstantin Akimov) 4fcc7b4 Revert "build: update to macOS 15.0 SDK from Xcode 16.0" (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented #7184 introduces a new product to maintain and take care, but without docs, without owner, without a processes to update it to new version. Particularly, this file: `export SDK_URL=${SDK_URL:-https://s3.us-west-2.amazonaws.com/dash-depends-sources}` ## What was done? Partially reverted #7184 and backported 34036 instead. ## How Has This Been Tested? See CI run ## Breaking Changes Xcode 26.1 is used instead Xcode 16. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 2c779e3 kwvg: utACK 2c779e3 Tree-SHA512: 155699f6a293d989c17d20ce1114a72e47adf06ea59db570ba03b89d81ca9fc3957aaba685f505db54fb8cab4dc706826adaa2328d838a598e8671233ea300b5
Additional Information
Dependency for build: add rudimentary support for Rust components in build system, crate vendoring, patch loader paths to Guix store locations #7109
To ensure our list of targets validated are matched with targets used for releases (see
guix-start), we switch outarm-linux-gnueabihfwithaarch64-linux-gnu, this is now reflected by changes to our GitHub CI configuration, build scripts, Guix CI action.As mentioned in dash#6927, we need to update the macOS SDK to ensure we have standards-conformant definitions to deal with dtolnay/cxx#1574.
SDKs shipped after Xcode 15.3 (15E204a) seem to have problems with Clang not able to determine the include paths correctly (see below for error). To resolve this issue, we now explicitly fetch the resource directory and add it to the includes list.
Build log:
Due to conflicting definitions between the native and cross toolchains,
C{,PLUS}_INCLUDE_PATHneeds to be unset to allow the cross toolchain's definitions to prevail (see build error below).Build log:
Breaking Changes
None expected.
Checklist