Make datadir handling more consistent

laanwj commented at 2:49 pm on February 7, 2014: member

Don’t error out with an exception when it is impossible to create the default data directory, but return an empty datadir just like when otherwise passing an invalid datadir. Add a friendly error message for when this really becomes a problem.
Move –help handling before data directory check and config file parsing.
Don’t check for existence of datadir in bitcoin-cli. It should be possible to run it without.
Don’t log in BitcoinApplication::~BitcoinApplication(), it’s very possible that the data directory isn’t set up. This avoids creating ~/.bitcoin on bitcoin-qt –help.

in src/util.cpp: in 4778ddd9ce outdated

1023@@ -1025,7 +1024,12 @@ void PrintExceptionContinue(std::exception* pex, const char* pszThread)
1024     if (fNetSpecific)
1025         path /= Params().DataDir();
1026 
1027-    fs::create_directories(path);
1028+    try {

sipa commented at 3:35 pm on February 9, 2014:

So, if we can’t create the requested datadir, we use "" (the current dir) ?

laanwj commented at 4:57 pm on February 9, 2014:

Well "" is simply an invalid error return value. We already return it if the -datadir is specified and doesn’t exist. At the beginning of bitcoind and -qt we check GetDataDir(False) for an invalid value and show an error if that is the case.

Edit: okay so indeed bitcoin-cli still needs to check this before calling ReadConfigFile() otherwise it may read it from the current directory

sipa commented at 5:07 pm on February 9, 2014: member

ACK

laanwj added this to the milestone 0.10.0 on Apr 23, 2014

Make datadir handling more consistent

- Don't error out with an exception when it is impossible to create
  the default data directory, but return an empty datadir just like
  when otherwise passing an invalid datadir.

- Move --help handling before data directory check and config
  file parsing.

- Don't force existence of datadir in bitcoin-cli. It
  should be possible to run it without.

- Don't log in BitcoinApplication::~BitcoinApplication(), it's very
  possible that the data directory isn't set up. This avoids creating
  ~/.bitcoin on bitcoin-qt --help.

b523a1a9a2

BitcoinPullTester commented at 9:37 am on May 10, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/b523a1a9a2cfe7da964859d05cffedf2f030e74d for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

in src/bitcoind.cpp: in b523a1a9a2

91+            } else {
92+                fprintf(stderr, "Error: Default data directory could not be created\n");
93+            }
94+            return false;
95+        }
96+

ajweiss commented at 6:10 pm on June 4, 2014:

Wouldn’t it make sense to attempt to read the config file before validating the data directory since the data directory can be specified in the config file? Consider the case where bitcoind is running as a daemon user with no homedir, and it is started with a -conf=…/bitcoin.conf where bitcoin.conf specifies the datadir. It will fail here when it is unable to create/validate ~/.bitcoin.

laanwj commented at 6:48 pm on June 4, 2014:

Yes, my fix is wrong here. It shouldn’t assume that the configuration file is in the data directory. I’m not confident that my changes preserve the current behavior in all edge cases, which is also why I haven’t merged this yet. It’s getting too complex. It may be better to document the behavior that we want, create test cases, and redesign the datadir handling according to that.

ajweiss commented at 7:59 pm on June 4, 2014:

Ok, I propose the following behavior:

For setting precedence, the following order is used (highest first):

Command line arguments
Configuration file settings
Defaults

Defaults (for config and datadir) are defined as:

datadir: ~/.bitcoin (or OS dependent equivalent) config: datadir/bitcoin.conf

The wiki mentions that specifying datadir in the config file is unsupported, so this would be a change, but ultimately I think it would make the most sense in light of being able to change data directories without having to change startup scripts and whatnot.

If you guys like the sound of this, I’ll put together a new PR with the changes and some tests… (and with some cleanup wrt GetDataDir() implicitly creating the datadir)

laanwj commented at 8:15 pm on June 4, 2014:

@ajweiss Sounds good to me, that has always been the precedence order for settings, but it’s surprisingly hard to get right for the datadir. Don’t forget that in the case of the GUI there’s a 2.5) Datadir from QSettings

And for -conf the precedence order is different and depends on -datadir being specified on the command line:

Command line argument -conf
bitcoin.conf in -datadir IF that option is provided on command line
bitcoin.conf in default datadir

…a -datadir in the configuration file will obviously not have an effect on -conf :)

laanwj commented at 8:47 am on June 10, 2014: member

Closing this as I don’t like this solution anymore.

laanwj closed this on Jun 10, 2014

DrahtBot locked this on Sep 8, 2021

Make datadir handling more consistent #3640