build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD

151henry151 commented at 10:35 pm on August 23, 2025: none

Remove deprecated CMake RPATH settings (CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH) that are no longer needed after reordering Guix build script to perform binary checks after installation.

Remove RPATH workarounds from CMakeLists.txt and src/CMakeLists.txt Reorder Guix script to check installed binaries instead of build tree binaries Update TODO comments

Builds successfully on Linux Full Guix build in progress

Addresses TODO items in CMakeLists.txt:620 and src/CMakeLists.txt:412

This is my first attempt at contributing to bitcoin core so please, provide me any feedback that you might have. Thanks!

DrahtBot added the label Build system on Aug 23, 2025

DrahtBot commented at 10:35 pm on August 23, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33247.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	hebasto, janb84
Concept ACK	purpleKarrot

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#33470 (build: Move CMAKE_SKIP_INSTALL_RPATH from CMake to Guix script by 151henry151)
#33455 (CPack by purpleKarrot)
#33278 (cmake: make missing Python interpreter behaviour more explicit by stickies-v)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot added the label CI failed on Aug 23, 2025

DrahtBot removed the label CI failed on Aug 23, 2025

hebasto commented at 7:15 am on August 24, 2025: member

This is my first attempt at contributing to bitcoin core so please, provide me any feedback that you might have. Thanks!

Thank you for your interest in contributing to this project!

The changes you are referring to as completed, i.e. “reordering Guix script commands to perform binary checks after the installation step”, have not actually been done yet. See contrib/guix/libexec/build.sh, from line 251 onwards.

151henry151 commented at 2:14 pm on August 24, 2025: none

This is my first attempt at contributing to bitcoin core so please, provide me any feedback that you might have. Thanks!

Thank you for your interest in contributing to this project!

The changes you are referring to as completed, i.e. “reordering Guix script commands to perform binary checks after the installation step”, have not actually been done yet. See contrib/guix/libexec/build.sh, from line 251 onwards.

Thanks for your helpful reply @hebasto. I’ve pushed another commit attempting to perform the actual reordering of the Guix script commands to perform those checks after the installation step; I tested it locally and it seemed to build successfully, hope I got the right idea here.

hebasto commented at 2:17 pm on August 24, 2025: member

In a02723427f5f0b142a6ef97b92df093e00ad6004, all checks are still performed on the binaries in the build tree, whereas they are supposed to be performed on the binaries in the installation location.

hebasto commented at 2:22 pm on August 24, 2025: member

… I tested it locally and it seemed to build successfully…

When a developer tests changes in the Guix scripts locally, they usually posts the resulting binary hashes. For example, as in #33217#pullrequestreview-3142128074.

151henry151 commented at 2:26 pm on August 24, 2025: none

Thanks for the feedback hebasto, I’ll continue working on this

Conflicts

Reviewers, this pull request conflicts with the following ones:
* [#32380](https://github.com/bitcoin/bitcoin/pull/32380) (Modernize use of UTF-8 in Windows code by hebasto)

The conflict resolves easily by keeping both changes.

janb84 commented at 3:18 pm on August 24, 2025: contributor

This is my first attempt at contributing to bitcoin core so please, provide me any feedback that you might have. Thanks!

Welcome, please also change this message for a description of the changes made in this PR. Helpful tips are in creating-the-pull-request

151henry151 commented at 3:23 pm on August 24, 2025: none

In a027234, all checks are still performed on the binaries in the build tree, whereas they are supposed to be performed on the binaries in the installation location.

Thanks for the feedback.

The previous change moved checks after installation but was still using cmake –build targets which check build tree binaries. I’ve now fixed this to directly call the security and symbol check scripts on the installed binaries in ${INSTALLPATH}/bin/, ensuring checks are performed on the final installed binaries with proper RPATHs.

When a developer tests changes in the Guix scripts locally, they usually posts the resulting binary hashes. For example, as in #33217#pullrequestreview-3142128074.

I’m currently running a full Guix build locally to generate the binary hashes. The build is in progress and I’ll post the resulting hashes once it completes.

151henry151 commented at 1:10 am on August 25, 2025: none

My Guix build resulting binary hashes:

075d4eb10b95a64c595ee1ddf31d3c1e0086958a1ae3d64f9e2bbc4930cd88366  dist-archive/bitcoin-e922e27a2ecd.tar.gz
1
2f7d2d6cd820561229ab5fa476c06484ced81a1fa8b90d2fbbd7a82fa915aa956  x86_64-linux-gnu/bitcoin-e922e27a2ecd-x86_64-linux-gnu-debug.tar.gz
3
4df24c2269cf96dbf011692fbc0b21dab7786abd43dbb82e4351f0117e95169e5  x86_64-linux-gnu/bitcoin-e922e27a2ecd-x86_64-linux-gnu.tar.gz

in CMakeLists.txt:621 in e922e27a2e outdated

617@@ -618,17 +618,16 @@ if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.29)
618   set(CMAKE_SKIP_TEST_ALL_DEPENDENCY FALSE)
619 endif()
620 
621-# TODO: The `CMAKE_SKIP_BUILD_RPATH` variable setting can be deleted
622-#       in the future after reordering Guix script commands to
623-#       perform binary checks after the installation step.
624+# TODO: The `CMAKE_SKIP_BUILD_RPATH` variable setting has been removed

