#35072 cmake: Remove optional definitions from `bitcoin-build-config.h`
https://github.com/bitcoin/bitcoin/pull/35072 · · +12/-61 in 25 files, 5 commits · labels: Build system, Needs rebase, CI failed · draft
Goal
- Prevent unnecessary full project rebuilds when developers reconfigure CMake build options
- Scope configuration flags so changing one option only recompiles affected targets
This pull request moves optional definitions (`USE_DBUS`, `USE_QRCODE`, `ENABLE_WALLET`, `ENABLE_TRACING`, and `ENABLE_EXTERNAL_SIGNER`) out of `bitcoin-build-config.h` and scopes them directly to their consuming CMake targets. Unneeded inclusions of `bitcoin-build-config.h` are also removed from various source files.
Problem: When reconfiguring the build with new CMake options (such as enabling GUI builds after a non-GUI build), modifying `bitcoin-build-config.h` forces a full recompilation of all targets that include the header, even if they do not depend on the reconfigured options.
Category: Build and CI (#33 of 55)
P3 · speedup
- P3 because it reduces rebuild times for developers toggling options like GUI or tracing
- Provides iteration speedups but fine-tunes configuration without broader build improvements
- Enables follow-up work in #29914
The PR addresses developer rebuild times when reconfiguring CMake options, specifically preventing complete rebuilds of unrelated targets when enabling GUI or tracing options. While unblocking faster iteration and linked to issue #29914, build configuration fine-tuning without broader build system fixes qualifies as a worthwhile, deferrable P3 improvement.
Membership: Changes CMakeLists configuration, module definitions, and header generation in `cmake/bitcoin-build-config.h.in`.
Factors: security/stability 0, bug 0, performance 1, user value 0, leverage 1
Reviewability: Stale: Needs rebase
- Needs rebase and has failing CI
- Author converted to draft to rework design and has been inactive
The PR has merge conflicts, failing CI, and has had no author activity for 111 days after being turned into a draft.
Author status: silent since 2026-05-29 after converting the PR to draft to address reviewer feedback
Open concerns:
- fanquake concept NACKed the change, arguing that shared config headers are intended to trigger rebuilds on configuration changes, and questioning whether scoping definitions to targets makes sense standalone without a coherent overall design.
- 151henry151 noted that auxiliary tooling like `gen-manpages.py` relies on `bitcoin-build-config.h` as a single source of truth for enabled features.
Resolved concerns:
- BrandonOdiwuor verified that only Qt targets are rebuilt when `BUILD_GUI=ON` is subsequently added.
Agreement: Disputed
- Concept objection: config headers are intended to trigger rebuilds on configuration changes (fanquake)
- Questioned whether scoping flags to targets makes sense without an overall design (fanquake)
- Concern that auxiliary tools rely on the header as a single source of truth (151henry151)
- Verified that enabling GUI options avoids rebuilding unrelated targets (BrandonOdiwuor)
Disputed: fanquake concept NACKed removing optional definitions from the config header; author moved PR to draft.
fanquake raised a concept objection regarding the purpose of config headers and the lack of clear criteria for what belongs there, while BrandonOdiwuor provided a tested ACK. The author moved the PR to draft to address the concerns and has not resumed work.
- BrandonOdiwuor gave a Tested ACK verifying that non-GUI targets are not rebuilt when toggling GUI builds.
- fanquake gave a Concept NACK stating: 'I think the point of a (shared) config header, is to trigger rebuilds of modules that use it, when the header/config changes.'
- 151henry151 commented that tools like `gen-manpages.py` use `bitcoin-build-config.h` as a canonical reference for enabled features.
Review verdicts (DrahtBot): 0 (+1) -1
- Stale ACK: BrandonOdiwuor
- Concept NACK: fanquake
Dependencies
Enables:
Files
5 lines under test/bench/ci.
- cmake/bitcoin-build-config.h.in +0/-16
- src/qt/CMakeLists.txt +6/-1
- cmake/module/FindUSDT.cmake +2/-4
- src/CMakeLists.txt +3/-0
- CMakeLists.txt +0/-2
- src/common/run_command.cpp +0/-2
- src/node/interfaces.cpp +0/-2
- src/qt/bitcoin.h +0/-2
- src/qt/bitcoingui.h +0/-2
- src/qt/createwalletdialog.cpp +0/-2
- src/qt/notificator.cpp +0/-2
- src/qt/notificator.h +0/-2
- src/qt/optionsmodel.cpp +0/-2
- src/qt/qrimagewidget.cpp +0/-2
- src/qt/receiverequestdialog.cpp +0/-2
- src/qt/rpcconsole.h +0/-2
- src/qt/test/test_main.cpp +0/-2
- src/rpc/external_signer.cpp +0/-2
- src/rpc/register.h +0/-2
- src/test/system_tests.cpp +0/-2
- src/util/trace.h +0/-2
- src/wallet/rpc/addresses.cpp +0/-2
- src/wallet/rpc/wallet.cpp +0/-2
- src/wallet/walletdb.cpp +0/-2
- src/qt/test/CMakeLists.txt +1/-0
Card
This PR moves optional feature definitions (such as DBus, QR code, and tracing support) out of the centralized bitcoin-build-config.h header into target-specific CMake compile definitions. Its purpose is to prevent unnecessary full recompilations when CMake configuration options are toggled in an existing build directory. The change represents a developer workflow convenience (P3 in build). Review is currently blocked by merge conflicts, failing CI, and an unresolved Concept NACK from fanquake questioning the architectural shift away from a canonical configuration header.