#33828 Check required interfaces before generating manpages

ReviewabilityReviewsAgreementSize
#33828 151henry151 Check required interfaces before generating manpagesAuthor silent 155d0 (+1)Positive3 + 53 tests
  • Prevent committing incomplete manpages when building with features disabled
  • Ensures documentation tools require all expected build features before running
  • Author silent for 117 days so responses to new reviews may be delayed
  • Untested concept approval based on plausible initial code (maflcko)
  • Resolved objection regarding missing interface checks in build cache (fanquake)
  • Thinks it lacks review and may not be a priority (sedited)
  • 3 lines added or modified outside tests
  • 53 lines added or modified in tests
  • 0 lines removed in total
  • 2 files, 1 commits
Build and CI#44 of 56
P3 · bug fix
  • P3 because it addresses an edge case in release documentation tooling
  • Prevents maintainers from accidentally committing manpages with missing options
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:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
fanquake (member)objection, correctnessHAVE_SYSTEM is not stored in CMakeCache.txt, causing the manpage interface check to failfixed · fix pushed · resolved by reading from bitcoin-build-config.h · v1, not yet re-checkedchangedyes2026-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, approachmixing compile definitions and build header defines for the same symbol confuses the build configurationanswered · author asked for other maintainers to weigh in on build header conventions without resolution · v1, not yet re-checkedansweredyes2026-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

dossier JSON · extract JSON · model ledger, generated 2026-09-23T01:21, confidence high, input hash 10e82dfd85f22c60
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