janb84 commented at 7:53 am on August 25, 2025:

If the TODO is solved than there is no more TO DO. Please remove the todo or reword it what is still left to do.

151henry151 commented at 12:45 pm on August 25, 2025:

I clicked commit suggestion too quickly – It looks a bit like a comment was cut in half there. Would we want to remove the additional lines of the TODO as well, or rephrase them as comments?

151henry151 commented at 2:13 pm on August 25, 2025:

Looking at this more closely, the answer was clear and so I removed the additional lines of the TODO as well.

purpleKarrot commented at 7:41 am on September 3, 2025:

You modify the TODO in one commit and then delete it in another. You way want to rewrite the git history so that the todo is removed directly.

maflcko commented at 10:59 am on August 25, 2025: member

Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

151henry151 commented at 10:59 am on August 25, 2025: none

Would it be better to leave the comments there and simply remove the TODO tag? It looks a bit like a comment was cut in half there, but perhaps adding back in the The `CMAKE_SKIP_BUILD_RPATH` variable setting has been removed Would be better than removing the rest of that information. What do you think?

151henry151 force-pushed on Aug 25, 2025

maflcko added the label DrahtBot Guix build requested on Aug 28, 2025

151henry151 force-pushed on Aug 29, 2025

151henry151 commented at 5:34 am on August 29, 2025: none

There was a TODO still in the src/CMakeLists.txt file; I cleaned it up and squashed the commits just now and think that it is now all correct.

DrahtBot commented at 6:56 am on August 29, 2025: contributor

Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]

File	commit 6ca6f3b37b992591726bd13b494369bee3bd6468(master)	commit 1ac12bc409e0570cefc640fcac1fe13edfa98a92(pull/33247/merge)
*-aarch64-linux-gnu-debug.tar.gz	`abcc0baac5e7b752...`	`8ce308bbc579ad49...`
*-aarch64-linux-gnu.tar.gz	`4a8653178ccbdddb...`	`ed54dfe44406b72a...`
*-arm-linux-gnueabihf-debug.tar.gz	`95d3d74d5682c24a...`	`86d72da5723c7b7d...`
*-arm-linux-gnueabihf.tar.gz	`e25759641901b072...`	`d53eee042815730f...`
*-arm64-apple-darwin-codesigning.tar.gz	`366f30df86e875e8...`	`9aee1e5a66c0b9d5...`
*-arm64-apple-darwin-unsigned.tar.gz	`1c83672238b93111...`	`e07f9aec2228734e...`
*-arm64-apple-darwin-unsigned.zip	`ad51645f12fb5dc7...`	`87aa37bf869b5b55...`
*-powerpc64-linux-gnu-debug.tar.gz	`df430f763352d69c...`	`39f3f3ebb73204d8...`
*-powerpc64-linux-gnu.tar.gz	`7dd5cbe718652deb...`	`0e6ef9dea9eeafd2...`
*-riscv64-linux-gnu-debug.tar.gz	`d258d893caa4d9b7...`	`48ad366e15a9c433...`
*-riscv64-linux-gnu.tar.gz	`3f182eee5fd9c395...`	`48cf7092c537d8cd...`
*-x86_64-apple-darwin-codesigning.tar.gz	`3998753c28bfa555...`	`7397d894c9cabe8c...`
*-x86_64-apple-darwin-unsigned.tar.gz	`b4474a38f90d8d72...`	`147d8db165c37ccd...`
*-x86_64-apple-darwin-unsigned.zip	`6028f331e23a3f0f...`	`60a73a9684ce8e1d...`
*-x86_64-linux-gnu-debug.tar.gz	`881ca6a16e6f2ac5...`	`b55f8fb6ec93d54b...`
*-x86_64-linux-gnu.tar.gz	`01fc4a45eb400557...`	`0aac741f62f5299c...`
*.tar.gz	`0711f5eb93cb4d47...`	`0191779babb2b28f...`
SHA256SUMS.part	`1801e58d5cf74ae8...`	`beb6ef46d809ac3d...`
guix_build.log	`cbdad833bb02e563...`	`b87aee0c1d625b91...`
guix_build.log.diff		`b3dfe2faa87aaa23...`

DrahtBot removed the label DrahtBot Guix build requested on Aug 29, 2025

151henry151 commented at 3:53 pm on September 1, 2025: none

Is there anything further required here? I understand that review for pull requests can take a long time (especially for non-critical things such as this one) but just wanted to check in and see if there’s further action needed from me, or anything about this pull request that I could improve.

in contrib/guix/libexec/build.sh:283 in 49c468860f outdated

276@@ -282,6 +277,14 @@ mkdir -p "$DISTSRC"
277             ;;
278     esac
279 
280+    # Perform basic security checks on INSTALLED executables.
281+    # These checks now happen after installation, so RPATHs point to final locations.
282+    echo "Checking binary security on installed executables..."
283+    python3 "${DISTSRC}/contrib/guix/security-check.py" "${INSTALLPATH}/bin/"*

fanquake commented at 4:01 pm on September 1, 2025:

If we’re going to call the scripts directly, then you should remove the targets from cmake, as there’s no point having them, if they aren’t used in the Guix build.

