contrib: don't use the default datadir in gen-bitcoin-conf.sh - #329
Conversation
|
ACK 49db8d1. I reviewed the changes to the bash script and found them well written and robust. I tested using the following commands on Ubuntu 24.04 on an ARM CPU: And then verified that the following file was created and contained the expected content: |
Shouldn't we fix this? |
|
Yes. Opened #341, which moves the With that in, the |
49db8d1 to
4f5c538
Compare
|
Rebased onto v29.0 as a daggy fix and force-pushed. |
Fixes #304.
gen-bitcoin-conf.shinvokesbitcoind --versionandbitcoind --help. Both of these runcommon::InitConfig()(src/bitcoind.cpp:122) before the-help/-versionearly return (src/bitcoind.cpp:138), despite the comment there claiming help and version are processed "before taking care about datadir". So each invocation touches the default datadir:fs::create_directories(base_path / "wallets")(src/common/init.cpp:58) creates$HOME/.bitcoinfs::exists(base_path / BITCOIN_CONF_FILENAME)(src/common/init.cpp:68) stats$HOME/.bitcoin/bitcoin.confWriteSettingsFile()(src/common/init.cpp:111) writes$HOME/.bitcoin/settings.jsonIf
$HOME/.bitcoinexists but is not traversable, thefs::existscall throwsfilesystem_erroronEACCESand both invocations fail, as @pdath reproduced in the issue.Point
bitcoindat a throwaway datadir instead. Note that-noconfalone is not sufficient: thefs::exists()call above runs regardless of whether a config file is in use, and-nosettingsdoes not help either. Only-datadiravoids all three accesses. Verified by running each variant against a build of this tree.Also add
set -eo pipefail, per the second half of the issue. Previously a failingbitcoindwas not detected at all: theVERSION_OUTPUT=$(...)assignment went unchecked, and the exit status of thebitcoind --help | sed | ... | sedpipeline was that of the trailingsed. The script printedbitcoind's errors and then exited 0 with a truncatedbitcoin.conf, which is the failure mode seen in the issue report.Bare
mktemp -dmatches the existing usage incontrib/devtools/check-deps.shand is portable to the BSDs, whosemktempbehaves "as if-t tmpwas supplied" when only-dis passed.Testing
Built with
-DWITH_ZMQ=ON -DWITH_MINIUPNPC=ONand confirmed the patched script regeneratesshare/examples/bitcoin.confbyte-for-byte identically to the committed file, so this change is output-neutral.bitcoind --helpoutput is unchanged by-datadir.Comparing against the script before this change:
$HOME.bitcoin/,wallets/,settings.json$HOMEchmod 444 ~/.bitcoin(issue #304)Permission denied, but exits 0bitcoind --versionfailsbitcoind --helpfailsbitcoindmissing / not executableThe temporary datadir is removed on exit.
test/lint/lint-shell.pypasses.One pre-existing wart is left alone: if
bitcoindfails midway, the header has already been written andshare/examples/bitcoin.confis left truncated. That is now at least signalled by a non-zero exit status. Generating into a temporary file and renaming it into place on success would close it, if wanted.