depends: add *FLAGS to gen_id #31125

pull fanquake wants to merge 2 commits into bitcoin:master from fanquake:external_flags_plus_linker_cache changing 2 files +30 −3
  1. fanquake commented at 5:10 pm on October 21, 2024: member
    The depends cache should be busted when flags change, the same as any other tooling change. I’d also like to start passing *FLAGS into depends inside the Guix env, which, without this change, doesn’t bust the cache.
  2. DrahtBot commented at 5:10 pm on October 21, 2024: contributor

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

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Concept ACK hebasto, theuni

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

  3. DrahtBot added the label Build system on Oct 21, 2024
  4. hebasto commented at 8:31 am on October 22, 2024: member
    Concept ACK.
  5. fanquake commented at 11:03 am on October 22, 2024: member
    Two other things that should happen at the same time as this are adding the linker (this is somewhat compensated for via adding the c/xx flags), as well as making the flag overriding work correctly.
  6. hebasto commented at 1:43 pm on October 22, 2024: member

    My Guix build:

     0aarch64
 16361934f5c9bd884aff2ef18000ca718974d8ea564939db6acf3f3ac6faa5e35  guix-build-cd048e03e258/output/aarch64-linux-gnu/SHA256SUMS.part
 2d3be037f7cd976b315f96303a56952e3eb5bb8f1963d01c0ede701af2cfee83a  guix-build-cd048e03e258/output/aarch64-linux-gnu/bitcoin-cd048e03e258-aarch64-linux-gnu-debug.tar.gz
 37c6e75134c8b6614758c024b7bdfcd3bddd0b652d374cbb852e365f00b360a18  guix-build-cd048e03e258/output/aarch64-linux-gnu/bitcoin-cd048e03e258-aarch64-linux-gnu.tar.gz
 44514a22fbffe658ef632b013941143c0778b5bacb54a363f4310d3277577b5f0  guix-build-cd048e03e258/output/arm-linux-gnueabihf/SHA256SUMS.part
 564ec808aba524f7630cb3b2c3a6944e9b88d72daefed60fbae2f9ddedae7f077  guix-build-cd048e03e258/output/arm-linux-gnueabihf/bitcoin-cd048e03e258-arm-linux-gnueabihf-debug.tar.gz
 6fa9ba95096142799b4febfee788959fd5a425be0c846dc345c65b0dbfe1a219f  guix-build-cd048e03e258/output/arm-linux-gnueabihf/bitcoin-cd048e03e258-arm-linux-gnueabihf.tar.gz
 72356c77fe389353d8e13f961ffb49ad7130a084ff782589e1159677e0a330c5e  guix-build-cd048e03e258/output/arm64-apple-darwin/SHA256SUMS.part
 8bc501aeb65d96defe117181a53d912fd381b60c7df85fbcf54b375a0f7200f5b  guix-build-cd048e03e258/output/arm64-apple-darwin/bitcoin-cd048e03e258-arm64-apple-darwin-unsigned.tar.gz
 93ac2543266223d53cd050bb5fd1394906e64b495ebeef1419763362e3645f55c  guix-build-cd048e03e258/output/arm64-apple-darwin/bitcoin-cd048e03e258-arm64-apple-darwin-unsigned.zip