151henry151 commented at 5:31 pm on September 1, 2025:

If we’re going to call the scripts directly, then you should remove the targets from cmake, as there’s no point having them, if they aren’t used in the Guix build.

Thanks @fanquake – I believe I’ve correctly removed the targets from cmake now in commit ef5dd15309f9. The Guix build hashes:

0b7775f39fab10c11713ca7bf38a8cac48a143a9478999ee64b927ea9c5040245  bitcoin-ef5dd15309f9.tar.gz
1e79d2bd8072a459eb75db8ef06f41a5dd9128ad63ca308cdf7c57c39670eb99f  bitcoin-ef5dd15309f9-x86_64-linux-gnu.tar.gz
2c734a7ff8f5552c70a703872df416b24bf705c72a9f77d9f81276715409e0a4c  bitcoin-ef5dd15309f9-x86_64-linux-gnu-debug.tar.gz

janb84 commented at 6:35 pm on September 18, 2025:

NIT: This only covers 1 of the 2 binary locations, nowadays we also have libexec should this not also be checked ?

151henry151 commented at 9:52 pm on September 19, 2025:

Thanks for the suggestion! I think I’ve addressed this correctly in 07027af – built for linux successfully; here’s my GUIX build hashes:

04ed2d3dfe075bf5f98042bfe2165d858a4913b6933c8098e52cf607fbe76c84b  x86_64-linux-gnu/bitcoin-07027afa155f-x86_64-linux-gnu-debug.tar.gz
17b11bd93f10a37554fffd6128ede1a47585348ad8d12687b39f7e7d4f0b144f8  x86_64-linux-gnu/bitcoin-07027afa155f-x86_64-linux-gnu.tar.gz

in src/CMakeLists.txt:415 in 7f312e4d7e outdated

409@@ -410,14 +410,13 @@ if(BUILD_UTIL_CHAINSTATE)
410   add_executable(bitcoin-chainstate
411     bitcoin-chainstate.cpp
412   )
413-  # TODO: The `SKIP_BUILD_RPATH` property setting can be deleted
414-  #       in the future after reordering Guix script commands to
415-  #       perform binary checks after the installation step.
416+  # TODO: The `SKIP_BUILD_RPATH` property setting has been removed
417+  #       after reordering Guix script commands to perform binary checks
418+  #       after the installation step.

purpleKarrot commented at 7:42 am on September 3, 2025:

Same here. You modify the TODO in one commit and then delete it in the next one. Just delete it directly.

151henry151 commented at 0:02 am on September 4, 2025:

I believe this has already been addressed by squashing the relevant commits.

in src/CMakeLists.txt:420 in 7f312e4d7e outdated

419   # Relevant discussions:
420   # - https://github.com/hebasto/bitcoin/pull/236#issuecomment-2183120953
421   # - https://github.com/bitcoin/bitcoin/pull/30312#issuecomment-2191235833
422   set_target_properties(bitcoin-chainstate PROPERTIES
423-    SKIP_BUILD_RPATH OFF
424   )

purpleKarrot commented at 7:43 am on September 3, 2025:

Here, you leave an empty set_target_properties call and remove it in the next commit. Just remove it directly.

151henry151 commented at 0:01 am on September 4, 2025:

I think these have been squashed correctly to address these clumsy mistakes and that the final three commits in the pull request reflect this.

maflcko commented at 6:12 am on September 4, 2025: member

Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

151henry151 force-pushed on Sep 4, 2025

151henry151 commented at 9:58 pm on September 4, 2025: none

Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

I’m still learning, but I think I did the squashing correctly now, hopefully! Thanks for your patience and hope I can be of some value to the project :)

151henry151 commented at 10:56 pm on September 11, 2025: none

I see that there were two failing checks – is there any further suggested action I could take to help with this? I’m beginning to investigate now to try to understand what failed and why. I see that it says

fatal: Remote branch llvmorg-21.1.1 not found in upstream origin

But I’m not fully understanding what’s causing this to happen.

davidgumberg commented at 0:33 am on September 12, 2025: contributor

This CI failure is spurious, see #33345, and has since stopped happening since llvm has pushed the llvmorg-21.1.1 tag to their git repo.

To force CI to rerun you can change your commit’s timestamp (and hash) by doing git commit --amend --no-edit and push it.

151henry151 force-pushed on Sep 12, 2025

151henry151 commented at 9:33 pm on September 13, 2025: none

Just following up to make sure this is ready for review. If anybody sees anything that should be addressed or that is not correct here please let me know.

151henry151 requested review from purpleKarrot on Sep 16, 2025

151henry151 requested review from hebasto on Sep 16, 2025

purpleKarrot commented at 1:51 pm on September 16, 2025: contributor

Code review ACK. I very much welcome the reduced complexity in the CMake configuration. Thanks for working on this!

I haven’t tested actually running the scripts yet.

janb84 commented at 6:36 pm on September 18, 2025: contributor

Concept ACK a9b211540b79832fb4e4f870d2f770cf03d35b11

This PR reorders the binary checks to after the installation step. And removes the code paths that are made superfluous by the reorg .

Open NIT/question on libexec inclusion.

Guix builds and the security tests runs now after install: ✅

