backport: bitcoin/bitcoin#34036: contrib: update macOS SDK to Xcode-26.1.1-17B100#7189
backport: bitcoin/bitcoin#34036: contrib: update macOS SDK to Xcode-26.1.1-17B100#7189PastaPastaPasta merged 8 commits intodashpay:developfrom
Conversation
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
|
Guix Automation has began to build this PR tagged as v23.1.0-devpr7189.112ae2dc. A new comment will be made when the image is pushed. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR updates macOS/Xcode and SDK-related build and CI configuration: bumps Xcode version and build ID from 16.0 (16A242d) to 26.1.1 (17B100); changes default SDK download URL from the project S3 bucket to https://bitcoincore.org/depends-sources/sdks and switches some archives from .tar.gz to .tar with updated sha256 sums; gen-sdk.py now produces a plain PAX tar and filters swiftmodule/modulemap files; several host makefiles and per-host CFLAGS/CXXFLAGS were adjusted or cleared; README examples and scripts updated accordingly. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 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.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@contrib/containers/guix/scripts/setup-sdk`:
- Around line 10-16: The XCODE_AR_HASH variable currently holds the SHA256 of
the original Xcode XIP, but setup uses the pre-built .tar.gz from the SDK_URL
and verifies that file against XCODE_AR_HASH; replace the current XCODE_AR_HASH
value with the actual sha256 checksum of the .tar.gz hosted at the SDK_URL
(compute the sha256sum of the .tar.gz served at depends-sources/sdks) so the
verification in setup-sdk succeeds—update the XCODE_AR_HASH constant with that
.tar.gz checksum.
ℹ️ 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
There was a problem hiding this comment.
🧹 Nitpick comments (1)
depends/hosts/darwin.mk (1)
16-17: Note: Changed from immediate (:=) to deferred (=) assignment.The
clang_progandclangxx_progvariables now use deferred assignment (=) instead of immediate assignment (:=). This means the shell command will be evaluated each time the variable is referenced rather than once at parse time. This is typically fine for tool lookups but could have minor performance implications if referenced repeatedly. Verify this matches the upstream change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@depends/hosts/darwin.mk` around lines 16 - 17, The change switched clang_prog and clangxx_prog from immediate assignment (:=) to deferred assignment (=), causing the shell lookup for clang/clang++ to run each time the variables are expanded; if this was accidental or causes repeated performance hits revert them back to immediate assignment (use := for clang_prog and clangxx_prog) so the command is executed once at parse time, otherwise add a comment near the clang_prog and clangxx_prog definitions documenting that the deferred assignment is intentional and why (e.g., dynamic PATH changes during the make run).
🤖 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`:
- Around line 16-17: The change switched clang_prog and clangxx_prog from
immediate assignment (:=) to deferred assignment (=), causing the shell lookup
for clang/clang++ to run each time the variables are expanded; if this was
accidental or causes repeated performance hits revert them back to immediate
assignment (use := for clang_prog and clangxx_prog) so the command is executed
once at parse time, otherwise add a comment near the clang_prog and clangxx_prog
definitions documenting that the deferred assignment is intentional and why
(e.g., dynamic PATH changes during the make run).
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
depends/funcs.mkdepends/hosts/darwin.mkdepends/hosts/freebsd.mkdepends/hosts/linux.mkdepends/hosts/mingw32.mkdepends/hosts/netbsd.mkdepends/hosts/openbsd.mk
✅ Files skipped from review due to trivial changes (1)
- depends/hosts/openbsd.mk
|
Guix Automation has failed with an unknown error for tag v23.1.0-devpr7189.112ae2dc |
|
Guix Automation has began to build this PR tagged as v23.1.0-devpr7189.ecdb9fa5. A new comment will be made when the image is pushed. |
|
Guix Automation has failed with an unknown error for tag v23.1.0-devpr7189.ecdb9fa5 |
a89e161 contrib: update macOS SDK to Xcode-26.1.1-17B100 (fanquake) 57a778e depends: use -Xclang -fno-cxx-modules in macOS cross build (fanquake) Pull request description: Updates the macOS SDK used for Guix builds to `Xcode-26.1.1-17B100`. Closes bitcoin#34034. ACKs for top commit: hebasto: ACK a89e161. sedited: ACK a89e161 janb84: concept ACK a89e161 Tree-SHA512: 4f8f9afee6fca594a0d30fbb3c150f5ed120b40f707954678ff69951bc806acc154aed4b5986d8642160f7b37523933c87c5734f296ff881555093188e29549e
…x determinism (across distros) 3e01b5d contrib: rename gen-sdk to gen-sdk.py (fanquake) c1213a3 macdeploy: disable compression in macOS gen-sdk script (fanquake) a33d034 contrib: more selectively pick files for macOS SDK (fanquake) Pull request description: This includes three changes. The first is to more selectively pick files for inclusion into our macOS SDK tarball (skip manpages, binaries etc), which is nice because it redues the size of the tarball (from ~80mb to 20mb), and makes the size increase that happens with the next commit, less-bad. The second change removes compression of the tarball. Starting with Python 3.11, Pythons gzip might delegate to zlib. Depending on the OS, i.e Ubuntu vs Fedora, the underlying zlib implementation might differ, resulting in different output. For now, or until a better solution exists, remove compression. This results in the SDK increasing in size to ~157mb. Which is not unreasonable, to regain determinism (and would be significantly worse without the previous commit). See: https://docs.python.org/3/library/gzip.html#gzip.compress The third renames `gen-sdk` to `gen-sdk.py`, so that it will be linted, along with the rest of our Python files. Fixes bitcoin#31873. We could probably also put this into 30.x. ACKs for top commit: stickies-v: ACK 3e01b5d modulo the new .tar SDK being uploaded davidgumberg: Tested ACK 3e01b5d Tree-SHA512: 272164a98e0e6f10822870162c1b3a405693c2f64d3ed085a2d2243a48641d940704b5ef6022256915ac9cf383e87a4f8d4dc2ec4eaa9d25e2abd30f5498778b
…han setting them per-host 9954d6c depends: hard-code necessary c(xx)flags rather than setting them per-host (Cory Fields) Pull request description: The per-host flag variables hold platform-specific defaults that are ignored when flag environment variables are set, so it was wrong for them to contain -std options relied on by package definitions. Additionally, these flags (-pipe and -std=xx) will no longer be passed into the CMake build, meaning less duplication in the build summary. Pulled out of bitcoin#31920. ACKs for top commit: achow101: ACK 9954d6c ryanofsky: Code review ACK 9954d6c. No changes since last review other than improving the commit message. Change overall makes sense because it deduplicates host definitions, stops dropping `-std` flags from package builds when custom CFLAGS/CXXFLAGS environment variables are set, and stops passing duplicate flags to cmake that have no effect. theuni: ACK 9954d6c Tree-SHA512: 62a2a21b741893cf708ca71fb5f0626c30d52685c845f9016f428a5e38fc8515acd4bf2c83635d6505b63830d1c296472026ec3d341c8069f1e490a991b6b5ef
|
Guix Automation has began to build this PR tagged as v23.1.0-devpr7189.2c779e34. A new comment will be made when the image is pushed. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@depends/hosts/darwin.mk`:
- Around line 60-67: Update the two TODO comment lines that currently read
"partiall reverted" to the correct spelling "partially reverted" (these are the
TODO comments surrounding the darwin_CC definition and the linked PR
references). Edit the comment text in the same lines so both instances of
"partiall" become "partially" while leaving the rest of the comment and the
darwin_CC variable untouched.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
ci/test/00_setup_env_mac.shcontrib/containers/guix/scripts/setup-sdkcontrib/guix/README.mdcontrib/macdeploy/README.mdcontrib/macdeploy/gen-sdk.pydepends/funcs.mkdepends/hosts/darwin.mkdepends/hosts/freebsd.mkdepends/hosts/linux.mkdepends/hosts/mingw32.mkdepends/hosts/netbsd.mkdepends/hosts/openbsd.mk
🚧 Files skipped from review as they are similar to previous changes (9)
- depends/hosts/netbsd.mk
- contrib/guix/README.md
- depends/hosts/linux.mk
- contrib/containers/guix/scripts/setup-sdk
- depends/hosts/openbsd.mk
- depends/hosts/freebsd.mk
- contrib/macdeploy/README.md
- depends/hosts/mingw32.mk
- depends/funcs.mk
|
Guix Automation has failed with an unknown error for tag v23.1.0-devpr7189.2c779e34 |
|
I don't know why CI fails, but there are my hashes: |
|
✅ Guix build complete! Release: https://github.com/dashpay/dash-dev-branches/releases/tag/v23.1.0-devpr7189.2c779e34 Checksums for 2c779e3 |
thephez
left a comment
There was a problem hiding this comment.
Built successfully for me 👍
f16158a56258968555c3ed025adadbc23550e86de40ea7efdb637c147cf8d3c9 dashcore-23.1.0-155-g2c779e34aa38-aarch64-linux-gnu.tar.gz
170680f11dd25b50bfae2772e617e81888086338597d2f59e05d0a8f601db64b dashcore-23.1.0-155-g2c779e34aa38-arm64-apple-darwin-unsigned.tar.gz
6011bc181b97326e936a658716e91b4884a9a19affceec90c6c9ac3d96f68089 dashcore-23.1.0-155-g2c779e34aa38-arm64-apple-darwin-unsigned.zip
be7048b60fd1d30a8c04d3e03caf371d99fde7cb66415a847994efc8b7d9884f dashcore-23.1.0-155-g2c779e34aa38-arm64-apple-darwin.tar.gz
dd5e3efa5bba03a4f0335422c6ebbfba14a83b02263389bf63885453efac3b59 dashcore-23.1.0-155-g2c779e34aa38.tar.gz
094a4cc7eae294ba25c381c7deefd247cf786f7a17c4f8114cacf7ad337e441c dashcore-23.1.0-155-g2c779e34aa38-riscv64-linux-gnu-debug.tar.gz
3932ce0a6c0f32702fa4ed14068e3427511723675eaf59131e2bd572126c7b33 dashcore-23.1.0-155-g2c779e34aa38-riscv64-linux-gnu.tar.gz
c49c2221e24ada5a6e592d2fe92be219b3ffe5e573c8bfa0216853cf204a650c dashcore-23.1.0-155-g2c779e34aa38-x86_64-apple-darwin-unsigned.tar.gz
e480bee1040e9eaa4c3d015563509cc6c7a03588167e31433bbbfd9763d37354 dashcore-23.1.0-155-g2c779e34aa38-x86_64-apple-darwin-unsigned.zip
ab9e9489d56ffa17098df569477a48f9e118b06c3cc99741cca3abbfea76932f dashcore-23.1.0-155-g2c779e34aa38-x86_64-apple-darwin.tar.gz
7b321727d17f783b04712bfb600229b9f4031127a1b1f98862ecbff660f27bb8 dashcore-23.1.0-155-g2c779e34aa38-x86_64-linux-gnu-debug.tar.gz
bf67c8fcd658a0cb82131b921889613ab76d98bd95fd33478d14cfc31fc2a068 dashcore-23.1.0-155-g2c779e34aa38-x86_64-linux-gnu.tar.gz
af8cf3521009ba0533cb4e8753c75af47364876cd088491e18b9727845045c61 dashcore-23.1.0-155-g2c779e34aa38-win64-debug.zip
80ed42b4ac110310a3f07823c9e469cab6a4afb45467eb1e5cd5cc9bbf7a52f8 dashcore-23.1.0-155-g2c779e34aa38-win64-setup-unsigned.exe
81bdff5132f1fdc8edc8c471a00aace677333e641a3eca967f5c8c7ee147520f dashcore-23.1.0-155-g2c779e34aa38-win64-unsigned.tar.gz
c24ab9de5274e43e3382260f202ac1fe11edd68332d94b81236760d297258143 dashcore-23.1.0-155-g2c779e34aa38-win64.zip
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: