cmake: Get rid of undocumented `BITCOIN_GENBUILD_NO_GIT` environment variable #32220

pull hebasto wants to merge 4 commits into bitcoin:master from hebasto:250404-tarball changing 4 files +68 −55

hebasto commented at 3:05 PM on April 4, 2025: member
In general, the Bitcoin Core build system attempts to fetch commit or tag details from git. This is handled by the cmake/script/GenerateBuildInfo.cmake script, which generates the src/bitcoin-build-info.h header within the build tree.

However, there are cases where the retrieved details may be incorrect—for example, when building from a source tarball or as a subproject within a git-aware project.

In the Autotools-based build system, the BITCOIN_GENBUILD_NO_GIT environment variable was introduced in v0.20.0 to address such scenarios:
The process for generating the source code release ("tarball") has changed in an effort to make it more complete, however, there are a few regressions in this release:

Instead of running make simply, you should instead run BITCOIN_GENBUILD_NO_GIT=1 make.
This PR automagically handles both of the aforementioned cases and removes the need for BITCOIN_GENBUILD_NO_GIT.

The user is still able to configure the build with -DCMAKE_DISABLE_FIND_PACKAGE_Git=ON to disable git execution manually, for reasons we don't know in advance.

Closes #31999.
hebasto added the label Build system on Apr 4, 2025
DrahtBot commented at 3:05 PM on April 4, 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/32220.



Reviews

See the guideline for information on the review process.

Type Reviewers

Concept NACK luke-jr

Concept ACK theuni, fanquake

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.



Conflicts

No conflicts as of last run.
hebasto added this to the milestone 29.0 on Apr 4, 2025
luke-jr commented at 9:05 PM on April 4, 2025: member