This PR:

 0[101%] Built target test_bitcoin
 1-- Install configuration: "RelWithDebInfo"
 2-- Installing: /distsrc-base/distsrc-a9b211540b79-riscv64-linux-gnu/installed/bitcoin-a9b211540b79/bin/bitcoin-wallet
 3-- Installing: /distsrc-base/distsrc-a9b211540b79-riscv64-linux-gnu/installed/bitcoin-a9b211540b79/share/man/man1/bitcoin-wallet.1
 4-- Installing: /distsrc-base/distsrc-a9b211540b79-riscv64-linux-gnu/installed/bitcoin-a9b211540b79/bin/bitcoin
 5-- Installing: /distsrc-base/distsrc-a9b211540b79-riscv64-linux-gnu/installed/bitcoin-a9b211540b79/bin/bitcoind
 6-- Installing: /distsrc-base/distsrc-a9b211540b79-riscv64-linux-gnu/installed/bitcoin-a9b211540b79/share/man/man1/bitcoind.1
 7-- Installing: /distsrc-base/distsrc-a9b211540b79-riscv64-linux-gnu/installed/bitcoin-a9b211540b79/libexec/bitcoin-node
 8-- Installing: /distsrc-base/distsrc-a9b211540b79-riscv64-linux-gnu/installed/bitcoin-a9b211540b79/bin/bitcoin-cli
 9-- Installing: /distsrc-base/distsrc-a9b211540b79-riscv64-linux-gnu/installed/bitcoin-a9b211540b79/share/man/man1/bitcoin-cli.1
10-- Installing: /distsrc-base/distsrc-a9b211540b79-riscv64-linux-gnu/installed/bitcoin-a9b211540b79/bin/bitcoin-tx
11-- Installing: /distsrc-base/distsrc-a9b211540b79-riscv64-linux-gnu/installed/bitcoin-a9b211540b79/share/man/man1/bitcoin-tx.1
12-- Installing: /distsrc-base/distsrc-a9b211540b79-riscv64-linux-gnu/installed/bitcoin-a9b211540b79/bin/bitcoin-util
13-- Installing: /distsrc-base/distsrc-a9b211540b79-riscv64-linux-gnu/installed/bitcoin-a9b211540b79/share/man/man1/bitcoin-util.1
14-- Installing: /distsrc-base/distsrc-a9b211540b79-riscv64-linux-gnu/installed/bitcoin-a9b211540b79/bin/bitcoin-qt
15-- Set runtime path of "/distsrc-base/distsrc-a9b211540b79-riscv64-linux-gnu/installed/bitcoin-a9b211540b79/bin/bitcoin-qt" to ""
16-- Installing: /distsrc-base/distsrc-a9b211540b79-riscv64-linux-gnu/installed/bitcoin-a9b211540b79/share/man/man1/bitcoin-qt.1
17-- Installing: /distsrc-base/distsrc-a9b211540b79-riscv64-linux-gnu/installed/bitcoin-a9b211540b79/libexec/bitcoin-gui
18-- Set runtime path of "/distsrc-base/distsrc-a9b211540b79-riscv64-linux-gnu/installed/bitcoin-a9b211540b79/libexec/bitcoin-gui" to ""
19-- Installing: /distsrc-base/distsrc-a9b211540b79-riscv64-linux-gnu/installed/bitcoin-a9b211540b79/libexec/test_bitcoin
20Checking binary security on installed executables...
21Running symbol and dynamic library checks on installed executables...

V30.0rc1:

 0[100%] Built target bitcoin-gui
 1Checking binary security...
 2[100%] Built target check-security
 3[  2%] Built target test_util
 4[  2%] Built target crc32c
 5[  8%] Built target leveldb
 6[  9%] Built target minisketch
 7[...]
 8[100%] Built target bitcoin-gui_autogen
 9[100%] Built target bitcoin-gui
10Running symbol and dynamic library checks...
11[100%] Built target check-symbols
12-- Install configuration: "RelWithDebInfo"
13-- Installing: /distsrc-base/distsrc-30.0rc1-riscv64-linux-gnu/installed/bitcoin-30.0rc1/bin/bitcoin-wallet
14-- Installing: /distsrc-base/distsrc-30.0rc1-riscv64-linux-gnu/installed/bitcoin-30.0rc1/share/man/man1/bitcoin-wallet.1
15-- Installing: /distsrc-base/distsrc-30.0rc1-riscv64-linux-gnu/installed/bitcoin-30.0rc1/bin/bitcoin
16-- Installing: /distsrc-base/distsrc-30.0rc1-riscv64-linux-gnu/installed/bitcoin-30.0rc1/bin/bitcoind
17-- Installing: /distsrc-base/distsrc-30.0rc1-riscv64-linux-gnu/installed/bitcoin-30.0rc1/share/man/man1/bitcoind.1
18-- Installing: /distsrc-base/distsrc-30.0rc1-riscv64-linux-gnu/installed/bitcoin-30.0rc1/libexec/bitcoin-node

151henry151 referenced this in commit 07027afa15 on Sep 19, 2025

151henry151 requested review from janb84 on Sep 21, 2025

151henry151 requested review from fanquake on Sep 21, 2025

janb84 commented at 1:17 pm on September 22, 2025: contributor

Concept ACK 07027afa155f8ec9715c761d80630b31723c2c32

