-
Notifications
You must be signed in to change notification settings - Fork 1.2k
backport: trivial commits #6677
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe updates include several targeted changes across build scripts, patches, and test files. In the symbol checking script, a version check was updated to require a newer tool version. The Guix build environment was adjusted by pinning to a new commit hash and introducing a helper function to apply a reproducibility patch to the MinGW winpthreads package, with corresponding updates to the cross-toolchain creation logic. Two patches were added or modified to the GCC and winpthreads Makefiles to append 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
contrib/devtools/symbol-check.py (1)
254-257: Necessary version update for MACHO tool validationThis update to the expected LLVM linker (LLD) version check makes sense as part of keeping the toolchain requirements current.
Consider simplifying this function by returning the condition directly:
def check_MACHO_lld(binary) -> bool: - if binary.build_version.tools[0].version == [18, 1, 8]: - return True - return False + return binary.build_version.tools[0].version == [18, 1, 8]🧰 Tools
🪛 Ruff (0.11.9)
255-257: Return the condition directly
Inline condition
(SIM103)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (6)
contrib/devtools/symbol-check.py(1 hunks)contrib/guix/libexec/prelude.bash(1 hunks)contrib/guix/manifest.scm(1 hunks)contrib/guix/patches/gcc-remap-guix-store.patch(1 hunks)contrib/guix/patches/winpthreads-remap-guix-store.patch(1 hunks)src/wallet/test/coinselector_tests.cpp(0 hunks)
💤 Files with no reviewable changes (1)
- src/wallet/test/coinselector_tests.cpp
🧰 Additional context used
🪛 Ruff (0.11.9)
contrib/devtools/symbol-check.py
255-257: Return the condition directly
Inline condition
(SIM103)
🔇 Additional comments (5)
contrib/guix/libexec/prelude.bash (1)
54-54: Update to newer Guix version looks goodThis change updates the pinned Guix commit hash to a newer version, supporting the PR's goal of improving build reproducibility.
contrib/guix/patches/winpthreads-remap-guix-store.patch (1)
1-18: Excellent reproducibility enhancement for debug symbolsThis patch improves build reproducibility by ensuring debug symbols don't contain architecture-specific Guix store paths. Instead, all paths under
/gnu/storeare remapped to/usrusing the-ffile-prefix-mapcompiler flag.The approach is well-documented in the patch header and consistent with similar changes for other components (like the GCC patch).
contrib/guix/patches/gcc-remap-guix-store.patch (1)
1-21: Excellent reproducibility enhancement for libgccThis patch correctly implements path remapping for libgcc debug symbols, ensuring that architecture-specific Guix store paths don't affect build reproducibility. The implementation is clean and mirrors the approach used in the winpthreads patch.
The consistent approach across multiple components demonstrates good systematic thinking about the build reproducibility problem.
contrib/guix/manifest.scm (2)
131-133: Well-structured helper function additionThe new
winpthreads-patchesfunction follows the same pattern as the existing patch helper functions, providing a clean way to apply the reproducibility patch to the MinGW winpthreads package.
139-141: Good integration with existing codeThis change properly wraps the
make-mingw-w64call with the newly definedwinpthreads-patchesfunction, maintaining the original functionality while adding the reproducibility improvement. This approach improves build reproducibility by ensuring debug symbols don't embed architecture-dependent Guix store paths.
UdjinM6
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fb1c6c1 test: Remove redundant test (yancy) Pull request description: I can't think of any reason to keep this test case around labeled [fix me](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L242). The test was originally added [here](bitcoin@4566ab7) however there was never an assertion about the coins that should be selected, only that a solution is found (which is a redundant solution to the test [above](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L222)). The comment was later added here to [fix](bitcoin@3842732) it, however it's unclear what exactly it's testing. A test was later added [here](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L366) where if the [long term fee](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L357) is less than the current [fee](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L356), then select fewer UTXOs, which may have been the original intent. ACKs for top commit: S3RK: ACK fb1c6c1 Zero-1729: Concept ACK fb1c6c1 achow101: ACK fb1c6c1 Tree-SHA512: bce2cdae669c144ffaa130237a1643e3b6728e13d603cebf5d9493c4c7c68b3635868e4d93d210783c2ded2a871f185ca09a2053168c05b26a1e056ff6edf68f
…f23802cfe75a737963c 6ee000e guix: bump time-machine to efc26826400762207cde9f23802cfe75a737963c (fanquake) cbeb2c2 guix: patch /gnu/store paths out of winpthreads (fanquake) Pull request description: Needed for bitcoin#30210. This doesn't switch runtimes, because upstream is still configured to use the old runtime. See: https://git.savannah.gnu.org/cgit/guix.git/commit/?id=17188be0f723e00377b21b767f5447d7938a116e. git-mimimal `2.45.1` -> `2.45.2` Kernel Headers `6.1.92` -> `6.1.100` LLVM `18.1.6` -> `18.1.8` mingw-w64 `11.0.1` -> `12.0.0` NSIS `3.09` -> `3.10` patch `2.7.6` -> `2.7.6-0.f144b35` ACKs for top commit: TheCharlatan: ACK 6ee000e Tree-SHA512: f4f99d16dd8cab5b2b7c5d82111af86de20e1669cc3b4054d72ab4a67b2956757df170f0df28c96d18653c1c9d2ebdd0ecc441005726a20cd963d98513b4a851
…f93ae88fbef25427252 eca20be guix: bump time-machine to 7bf1d7aeaffba15c4f680f93ae88fbef25427252 (fanquake) Pull request description: Includes: https://git.savannah.gnu.org/cgit/guix.git/commit/?id=d428237642e1e4ac8fda4597205ffec89926c0ec. which removes the need to build Python2, and OpenSSL `1.x` (which has historically caused issues) when building for Windows (Python2 (with a dependency on OpenSSL `1.x`) used to be a dependency of NSIS). Linux Kernel Headers `6.1.100` -> `6.1.102` ```bash d079858fb1bc526217ee06f312d97a56c34986440e5f9e108af66eaecacea073 guix-build-eca20bead2da/output/aarch64-linux-gnu/SHA256SUMS.part 2db780ffe39210a3ba113e52362d94840449218ac1747e3a3484606cc36acead guix-build-eca20bead2da/output/aarch64-linux-gnu/bitcoin-eca20bead2da-aarch64-linux-gnu-debug.tar.gz b56b602bd87e73b11a6b68147c52c6dfa53f0ec4bac52ac749765025e7b43bc9 guix-build-eca20bead2da/output/aarch64-linux-gnu/bitcoin-eca20bead2da-aarch64-linux-gnu.tar.gz d56a9a6ac683da2e347d2ea71fab0cd54a126604ac1c9cc4d8fa89f6343ddb52 guix-build-eca20bead2da/output/arm-linux-gnueabihf/SHA256SUMS.part 6555b6d837605b35c5cf72ab4e5728d9205f8481e3e4bc9e1bbe44e09a1aa1a3 guix-build-eca20bead2da/output/arm-linux-gnueabihf/bitcoin-eca20bead2da-arm-linux-gnueabihf-debug.tar.gz d310e9a532d035db238552ad3fa3779b93d062e655f8e477ff029682af4f7cf4 guix-build-eca20bead2da/output/arm-linux-gnueabihf/bitcoin-eca20bead2da-arm-linux-gnueabihf.tar.gz eab39d953890e2d36b92b53e43a711ab615090d5b9030861229386e4a41344d8 guix-build-eca20bead2da/output/arm64-apple-darwin/SHA256SUMS.part 394473362a4d4895431d9250702e9e87f8ccb880a2b723eeb0cfd8eed1635518 guix-build-eca20bead2da/output/arm64-apple-darwin/bitcoin-eca20bead2da-arm64-apple-darwin-unsigned.tar.gz 3e72306a34e69647200fe5b3d5e8da3bc3b75105d3eeb9f5d5b5332de5d8e464 guix-build-eca20bead2da/output/arm64-apple-darwin/bitcoin-eca20bead2da-arm64-apple-darwin-unsigned.zip 1b0515ab24a57706278bde37b88a0dc51188373154876997dcbf61f2c9cf2c65 guix-build-eca20bead2da/output/arm64-apple-darwin/bitcoin-eca20bead2da-arm64-apple-darwin.tar.gz fba7470833ff076c01de5f2ed4a4d697f8a2e06cec49098f89d40e84c4798357 guix-build-eca20bead2da/output/dist-archive/bitcoin-eca20bead2da.tar.gz 935cc2ebec9b595da67bcde22a395060817fdf16b3a31f14e6b2252cb5073640 guix-build-eca20bead2da/output/powerpc64-linux-gnu/SHA256SUMS.part 85ee64aa6d0ab6d3431b7c0af3e26a9bfdb365343ba1b5198e321ae0f6778d33 guix-build-eca20bead2da/output/powerpc64-linux-gnu/bitcoin-eca20bead2da-powerpc64-linux-gnu-debug.tar.gz 52a1a8e8fdb48589cfb00c35001ae8765f6127d472d11ad03c3faa3621e45032 guix-build-eca20bead2da/output/powerpc64-linux-gnu/bitcoin-eca20bead2da-powerpc64-linux-gnu.tar.gz 72ccb2577dd11342dfce124346d359b19d8bc4af12cd445447e0568321dd39b9 guix-build-eca20bead2da/output/riscv64-linux-gnu/SHA256SUMS.part 80a0d80c3adb8e2de27605ed0b2bd7f5442c8316397a53a3e0e840a14587b057 guix-build-eca20bead2da/output/riscv64-linux-gnu/bitcoin-eca20bead2da-riscv64-linux-gnu-debug.tar.gz 62de93f8defc9e561caed1586bee20e208be1d66cdc8bf593f5e09e3c28d03a6 guix-build-eca20bead2da/output/riscv64-linux-gnu/bitcoin-eca20bead2da-riscv64-linux-gnu.tar.gz e5af6b6bb63d88f7797531f855ad0a8fb5bf0e4645a7b2f83516b4cb26bf1da4 guix-build-eca20bead2da/output/x86_64-apple-darwin/SHA256SUMS.part 3743509c6ad5fb3837bd8d420885b49221515e89d1bba12f8816f401879bee7a guix-build-eca20bead2da/output/x86_64-apple-darwin/bitcoin-eca20bead2da-x86_64-apple-darwin-unsigned.tar.gz cc6fb35a57506250790ddea4aaa7888aa9d1db66f8ce3f09c830898f83f2f39f guix-build-eca20bead2da/output/x86_64-apple-darwin/bitcoin-eca20bead2da-x86_64-apple-darwin-unsigned.zip 4b4d2096e87ca10847e5a543ff32f002325c882856523f0fc5d70564009f9244 guix-build-eca20bead2da/output/x86_64-apple-darwin/bitcoin-eca20bead2da-x86_64-apple-darwin.tar.gz 1989106147fc5f77bc27a08886bb2120ff0c49cbe6ea97b9e234752740ec81ad guix-build-eca20bead2da/output/x86_64-linux-gnu/SHA256SUMS.part bdb48a649f9ca026e6bbab28159f716a1ad4b84257588e1a12bf4467e4c7acb6 guix-build-eca20bead2da/output/x86_64-linux-gnu/bitcoin-eca20bead2da-x86_64-linux-gnu-debug.tar.gz aff4717e841508bd6284d846d8c6da7da3622bf54d68a8919e3fd95814beb309 guix-build-eca20bead2da/output/x86_64-linux-gnu/bitcoin-eca20bead2da-x86_64-linux-gnu.tar.gz fdc8346e0b0f03648399b74a0d38d961c985c5ec8128193443be0b7208632f06 guix-build-eca20bead2da/output/x86_64-w64-mingw32/SHA256SUMS.part a50f517e3f2467e5931349315bbe0968e190e8bcdbb024e3a8d4c37333938155 guix-build-eca20bead2da/output/x86_64-w64-mingw32/bitcoin-eca20bead2da-win64-debug.zip 38223484c214a90193f88f8c60743b376ce0c80f9401ec863ccb36a1337c85a2 guix-build-eca20bead2da/output/x86_64-w64-mingw32/bitcoin-eca20bead2da-win64-setup-unsigned.exe 07fca2496b2c59ea928684c4bf4ef163686f8fb11934117c6c37407a3a374363 guix-build-eca20bead2da/output/x86_64-w64-mingw32/bitcoin-eca20bead2da-win64-unsigned.tar.gz 8e71711db17c69000627c44965eb7070fc92788f792ea3d0778a55bdadf36cdb guix-build-eca20bead2da/output/x86_64-w64-mingw32/bitcoin-eca20bead2da-win64.zip ``` ACKs for top commit: TheCharlatan: Nice, ACK eca20be Tree-SHA512: eb1795dcc82bd975b76631326c72c4456d71be8b8cf509195295021c0581abee6da36b55d7faddb440f953e24996ee70b01b35ae9e945fcbb0799efb2e29b916
Issue being fixed or feature implemented
(extra) small batch of trivial backports
How Has This Been Tested?
CI
Breaking Changes
None
Checklist:
Go over all the following points, and put an
xin all the boxes that apply.