doc: gen-manpages.py should check build options #33085
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33085. ReviewsSee the guideline for information on the review process. LLM Linter (✨ experimental)Possible typos and grammar issues:
drahtbot_id_4_m |
f860622 to
70fe2a8
Compare
|
🚧 At least one of the CI tasks failed. HintsTry to run the tests locally, according to the documentation. However, a CI failure may still
Leave a comment here, if you need help tracking down a confusing failure. |
70fe2a8 to
db2c589
Compare
|
Personally I like the intent to structure the code more (the introduction of functions). Paused reviewing this PR:
(P.S. This is not meant to be harsh but to explain why I paused reviewing the code instead of silently ignoring the PR. This is an effort to either get the PR merged or closed, not leaving PR's in limbo.) |
|
@janb84 thanks for having a look I think introducing a main function would require a huge refactor to the file for such small check. I would also love to hear some advantages of adding a main function here |
| disabled_options = check_build_options() | ||
| if disabled_options and not args.skip_build_options_check: | ||
| error_msg = ( | ||
| "Aborting generating manpages...\n" |
There was a problem hiding this comment.
"Aborting generating manpages..." -> "Aborting generation of manpages..." [corrects ungrammatical gerund usage]
| @@ -27,6 +29,11 @@ | |||
| default=False, | |||
| help="skip generation for binaries that are not found in the build path", | |||
| ) | |||
| parser.add_argument( | |||
| '--skip-build-options-check', | |||
There was a problem hiding this comment.
in theory, --skip-missing-binaries can now be removed and the list of binaries derived from the build options.
| 'ENABLE_BITCOIN_CHAINSTATE': 'Build experimental bitcoin-chainstate executable', | ||
| 'ENABLE_FUZZ_BINARY': 'Build fuzz binary', |
There was a problem hiding this comment.
what is the point of listing those, which are unused?
Fixes #17506. Second attempt after PR: #29457 (closed)
This PR fixes the
gen-manpages.pyscript to check the build options and exit when there are missing components. It also adds an option to--skip-build-options-checkThe PR also addresses the issues raised on PR: #29457:
- #29457 (comment) - the default behaviors is fail
- #29457 (comment) - by adding
--skip-build-options-check- #29457 (review) - The PR uses
builddir/src/bitcoin-build-config.hto check forHAVE_SYSTEMoption andbuilddir/test/config.inifor the rest as they are only available there respectively