NIT: The separate commit has no additional value, please squash them.

Checked that

the bash script halts if python script exits with code 1 ✅
All the binaries are checked by the python script ✅

 0
 1Checking binary security on installed executables...
 2
 3Checking /distsrc-base/distsrc-26c40cec0448-riscu64-linux-gnu/installed/bitcoin-26c40cec0448/bin/bitcoin (ELF RISCU)
 4Checking /distsrc-base/distsrc-26c40cec0448-riscu64-linux-gnu/installed/bitcoin-26c40cec0448/bin/bitcoin-cli (ELF RISCV)
 5Checking /distsrc-base/distsrc-26c40cec0448-riscu64-linux-gnu/installed/bitcoin-26c40cec0448/bin/bitcoin-qt (ELF RISCV)
 6Checking /distsrc-base/distsrc-26c40cec0448-riscu64-linux-gnu/installed/bitcoin-26c40cec0448/bin/bitcoin-tx (ELF RISCV)
 7Checking /distsrc-base/distsrc-26c40cec0448-riscu64-linux-gnu/installed/bitcoin-26c40cec0448/bin/bitcoin-util (ELF RISCV)
 8Checking /distsrc-base/distsrc-26c40cec0448-riscu64-linux-gnu/installed/bitcoin-26c40cec0448/bin/bitcoin-wallet (ELF RISCV)
 9Checking /distsrc-base/distsrc-26c40cec0448-riscu64-linux-gnu/installed/bitcoin-26c40cec0448/bin/bitcoind (ELF RISCV)
10Checking /distsrc-base/distsrc-26c40cec0448-riscu64-linux-gnu/installed/bitcoin-26c40cec0448/libexec/bitcoin-gui (ELF RISCU)
11Checking /distsrc-base/distsrc-26c40cec0448-riscu64-linux-gnu/installed/bitcoin-26c40cec0448/libexec/bitcoin-node (ELF RISCU)
12Checking /distsrc-base/distsrc-26c40cec0448-riscu64-linux-gnu/installed/bitcoin-26c40cec0448/libexec/test_bitcoin (ELF RISCU)

in CMakeLists.txt:625 in 07027afa15 outdated

627 # NetBSD always requires runtime paths to be set for executables.
628 if(CMAKE_SYSTEM_NAME STREQUAL "NetBSD")
629   set(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE)
630 else()
631-  set(CMAKE_SKIP_BUILD_RPATH TRUE)
632   set(CMAKE_SKIP_INSTALL_RPATH TRUE)

hebasto commented at 6:50 pm on September 22, 2025:

It’s outside the scope of this PR, but CMAKE_SKIP_INSTALL_RPATH should be set in the Guix script, keeping the build system free of hardcoded behaviour.

hebasto commented at 6:53 pm on September 22, 2025: member

Approach ACK 07027afa155f8ec9715c761d80630b31723c2c32.

The comment “TODO: The CMAKE_SKIP_BUILD_RPATH variable setting can be deleted...” does not imply that anything is deprecated. Could you please update the PR description and commit message accordingly?

151henry151 force-pushed on Sep 23, 2025

151henry151 referenced this in commit 579c87a12e on Sep 23, 2025

151henry151 referenced this in commit eb98b612f8 on Sep 23, 2025

janb84 commented at 2:47 pm on September 24, 2025: contributor

ACK 9c13be9c45cad19d9db78e318b1e8376d56d6ec5

Thanks for squashing your commits!

My Guix build checksums: Commit: 9c13be9c45ca

 0 22ad5809ed8d8399b0c7960e8c57cbaae5220b2667650262a7ec98a25eaf671f  guix-build-9c13be9c45ca/output/aarch64-linux-gnu/SHA256SUMS.part
 1 60b2048157462a5bd7ae00fa60ec8f1d59d84857d55125b0eafa98fad4aa780e  guix-build-9c13be9c45ca/output/aarch64-linux-gnu/bitcoin-9c13be9c45ca-aarch64-linux-gnu-debug.tar.gz
 2 7a7280f6b1221a62e9e8591365431f2cc3a0fe111abe76799df774e5b1e82207  guix-build-9c13be9c45ca/output/aarch64-linux-gnu/bitcoin-9c13be9c45ca-aarch64-linux-gnu.tar.gz
 3 24c34eb9d583c6dc1f9adddf7d3ebd6e717c278cd51b454c767c5f6bf2c78ec0  guix-build-9c13be9c45ca/output/arm-linux-gnueabihf/SHA256SUMS.part
 4 cdd558dab24381ac72ac805ad5f8715bebebe291e3ba7fc7c71070364a18d066  guix-build-9c13be9c45ca/output/arm-linux-gnueabihf/bitcoin-9c13be9c45ca-arm-linux-gnueabihf-debug.tar.gz
 5 fdba945dd73d5cf2ba6413ec499c459708eda217d6c64d89b1ecf8300721c88a  guix-build-9c13be9c45ca/output/arm-linux-gnueabihf/bitcoin-9c13be9c45ca-arm-linux-gnueabihf.tar.gz
 6 edf1f03a79da8eda440e45625e0617e486847868215cc51ed6222900df94a8e9  guix-build-9c13be9c45ca/output/arm64-apple-darwin/SHA256SUMS.part
 7 b6ecf45d5fff2668a6f6d127da7e37c22680e06855a19b89f7b719ad436a3cdf  guix-build-9c13be9c45ca/output/arm64-apple-darwin/bitcoin-9c13be9c45ca-arm64-apple-darwin-codesigning.tar.gz
 8 fbcdc8bb52facac2e0f686c874f9a89c3850564deb106bcc2b2030673e48464f  guix-build-9c13be9c45ca/output/arm64-apple-darwin/bitcoin-9c13be9c45ca-arm64-apple-darwin-unsigned.tar.gz
 9 692a571078b0ee482fb7559805087b030302d368439088c7d6d2c68e1b09bac2  guix-build-9c13be9c45ca/output/arm64-apple-darwin/bitcoin-9c13be9c45ca-arm64-apple-darwin-unsigned.zip