106fa6115d33fe9387c534d9f898b4bc4320125a74dfac349545ed0ea1d5215d05  guix-build-cd048e03e258/output/arm64-apple-darwin/bitcoin-cd048e03e258-arm64-apple-darwin.tar.gz
11ff6c4ccefcfa783457a5cde35289e77a732bb20ef1052f7e38b18608d2974d94  guix-build-cd048e03e258/output/dist-archive/bitcoin-cd048e03e258.tar.gz
12ffd97825fdd6c81eb0f10d283b9a5d50ec1076f8cabc70634e3421ca783a4466  guix-build-cd048e03e258/output/powerpc64-linux-gnu/SHA256SUMS.part
13bf91ae047f7941af0de90dcd19720fee5ddb52badcadbb54de08fdb2b66a952f  guix-build-cd048e03e258/output/powerpc64-linux-gnu/bitcoin-cd048e03e258-powerpc64-linux-gnu-debug.tar.gz
14b320cd3d24fbea6e18a872a3e96814039f32311bff8206fe23231f45d22fceb9  guix-build-cd048e03e258/output/powerpc64-linux-gnu/bitcoin-cd048e03e258-powerpc64-linux-gnu.tar.gz
15fb160f2c2333cac19c89a48ee8c9ec67638cda2dae1449520f2fd5186b8cf007  guix-build-cd048e03e258/output/riscv64-linux-gnu/SHA256SUMS.part
166e4103f2b8467f049d9f7bb849fdfd96e04af2b8c4d48c11e2b1fa1e77c2db67  guix-build-cd048e03e258/output/riscv64-linux-gnu/bitcoin-cd048e03e258-riscv64-linux-gnu-debug.tar.gz
1718ec18d121cac286ec2f1f0159841bbdac04b1c3bcc2f820b381db73d063b63b  guix-build-cd048e03e258/output/riscv64-linux-gnu/bitcoin-cd048e03e258-riscv64-linux-gnu.tar.gz
1815b48d85bbe68a29ed5ccc27212b0afc03e7aa4e1ae84490dba3deeae693aab2  guix-build-cd048e03e258/output/x86_64-apple-darwin/SHA256SUMS.part
19c3bdc254e4d3e25514e73c8959e2effb28c16422a447fcd324739de77c384dad  guix-build-cd048e03e258/output/x86_64-apple-darwin/bitcoin-cd048e03e258-x86_64-apple-darwin-unsigned.tar.gz
20e25add8b347ebf77acff1548ac14a9acab870efefa557c598366a275c0653687  guix-build-cd048e03e258/output/x86_64-apple-darwin/bitcoin-cd048e03e258-x86_64-apple-darwin-unsigned.zip
21e7d43cfc0bcad5e4d6b8b9d3c40262bef95a4c749c5f6db3211c422f421e9ccd  guix-build-cd048e03e258/output/x86_64-apple-darwin/bitcoin-cd048e03e258-x86_64-apple-darwin.tar.gz
227690e77c3948e216bb06bf3205f9be28cac4594b6f187ba98da40b398e6792b3  guix-build-cd048e03e258/output/x86_64-linux-gnu/SHA256SUMS.part
23cf1fc1da25f4f82de785294df3957ed967893fa1a77b715763fc5adcd3bc7c69  guix-build-cd048e03e258/output/x86_64-linux-gnu/bitcoin-cd048e03e258-x86_64-linux-gnu-debug.tar.gz
246958cb04755c26f0c3fd44599c00b4b5d14e97897e1177bb4994f5371e76c395  guix-build-cd048e03e258/output/x86_64-linux-gnu/bitcoin-cd048e03e258-x86_64-linux-gnu.tar.gz
25f9b87fcaf34cf0182ad620d57b178c4decd562d165944da87593391e7589c24f  guix-build-cd048e03e258/output/x86_64-w64-mingw32/SHA256SUMS.part
26a74fadfd30da6af3764191ce910bc1eb8ac9c3a8d8d46a2c5e9e194cc811b1e0  guix-build-cd048e03e258/output/x86_64-w64-mingw32/bitcoin-cd048e03e258-win64-debug.zip
2716347d423ed3d83a487fd3d81cef30b1c8998b9c0986ebb1b574f30f7d1a39a0  guix-build-cd048e03e258/output/x86_64-w64-mingw32/bitcoin-cd048e03e258-win64-setup-unsigned.exe
28b5e493fbbfc3da13a8969402ccfc7bc8146042bbaa93aee6f12dbbad00698fb6  guix-build-cd048e03e258/output/x86_64-w64-mingw32/bitcoin-cd048e03e258-win64-unsigned.tar.gz
29e92c1f39f88d414bba6a04ce0b3fd53f50714ccd463e44f9c1f166091bba8a83  guix-build-cd048e03e258/output/x86_64-w64-mingw32/bitcoin-cd048e03e258-win64.zip
  7. in depends/Makefile:146 in cd048e03e2 outdated 
    141@@ -142,8 +142,15 @@ include packages/packages.mk
