On master the gen-bitcoin-conf script doesn’t work as it cannot find bitcoind. This is because of the build dir that is now being used since cmake.
This PR fixes it.
master
0./gen-bitcoin-conf.sh
1/home/marnix/projects/bitcoin/src/bitcoind not found or not executable.
PR
0./gen-bitcoin-conf.sh
1Generating example bitcoin.conf file in share/examples/
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31332.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | lucasbalieiro, BrandonOdiwuor, jonatack |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
Hi @MarnixCroes!
tACK b3a96b3
This PR addresses an issue introduced by the new cmake build process, which places the build output in the ./build directory. Specifically, it updates the BUILDDIR path in the script to point to this directory, ensuring compatibility with in-tree builds.
This behavior is consistent with the instructions outlined in contrib/devtools/README.md:
With in-tree builds this tool can be run from any directory within the
repository. To use this tool with out-of-tree builds setBUILDDIR. For
example:0BUILDDIR=$PWD/build contrib/devtools/gen-bitcoin-conf.sh
It’s worth noting that this PR conflicts with #31161 by @hebasto, which proposes a more holistic solution to this issue.
If Hebasto’s PR is merged first, this PR could potentially break those changes. It might be beneficial to coordinate between these two PRs to avoid introducing unnecessary conflicts or redundant solutions.
I was able to reproduce the issue on the master branch. The script fails to locate bitcoind due to the directory discrepancy caused by the cmake build process. Evidence of the issue can be seen in the screenshot below:
Testing the PR resolved the issue, and the script successfully generated the bitcoin.conf file in the correct location. Evidence of the fix is shown in the screenshot below:
To ensure stability, I rebuilt the entire Bitcoin Core project with the changes from this PR and encountered no issues. Everything worked as expected.
This PR effectively resolves the issue in its current context, but careful coordination with #31161 is necessary to avoid conflicts or redundant fixes. Great work on the implementation!
contrib/devtools/README.md needs to be updated to mention this as then the script would no longer work for in-tree builds (if we ever do that with cmake, but I have asked for it before).
Furthermore, that documentation is pretty clear that out of tree builds should set BUILDDIR, and with cmake, all builds will be out of tree, so I’m not sure that this change is actually useful.