10 03bb2e49ce986ddb620d7672c7c30168880c45802008c81e9eb6f878f4081d1c  guix-build-9c13be9c45ca/output/dist-archive/bitcoin-9c13be9c45ca.tar.gz
11 4d1c2db2282d76393f0024ce8f39a82eeb5892df8f7196b333458e9e4cdca719  guix-build-9c13be9c45ca/output/powerpc64-linux-gnu/SHA256SUMS.part
12 5d1c08b16f990ae7f74c6503b08aeb378080110af5c3a92507e7285694da81fd  guix-build-9c13be9c45ca/output/powerpc64-linux-gnu/bitcoin-9c13be9c45ca-powerpc64-linux-gnu-debug.tar.gz
13 72609c0794620a5977c49949202d59222c0723eaa9eabd0e37f183e902ff1929  guix-build-9c13be9c45ca/output/powerpc64-linux-gnu/bitcoin-9c13be9c45ca-powerpc64-linux-gnu.tar.gz
14 b1354bb7e032157a3a68eb39f15a5a0aefb3fb071cfac6d2d13ce87ce22c0f52  guix-build-9c13be9c45ca/output/riscv64-linux-gnu/SHA256SUMS.part
15 918d66e4d0f000c082f7a9bb62aa4606df612d13adefc1270e16d6d4351f8da8  guix-build-9c13be9c45ca/output/riscv64-linux-gnu/bitcoin-9c13be9c45ca-riscv64-linux-gnu-debug.tar.gz
16 95fc99008bfcf913d411234fc63fb4abb433e57d9205617a5a8786c2ab936ad9  guix-build-9c13be9c45ca/output/riscv64-linux-gnu/bitcoin-9c13be9c45ca-riscv64-linux-gnu.tar.gz
17 c4f3ee6367485a4a0f37495d93bb19a1acc6fe6e485c47510cf7fb661d4dc002  guix-build-9c13be9c45ca/output/x86_64-apple-darwin/SHA256SUMS.part
18 eb147cdac24259b279b8999745cb89bd17762df8261bb678e06148cc30348e06  guix-build-9c13be9c45ca/output/x86_64-apple-darwin/bitcoin-9c13be9c45ca-x86_64-apple-darwin-codesigning.tar.gz
19 1856a8e2f4f5d06db427f3f66066b01d411eeb6b87980b878f374460e13502dc  guix-build-9c13be9c45ca/output/x86_64-apple-darwin/bitcoin-9c13be9c45ca-x86_64-apple-darwin-unsigned.tar.gz
20 3f87e1e08ff5c6755765ec85160f31c25cac3351dc49a4d52af08ce894315490  guix-build-9c13be9c45ca/output/x86_64-apple-darwin/bitcoin-9c13be9c45ca-x86_64-apple-darwin-unsigned.zip
21 98727d830043e223bc75d7e756412a9e56b995af1c3db3a963fee3f5ce92a6ed  guix-build-9c13be9c45ca/output/x86_64-linux-gnu/SHA256SUMS.part
22 d781e85479e6c0c62eedf4b59db5c32c5342ed9dfe49daa423b71cfcc0906bf2  guix-build-9c13be9c45ca/output/x86_64-linux-gnu/bitcoin-9c13be9c45ca-x86_64-linux-gnu-debug.tar.gz
23 799c230ba61826a9ebe39088d074d4c5608266e469da9356044ebbf50c60b28e  guix-build-9c13be9c45ca/output/x86_64-linux-gnu/bitcoin-9c13be9c45ca-x86_64-linux-gnu.tar.gz
24 916d6a93a5b0f174319afb351eaf3dfa5edcf6648fec3b0b23250bc6be6f9395  guix-build-9c13be9c45ca/output/x86_64-w64-mingw32/SHA256SUMS.part
25 0a7413e714a7bfd7dbc327ef9e6f62e9472f3214d40c678c62f818aadb93343d  guix-build-9c13be9c45ca/output/x86_64-w64-mingw32/bitcoin-9c13be9c45ca-win64-codesigning.tar.gz
26 d26b0ff872e8e5db1b5f87468e8d66be0114a3088ef0fca740dd7f629a966738  guix-build-9c13be9c45ca/output/x86_64-w64-mingw32/bitcoin-9c13be9c45ca-win64-debug.zip
27 a915942305a64eb76bb48afef01c4a8532e34ec2434239a3cc558e279dcf952b  guix-build-9c13be9c45ca/output/x86_64-w64-mingw32/bitcoin-9c13be9c45ca-win64-setup-unsigned.exe
28 461d043ae6a8b0208eaa492a23f9d9ec8f8eda9773360d9e4c5a200f873ed26b  guix-build-9c13be9c45ca/output/x86_64-w64-mingw32/bitcoin-9c13be9c45ca-win64-unsigned.zip