Concept NACK to removing the ability for the user to disable git execution manually (perhaps for reasons we don't know in advance).
in cmake/script/GenerateBuildInfo.cmake:32 in 159b5a43a8 outdated
```
  27 | @@ -28,9 +28,12 @@ else()
  28 |    set(WORKING_DIR ${CMAKE_CURRENT_SOURCE_DIR})
  29 |  endif()
  30 |  
  31 | -set(GIT_TAG)
  32 | -set(GIT_COMMIT)
  33 | -if(NOT "$ENV{BITCOIN_GENBUILD_NO_GIT}" STREQUAL "1")
  34 | +set(IS_SOURCE_TARBALL FALSE)
  35 | +# git will put "set(IS_SOURCE_TARBALL TRUE)" on the next line inside archives. $Format:%nset(IS_SOURCE_TARBALL TRUE)$
```
luke-jr commented at 9:08 PM on April 4, 2025:

Maybe we should use $Format:%(describe)%$ to actually include build info?

hebasto commented at 1:17 PM on April 7, 2025:

Maybe we should use $Format:%(describe)%$ to actually include build info?

Could you clarify how this improves the PR?

luke-jr commented at 12:57 AM on April 10, 2025:

This way we can put the correct build info in, rather than just failing to get any build info.
hebasto commented at 1:26 AM on April 5, 2025: member

Concept NACK to removing the ability for the user to disable git execution manually (perhaps for reasons we don't know in advance).

The user is still able to configure the build with -DCMAKE_DISABLE_FIND_PACKAGE_Git=ON.
maflcko commented at 8:02 AM on April 7, 2025: member

The user is still able to configure the build with -DCMAKE_DISABLE_FIND_PACKAGE_Git=ON.

Could mention in the pull description? Also, it looks like the tail needs editing? Ref:

This PR handles with both mentioned cases automagically and gets rid of BITCOIN_GENBUILD_NO_GIT.

This PR automagically handles both of the aforementioned cases and removes the need for BITCOIN_GENBUILD_NO_GIT.
hebasto commented at 10:23 AM on April 7, 2025: member

The user is still able to configure the build with -DCMAKE_DISABLE_FIND_PACKAGE_Git=ON.

Could mention in the pull description? Also, it looks like the tail needs editing? Ref:

This PR handles with both mentioned cases automagically and gets rid of BITCOIN_GENBUILD_NO_GIT. This PR automagically handles both of the aforementioned cases and removes the need for BITCOIN_GENBUILD_NO_GIT.

Thanks! Updated.
maflcko closed this on Apr 7, 2025
maflcko reopened this on Apr 7, 2025
hebasto marked this as a draft on Apr 7, 2025
hebasto force-pushed on Apr 7, 2025
hebasto marked this as ready for review on Apr 7, 2025
whitslack commented at 1:45 PM on April 7, 2025: contributor

The user is still able to configure the build with -DCMAKE_DISABLE_FIND_PACKAGE_Git=ON to disable git execution manually, for reasons we don't know in advance.

But that won't work, will it? The find_package(Git) call is in cmake/script/GenerateBuildInfo.cmake, which is executed recursively in an independent CMake process by the generate_build_info custom target of src/CMakeLists.txt. (Did we really learn nothing from "Recursive Make Considered Harmful"? :man_facepalming:) The only reason BITCOIN_GENBUILD_NO_GIT was effective in the sub-CMake process is because the environment is inherited by child processes. CMake variable definitions are not inherited.
hebasto force-pushed on Apr 7, 2025
hebasto commented at 2:23 PM on April 7, 2025: member

@whitslack

The user is still able to configure the build with -DCMAKE_DISABLE_FIND_PACKAGE_Git=ON to disable git execution manually, for reasons we don't know in advance.

But that won't work, will it? The find_package(Git) call is in cmake/script/GenerateBuildInfo.cmake, which is executed recursively in an independent CMake process by the generate_build_info custom target of src/CMakeLists.txt. (Did we really learn nothing from "Recursive Make Considered Harmful"? 🤦‍♂️) The only reason BITCOIN_GENBUILD_NO_GIT was effective in the sub-CMake process is because the environment is inherited by child processes. CMake variable definitions are not inherited.

Thank you for your feedback!

Reworked.
maflcko added the label DrahtBot Guix build requested on Apr 16, 2025
maflcko removed this from the milestone 29.0 on Apr 17, 2025

Type	Reviewers
Concept NACK	luke-jr
Concept ACK	theuni, fanquake

DrahtBot commented at 4:58 AM on April 18, 2025: contributor

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

File	commit bfeacc18b36132ba8ac70142133cd6c0e63b6763<br>(master)	commit 44cc45fad482d389d6e17ccbdeb2c9705c171a07<br>(pull/32220/merge)
*-aarch64-linux-gnu-debug.tar.gz	`86fc8d611619fdb8...`	`be7719d7dbfdd6d5...`
*-aarch64-linux-gnu.tar.gz	`a6492217b6b3bdcd...`	`c335f390126366cb...`
*-arm-linux-gnueabihf-debug.tar.gz	`c971aaa95e08b174...`	`0b9306fa34e5e778...`
*-arm-linux-gnueabihf.tar.gz	`d1f629c7542c6d4d...`	`a70fb8ceaa1882d2...`
*-arm64-apple-darwin-codesigning.tar.gz	`49ed3eb24526e927...`	`7ddd4f526a844195...`
*-arm64-apple-darwin-unsigned.tar.gz	`21bc8eb1ab756650...`	`368061ea22431be0...`
*-arm64-apple-darwin-unsigned.zip	`ecb605f720eeb415...`	`43a34ce5677be8ad...`
*-powerpc64-linux-gnu-debug.tar.gz	`30cc378004bb7754...`	`9fe33adfb084524a...`
*-powerpc64-linux-gnu.tar.gz	`0cec50135de35e74...`	`24ed680d63a0f6ff...`
*-riscv64-linux-gnu-debug.tar.gz	`102a576d8b5643d6...`	`f2d0e11d5fa2fd2c...`
*-riscv64-linux-gnu.tar.gz	`3fda9a48201f7574...`	`c8ceb8d0f60660b2...`
*-x86_64-apple-darwin-codesigning.tar.gz	`86b5ef5bfc3003b4...`	`e8b0600ff551e410...`
*-x86_64-apple-darwin-unsigned.tar.gz	`8944c1cc9a0411bd...`	`02ca66a520216fc0...`
*-x86_64-apple-darwin-unsigned.zip	`43f9c4bfe8f2fed8...`	`e1f44a5670bd4ff2...`
*-x86_64-linux-gnu-debug.tar.gz	`674b63be4f3dc2a7...`	`9c8e690b4107138e...`
*-x86_64-linux-gnu.tar.gz	`5e63f16f3fef1fc9...`	`d2be2dd647a2dff1...`
*.tar.gz	`c4f2b87871ac0196...`	`8b38cc7d7435cfbe...`
SHA256SUMS.part	`19e25c98174ba2bf...`	`abacaca071e7c1c3...`
guix_build.log	`836f59e077fc04f1...`	`18532b8ffd8a042f...`
guix_build.log.diff		`778e1886d381b11f...`

DrahtBot removed the label DrahtBot Guix build requested on Apr 18, 2025
DrahtBot added the label CI failed on Apr 29, 2025
DrahtBot removed the label CI failed on May 2, 2025
hebasto force-pushed on May 9, 2025
hebasto force-pushed on Oct 13, 2025
hebasto commented at 3:34 PM on October 13, 2025: member

Rebased to refresh the CI.
theuni commented at 2:26 PM on May 7, 2026: member

Concept ACK and quick review ACK. Nice to have git archive setting this.

And disabling git from CMake seems like a perfectly reasonable opt-out to me.
fanquake commented at 10:26 AM on May 8, 2026: member

Concept ACK

cmake, refactor: Move `find_package(Git)` to `src/CMakeLists.txt`

This change allows the use of the `CMAKE_DISABLE_FIND_PACKAGE_Git`
variable to explicitly disable git usage.

9a2cced23a

cmake: Remove unnecessary `BITCOIN_GENBUILD_NO_GIT` environment variable

This functionality is now covered by the `CMAKE_DISABLE_FIND_PACKAGE_Git`
variable.

Note for reviewers: consider reviewing with `git diff -w`.

fe941938e8

cmake: Skip using git when building from source tarball or as subproject b71cd5c162
doc: Add release notes for #32220 3142e5f8cf

in src/CMakeLists.txt:41 in 1292aab222

  36 | @@ -37,9 +37,14 @@ if(NOT CMAKE_ARCHIVE_OUTPUT_DIRECTORY)
  37 |    set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/lib)
  38 |  endif()
  39 |  
  40 | +set(IS_SOURCE_TARBALL FALSE)
  41 | +# git will put "set(IS_SOURCE_TARBALL TRUE)" on the next line inside archives. $Format:%nset(IS_SOURCE_TARBALL TRUE)$

maflcko commented at 7:17 PM on May 13, 2026:

style nit: maybe rebase on top of faf99ae3796 and fix it here as well

hebasto commented at 1:46 PM on May 14, 2026:

Thanks. Done.

hebasto force-pushed on May 14, 2026
hebasto commented at 1:46 PM on May 14, 2026: member

style nit: maybe rebase on top of faf99ae and fix it here as well

Rebased.

Release notes have been added.
DrahtBot added the label CI failed on May 14, 2026