#33828 Check required interfaces before generating manpages
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #33828 Check required interfaces before generating manpages | Author silent 155d | 0 (+1) | Positive | 3 + 53 tests | |||
|
|
|
| ||||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Check required build interfaces before running the manpage generator script. The script inspects the generated `bitcoin-build-config.h` header and aborts if `HAVE_SYSTEM`, `ENABLE_WALLET`, or `WITH_ZMQ` are missing, while also adding `WITH_ZMQ` to the CMake configuration template.
Problem: Manpages are generated directly from binary `--help` output. If a developer builds Bitcoin Core with certain features disabled (like the wallet or ZMQ) and runs `gen-manpages.py`, the generated documentation omits those options and can be mistakenly committed.
Evidence of importance:
- Fixes issue #17506, 'gen-manpages output depends on build options, so needs to check them'.
Reviewability
Stale · Author silent 155d
No author activity for 155 days; a review may wait a long time for a response.
Waiting on (from the thread read):
- reviewer: review, though sedited noted the PR lacks review and may not be a priority 2026-09-22
Agreement
Positive
support without stated reasons, no open objection (maflcko)
Thread notes: Added sedited's follow-up comment noting that the PR is not getting review and may not be a priority. Claim rc:2726653497 remains open awaiting reviewer input.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| fanquake (member) | objection, correctness | HAVE_SYSTEM is not stored in CMakeCache.txt, causing the manpage interface check to fail | fixed · fix pushed · resolved by reading from bitcoin-build-config.h · v1, not yet re-checked | changed | yes | 2026-01-23: 'HAVE_SYSTEM is never present in CMakeCache.txt, so I don't see how this could work.' Status from: 2026-01-24: 151henry151: 'The script now reads all three features (HAVE_SYSTEM, ENABLE_WALLET, and WITH_ZMQ) from bitcoin-build-config.h as the si' |
| maflcko (member) | objection, approach | mixing compile definitions and build header defines for the same symbol confuses the build configuration | answered · author asked for other maintainers to weigh in on build header conventions without resolution · v1, not yet re-checked | answered | yes | 2026-01-26: 'i don't think compile definitions and build header defines should be mixed for the same symbol. This makes it triply confusing' |
Support:
- maflcko (member) (ACK, 2026-01-22): I haven't tested this, but the initial code looks plausible [not substantive]
Participants:
- fanquake (member, objection, 3 statements 2025-11-10 to 2026-01-23): noted HAVE_SYSTEM is missing from CMakeCache.txt, breaking the initial implementation
- maflcko (member, objection, 13 statements 2026-01-12 to 2026-04-28): objected to mixing compile definitions and build header defines for WITH_ZMQ; gave an initial non-substantive ACK
- sedited (member, neutral, 2 statements 2026-08-10 to 2026-09-22): noted lack of review and that the PR may not be a priority
Files
53 lines under test/bench/ci.
- contrib/devtools/gen-manpages.py +53/-0
- cmake/bitcoin-build-config.h.in +3/-0
Card
The short text the ranking pass compares across PRs.
This PR updates contrib/devtools/gen-manpages.py to check bitcoin-build-config.h for required build options (HAVE_SYSTEM, ENABLE_WALLET, WITH_ZMQ) before generating manpages, adding an environment variable escape hatch to bypass the check. Because manpages are generated from binary help output, building without these features causes options to be omitted from the generated manpages. This resolves issue #17506 to prevent contributors from inadvertently committing incomplete documentation.
Data
thread read through 2026-09-22 (33 statements), head bd7c0497d2
Discussion read under schema 1 rules: suggestions and questions may be missing, and an objection may combine several harms (schema history).
labels: none
Categories
Build and CI
P3 · bug fix
Prevents mistakenly generating and committing incomplete manpages when features like wallet or ZMQ are disabled. As noted in issue #17506, `gen-manpages.py` output depends on build options, and checking for required build definitions provides a clear safeguard against committing incomplete documentation.
Membership: Modifies `cmake/bitcoin-build-config.h.in` and `contrib/devtools/gen-manpages.py`, which is tooling under contrib used for release documentation.
Factors: security/stability 0, bug 1, performance 0, user value 0, leverage 0