DrahtBot requested review from hebasto on Sep 24, 2025

fanquake renamed this:
~~build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings~~
build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings
on Sep 24, 2025

151henry151 referenced this in commit dd5c517757 on Sep 24, 2025

in contrib/guix/libexec/build.sh:280 in 9c13be9c45

276@@ -282,6 +277,14 @@ mkdir -p "$DISTSRC"
277             ;;
278     esac
279 
280+    # Perform basic security checks on INSTALLED executables.

hebasto commented at 12:25 pm on September 25, 2025:

Is there any particular reason to capitalize “installed” here?

in contrib/guix/libexec/build.sh:281 in 9c13be9c45

276@@ -282,6 +277,14 @@ mkdir -p "$DISTSRC"
277             ;;
278     esac
279 
280+    # Perform basic security checks on INSTALLED executables.
281+    # These checks now happen after installation, so RPATHs point to final locations.

hebasto commented at 12:27 pm on September 25, 2025:

This comment doesn’t seem correct to me, since we check that installed binaries have no RUNPATH or RPATH set. Maybe drop this line?

DrahtBot requested review from hebasto on Sep 25, 2025

hebasto commented at 12:46 pm on September 25, 2025: member

I’ve tested 9c13be9c45cad19d9db78e318b1e8376d56d6ec5, including running with V=1. It looks good.

Please amend the comments in contrib/guix/libexec/build.sh as noted above.

DrahtBot requested review from hebasto on Sep 25, 2025

build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings

Remove CMake settings that are no longer needed after reordering Guix build
script to perform binary checks after installation. Also include libexec/
binaries in security checks since PR #31679 moved internal binaries there.

Changes:
- Remove CMAKE_SKIP_BUILD_RPATH TRUE setting from main CMakeLists.txt
- Remove SKIP_BUILD_RPATH OFF property setting from src/CMakeLists.txt
- Reorder Guix script to check installed binaries instead of build tree binaries
- Remove unused CMake maintenance targets (check-security, check-symbols)
- Update security checks to include libexec/ directory binaries
- Update TODO comments to reflect completed changes

This addresses the TODO items mentioned in:
- CMakeLists.txt:620
- src/CMakeLists.txt:412

Relevant discussions:
- https://github.com/hebasto/bitcoin/pull/236#issuecomment-2183120953
- https://github.com/bitcoin/bitcoin/pull/30312#issuecomment-2191235833

d557536262

151henry151 force-pushed on Sep 26, 2025

151henry151 commented at 0:20 am on September 26, 2025: none

Please amend the comments in contrib/guix/libexec/build.sh as noted above.

I’ve made the recommended amendments, thanks for the guidance.

hebasto commented at 7:24 am on September 26, 2025: member

My Guix build:

 0aarch64
 1ab375345e39248d2c61fe6fce1afeb7b95623cfc9db452a7cfbd5b6797bf9c16  guix-build-d55753626292/output/aarch64-linux-gnu/SHA256SUMS.part
 285bd20c1b7076387d8efcb5ade58d29774f8114a712492a77315627d0db41094  guix-build-d55753626292/output/aarch64-linux-gnu/bitcoin-d55753626292-aarch64-linux-gnu-debug.tar.gz
 3f52e90c92d99b17c916b2c3ff2b57608597ca39ec7324ff5840550636cfd3a50  guix-build-d55753626292/output/aarch64-linux-gnu/bitcoin-d55753626292-aarch64-linux-gnu.tar.gz
 49c8c935f51cc751bb7f92f918bcb7197518b906f103cc8c1ba5d8a92aa8d34ec  guix-build-d55753626292/output/arm-linux-gnueabihf/SHA256SUMS.part
 52296587e0370c300eb289ac6fd8216de97ec2eb2516b5af3cea6d390ac7de478  guix-build-d55753626292/output/arm-linux-gnueabihf/bitcoin-d55753626292-arm-linux-gnueabihf-debug.tar.gz
 64a352b62b9ab50f830c7df6450ff6510862dc867a265480855a28429fb080942  guix-build-d55753626292/output/arm-linux-gnueabihf/bitcoin-d55753626292-arm-linux-gnueabihf.tar.gz
 7be4ee4d187d92b9387e84e8e6208254a2c5bab175cac2a70e44bf2a078ba38e7  guix-build-d55753626292/output/arm64-apple-darwin/SHA256SUMS.part
 806d67aa4b7a313de6da9a3f30cb75ec5047c9384e54aef0eb0de262d2ada71e6  guix-build-d55753626292/output/arm64-apple-darwin/bitcoin-d55753626292-arm64-apple-darwin-codesigning.tar.gz
 9ae1857537795adf11526d03f97185bb560269b2c9f01aac1e529c5795c59d10d  guix-build-d55753626292/output/arm64-apple-darwin/bitcoin-d55753626292-arm64-apple-darwin-unsigned.tar.gz