142 #     2. Before including packages/*.mk (excluding packages/packages.mk), since
143 #        they rely on the build_id variables
144 #
145-build_id:=$(shell env CC='$(build_CC)' C_STANDARD='$(C_STANDARD)' CXX='$(build_CXX)' CXX_STANDARD='$(CXX_STANDARD)' AR='$(build_AR) 'NM='$(build_NM)' RANLIB='$(build_RANLIB)' STRIP='$(build_STRIP)' SHA256SUM='$(build_SHA256SUM)' DEBUG='$(DEBUG)' LTO='$(LTO)' NO_HARDEN='$(NO_HARDEN)' ./gen_id '$(BUILD_ID_SALT)' 'GUIX_ENVIRONMENT=$(realpath $(GUIX_ENVIRONMENT))')
146-$(host_arch)_$(host_os)_id:=$(shell env CC='$(host_CC)' C_STANDARD='$(C_STANDARD)' CXX='$(host_CXX)' CXX_STANDARD='$(CXX_STANDARD)' AR='$(host_AR)' NM='$(host_NM)' RANLIB='$(host_RANLIB)' STRIP='$(host_STRIP)' SHA256SUM='$(build_SHA256SUM)' DEBUG='$(DEBUG)' LTO='$(LTO)' NO_HARDEN='$(NO_HARDEN)' ./gen_id '$(HOST_ID_SALT)' 'GUIX_ENVIRONMENT=$(realpath $(GUIX_ENVIRONMENT))')
147+build_id:=$(shell env CC='$(build_CC)' C_STANDARD='$(C_STANDARD)' CXX='$(build_CXX)' CXX_STANDARD='$(CXX_STANDARD)' \
148+                      CPPFLAGS='$(CPPFLAGS)' CFLAGS='$(CFLAGS)' CXXFLAGS='$(CXXFLAGS)' LDFLAGS='$(LDFLAGS)' \
    hebasto commented at 2:00 pm on October 22, 2024:

    I’m not sure about that as none of the {CPP,C,CXX,LD}FLAGS variables is propagated to the native packages:

    0$ cd depends
1$ make HOST=arm64-apple-darwin MULTIPROCESS=1 print-native_libmultiprocess_cxxflags CXXFLAGS=-some-fancy-flag
2native_libmultiprocess_cxxflags=
    TheCharlatan commented at 2:03 pm on October 22, 2024:
    Mmh, isn’t that intentional?
    theuni commented at 3:45 pm on October 22, 2024:
    So why bother passing them? I don’t see any need to rebuild the native packages when changing host flags?
  8. in depends/gen_id:4 in cd048e03e2 outdated 
    0@@ -1,6 +1,7 @@
1 #!/usr/bin/env bash
2 
3 # Usage: env [ CC=... ] [ C_STANDARD=...] [ CXX=... ] [CXX_STANDARD=...] \
4+#            [ CPPFLAGS=... ] [CFLAGS=...] [CXXFLAGS=...] [LDFLAGS=...]
    hebasto commented at 2:00 pm on October 22, 2024:

    nit:

    0#            [ CPPFLAGS=... ] [CFLAGS=...] [CXXFLAGS=...] [LDFLAGS=...] \

    While touching this code, the comment can be improved:

     0--- a/depends/gen_id
 1+++ b/depends/gen_id
 2@@ -3,7 +3,7 @@
 3 # Usage: env [ CC=... ] [ C_STANDARD=...] [ CXX=... ] [CXX_STANDARD=...] \
 4 #            [ CPPFLAGS=... ] [CFLAGS=...] [CXXFLAGS=...] [LDFLAGS=...]
 5 #            [ AR=... ] [ NM=... ] [ RANLIB=... ] [ STRIP=... ] [ DEBUG=... ] \
 6-#            [ LTO=... ] [ NO_HARDEN=... ] ./build-id [ID_SALT]...
 7+#            [ LTO=... ] [ NO_HARDEN=... ] ./gen-id [ID_SALT]...
 8 #
 9 # Prints to stdout a SHA256 hash representing the current toolset, used by
10 # depends/Makefile as a build id for caching purposes (detecting when the
    fanquake commented at 2:16 pm on October 22, 2024:
    Added.
  9. in depends/Makefile:148 in cd048e03e2 outdated 
    141@@ -142,8 +142,15 @@ include packages/packages.mk
142 #     2. Before including packages/*.mk (excluding packages/packages.mk), since
143 #        they rely on the build_id variables
144 #
145-build_id:=$(shell env CC='$(build_CC)' C_STANDARD='$(C_STANDARD)' CXX='$(build_CXX)' CXX_STANDARD='$(CXX_STANDARD)' AR='$(build_AR) 'NM='$(build_NM)' RANLIB='$(build_RANLIB)' STRIP='$(build_STRIP)' SHA256SUM='$(build_SHA256SUM)' DEBUG='$(DEBUG)' LTO='$(LTO)' NO_HARDEN='$(NO_HARDEN)' ./gen_id '$(BUILD_ID_SALT)' 'GUIX_ENVIRONMENT=$(realpath $(GUIX_ENVIRONMENT))')
146-$(host_arch)_$(host_os)_id:=$(shell env CC='$(host_CC)' C_STANDARD='$(C_STANDARD)' CXX='$(host_CXX)' CXX_STANDARD='$(CXX_STANDARD)' AR='$(host_AR)' NM='$(host_NM)' RANLIB='$(host_RANLIB)' STRIP='$(host_STRIP)' SHA256SUM='$(build_SHA256SUM)' DEBUG='$(DEBUG)' LTO='$(LTO)' NO_HARDEN='$(NO_HARDEN)' ./gen_id '$(HOST_ID_SALT)' 'GUIX_ENVIRONMENT=$(realpath $(GUIX_ENVIRONMENT))')
147+build_id:=$(shell env CC='$(build_CC)' C_STANDARD='$(C_STANDARD)' CXX='$(build_CXX)' CXX_STANDARD='$(CXX_STANDARD)' \
148+                      CPPFLAGS='$(CPPFLAGS)' CFLAGS='$(CFLAGS)' CXXFLAGS='$(CXXFLAGS)' LDFLAGS='$(LDFLAGS)' \
149+                      AR='$(build_AR) 'NM='$(build_NM)' RANLIB='$(build_RANLIB)' STRIP='$(build_STRIP)' SHA256SUM='$(build_SHA256SUM)' \
150+                      DEBUG='$(DEBUG)' LTO='$(LTO)' NO_HARDEN='$(NO_HARDEN)' ./gen_id '$(BUILD_ID_SALT)' 'GUIX_ENVIRONMENT=$(realpath $(GUIX_ENVIRONMENT))')
    TheCharlatan commented at 2:02 pm on October 22, 2024:
    Nit: While you are at it maybe break before ./gen_id too for readability purposes?
    fanquake commented at 2:16 pm on October 22, 2024:
    Broken.
  10. fanquake force-pushed on Oct 22, 2024
  11. theuni commented at 3:46 pm on October 22, 2024: member
    Concept ACK
  12. depends: add *FLAGS to gen_id 
    The depends cache should be busted when flags change, the same as any
other tooling change. Id also like to start passing *FLAGS into depends
inside the Guix env, which, without this change, doesn't bust the cache.
    720b29fee0
  13. depends: add mold & ld.lld to gen_id 
    We use `lld` when cross-compiling for macOS, and it's version should
be tied to LLVM. However someone compiling with GCC and `-fuse-ld=lld`
would not see a cache bust if the LLVM toolchain was updated.

We don't use `mold` directly, but I'm aware of it's usage in
infrastructure, along with depends, used to test the project.
    8718a067d3
  14. fanquake force-pushed on Oct 24, 2024


fanquake DrahtBot hebasto TheCharlatan theuni

Labels
Build system

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-03 15:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me