10c7785d204823eededf39ae432c025ef4273305a28808b18f08fa8af04f9eed80  guix-build-d55753626292/output/arm64-apple-darwin/bitcoin-d55753626292-arm64-apple-darwin-unsigned.zip
117db1175dc6522b2bfdc555f4cf531b0d4b24a2cd8ebf122526fb812f5deee815  guix-build-d55753626292/output/dist-archive/bitcoin-d55753626292.tar.gz
12e944b955363d82291dd61f89d5c145ff0e15c89769dfe4e94d7b305686578b7d  guix-build-d55753626292/output/powerpc64-linux-gnu/SHA256SUMS.part
13f5783aaba0d7bd34d357bb125cc81570399873457276f402969d65ecaf67ef08  guix-build-d55753626292/output/powerpc64-linux-gnu/bitcoin-d55753626292-powerpc64-linux-gnu-debug.tar.gz
14e357b89396ad99721becfb6092918a67e2f7fdb2a60e3e8b56bbc6662d1686d0  guix-build-d55753626292/output/powerpc64-linux-gnu/bitcoin-d55753626292-powerpc64-linux-gnu.tar.gz
1558b0b34b6f1cfebdfac5b8e86dc70710644fbfc6c1f227b938a05a6267bdc108  guix-build-d55753626292/output/riscv64-linux-gnu/SHA256SUMS.part
16745e1c1cc8dfef7cd72b9d50562acab5281237e198e34ca6d69273fa10bab1fa  guix-build-d55753626292/output/riscv64-linux-gnu/bitcoin-d55753626292-riscv64-linux-gnu-debug.tar.gz
171356ff14875a5828573d0b9dea35f8c147f011fb6c766dfcd3d11b1452c91b4a  guix-build-d55753626292/output/riscv64-linux-gnu/bitcoin-d55753626292-riscv64-linux-gnu.tar.gz
180f341adc5565f4142ba60adbafc191a7db3c2a46d09691a1d2057d8f883f60b3  guix-build-d55753626292/output/x86_64-apple-darwin/SHA256SUMS.part
1901bf822e17e7c29dcf19915dc3ed86ec5a5e71b46ac08fa9d56c62070b5d8d8a  guix-build-d55753626292/output/x86_64-apple-darwin/bitcoin-d55753626292-x86_64-apple-darwin-codesigning.tar.gz
209178b67b056bbe462f080886409372eea8b7536be1cf6d86ea92ae2df6e89381  guix-build-d55753626292/output/x86_64-apple-darwin/bitcoin-d55753626292-x86_64-apple-darwin-unsigned.tar.gz
21f014d3dba23bbd1f765b3b9541ef8af47f2d807b2e1bc56c20ccd1aa2e11d1c0  guix-build-d55753626292/output/x86_64-apple-darwin/bitcoin-d55753626292-x86_64-apple-darwin-unsigned.zip
22b3e0d775e5aac539f33a1bfd39059dbb19368b38ada2c1dabf5ebd6fd782b820  guix-build-d55753626292/output/x86_64-linux-gnu/SHA256SUMS.part
23ea51bd23d6e40ae306c94a04d145a2973ed888603c034c5a702942aaaba9a551  guix-build-d55753626292/output/x86_64-linux-gnu/bitcoin-d55753626292-x86_64-linux-gnu-debug.tar.gz
249c1d72510c0d78ba602ceff3b4d62455bf8757dfa28f95388834f027990f72ae  guix-build-d55753626292/output/x86_64-linux-gnu/bitcoin-d55753626292-x86_64-linux-gnu.tar.gz
25c7e716768276db9227265512b73a85e5dbc3d70a9bfa05f55ce3b90047f12ca3  guix-build-d55753626292/output/x86_64-w64-mingw32/SHA256SUMS.part
26ad89a26a6f61bc65ae4904f8a8ff48f4a2e62c3b49bb4e6ebfdd4a8fae8fdd61  guix-build-d55753626292/output/x86_64-w64-mingw32/bitcoin-d55753626292-win64-codesigning.tar.gz
279447f5cde1617225b65caa70d0d76faf864320d050005160bf1a69c3d0f18175  guix-build-d55753626292/output/x86_64-w64-mingw32/bitcoin-d55753626292-win64-debug.zip
280dc73ff188474f8ea27194b28c09e7e4846c13264b2ddf740152a2e4481f5662  guix-build-d55753626292/output/x86_64-w64-mingw32/bitcoin-d55753626292-win64-setup-unsigned.exe
29a1dbb15ca1999363cc936e2bab3eed5c512a4b1acc5d5f97aa125e86687bfb8f  guix-build-d55753626292/output/x86_64-w64-mingw32/bitcoin-d55753626292-win64-unsigned.zip

hebasto approved

hebasto commented at 7:31 am on September 26, 2025: member

ACK d55753626292be17fbb300b54601942ff2730729.

DrahtBot requested review from janb84 on Sep 26, 2025

janb84 commented at 8:05 am on September 26, 2025: contributor

re ACK d55753626292be17fbb300b54601942ff2730729

change since last ack:

changes in comments in script.

build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings #33247

Code Coverage & Benchmarks

Reviews

Conflicts

Conflicts

Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]