Merge #15864: Fix datadir handling

ffea41f530 Enable all tests in feature_config_args.py (Hennadii Stepanov)
66f5c17f8a Use CheckDataDirOption() for code uniformity (Hennadii Stepanov)
7e33a18a34 Fix datadir handling in bitcoin-cli (Hennadii Stepanov)
b28dada374 Fix datadir handling in bitcoin-qt (Hennadii Stepanov)
50824093bb Fix datadir handling in bitcoind (Hennadii Stepanov)
740d41ce9f Add CheckDataDirOption() function (Hennadii Stepanov)
c1f325126c Return absolute path early in AbsPathForConfigVal (Hennadii Stepanov)

Pull request description:

  Fix #15240, see: https://github.com/bitcoin/bitcoin/issues/15240#issuecomment-487353760
  Fix #15745
  Fix broken `feature_config_args.py` tests (disabled by MarcoFalke@fabe28a0cdcfa13e0e595a0905e3642a960d3077). All test are enabled now.
  This PR is alternative to #13621.

  User's `$HOME` directory is not touched unnecessarily now.

  ~To make reviewing easier only `bitcoind` code is modified (neither `bitcoin-cli` nor `bitcoin-qt`).~

  Refs:
  - https://github.com/bitcoin/bitcoin/issues/15745#issuecomment-479852569 by **laanwj**
  - #16220

Top commit has no ACKs.

Tree-SHA512: 4a4cda10e0b67c8f374da0c9567003d2b566d948e7f8550fe246868b5794c15010e88ea206009480b9cd2f737f310a15e984f920730448f99a895893bed351df

src/bitcoin-cli.cpp

@@ -125,7 +125,7 @@ static int AppInitRPC(int argc, char* argv[])
         }
         return EXIT_SUCCESS;
     }
-    if (!fs::is_directory(GetDataDir(false))) {
+    if (!CheckDataDirOption()) {
         tfm::format(std::cerr, "Error: Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", "").c_str());
         return EXIT_FAILURE;
     }

src/bitcoin-wallet.cpp

@@ -57,7 +57,7 @@ static bool WalletAppInit(int argc, char* argv[])
     // check for printtoconsole, allow -debug
     LogInstance().m_print_to_console = gArgs.GetBoolArg("-printtoconsole", gArgs.GetBoolArg("-debug", false));
 
-    if (!fs::is_directory(GetDataDir(false))) {
+    if (!CheckDataDirOption()) {
         tfm::format(std::cerr, "Error: Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", "").c_str());
         return false;
     }

src/bitcoind.cpp

@@ -95,8 +95,7 @@ static bool AppInit(int argc, char* argv[])
 
     try
     {
-        if (!fs::is_directory(GetDataDir(false)))
+        if (!CheckDataDirOption()) {
-        {
             return InitError(strprintf("Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", "")));
         }
         if (!gArgs.ReadConfigFiles(error, true)) {

src/qt/bitcoin.cpp

@@ -490,10 +490,9 @@ int GuiMain(int argc, char* argv[])
     if (!Intro::pickDataDirectory(*node))
         return EXIT_SUCCESS;
 
-    /// 6. Determine availability of data and blocks directory and parse bitcoin.conf
+    /// 6. Determine availability of data directory and parse bitcoin.conf
     /// - Do not call GetDataDir(true) before this step finishes
-    if (!fs::is_directory(GetDataDir(false)))
+    if (!CheckDataDirOption()) {
-    {
         node->initError(strprintf("Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", "")));
         QMessageBox::critical(nullptr, PACKAGE_NAME,
             QObject::tr("Error: Specified data directory \"%1\" does not exist.").arg(QString::fromStdString(gArgs.GetArg("-datadir", ""))));

src/util/system.cpp

@@ -748,8 +748,9 @@ const fs::path &GetDataDir(bool fNetSpecific)
     // this function
     if (!path.empty()) return path;
 
-    if (gArgs.IsArgSet("-datadir")) {
+    std::string datadir = gArgs.GetArg("-datadir", "");
-        path = fs::system_complete(gArgs.GetArg("-datadir", ""));
+    if (!datadir.empty()) {
+        path = fs::system_complete(datadir);
         if (!fs::is_directory(path)) {
             path = "";
             return path;
@@ -768,6 +769,12 @@ const fs::path &GetDataDir(bool fNetSpecific)
     return path;
 }
 
+bool CheckDataDirOption()
+{
+    std::string datadir = gArgs.GetArg("-datadir", "");
+    return datadir.empty() || fs::is_directory(fs::system_complete(datadir));
+}
+
 void ClearDatadirCache()
 {
     LOCK(csPathCached);
@@ -937,7 +944,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
 
     // If datadir is changed in .conf file:
     ClearDatadirCache();
-    if (!fs::is_directory(GetDataDir(false))) {
+    if (!CheckDataDirOption()) {
         error = strprintf("specified data directory \"%s\" does not exist.", gArgs.GetArg("-datadir", "").c_str());
         return false;
     }
@@ -1205,6 +1212,9 @@ int64_t GetStartupTime()
 
 fs::path AbsPathForConfigVal(const fs::path& path, bool net_specific)
 {
+    if (path.is_absolute()) {
+        return path;
+    }
     return fs::absolute(path, GetDataDir(net_specific));
 }
 

src/util/system.h

@@ -71,6 +71,8 @@ fs::path GetDefaultDataDir();
 // The blocks directory is always net specific.
 const fs::path &GetBlocksDir();
 const fs::path &GetDataDir(bool fNetSpecific = true);
+// Return true if -datadir option points to a valid directory or is not specified.
+bool CheckDataDirOption();
 /** Tests only */
 void ClearDatadirCache();
 fs::path GetConfigFile(const std::string& confPath);

test/functional/feature_config_args.py

@@ -109,17 +109,15 @@ class ConfArgsTest(BitcoinTestFramework):
             f.write("datadir=" + new_data_dir + "\n")
             f.write(conf_file_contents)
 
-        # Temporarily disabled, because this test would access the user's home dir (~/.bitcoin)
+        self.nodes[0].assert_start_raises_init_error(['-conf=' + conf_file], 'Error: Error reading configuration file: specified data directory "' + new_data_dir + '" does not exist.')
-        #self.nodes[0].assert_start_raises_init_error(['-conf=' + conf_file], 'Error reading configuration file: specified data directory "' + new_data_dir + '" does not exist.')
 
         # Create the directory and ensure the config file now works
         os.mkdir(new_data_dir)
-        # Temporarily disabled, because this test would access the user's home dir (~/.bitcoin)
+        self.start_node(0, ['-conf='+conf_file, '-wallet=w1'])
-        #self.start_node(0, ['-conf='+conf_file, '-wallet=w1'])
+        self.stop_node(0)
-        #self.stop_node(0)
+        assert os.path.exists(os.path.join(new_data_dir, 'regtest', 'blocks'))
-        #assert os.path.exists(os.path.join(new_data_dir, 'regtest', 'blocks'))
+        if self.is_wallet_compiled():
-        #if self.is_wallet_compiled():
+            assert os.path.exists(os.path.join(new_data_dir, 'regtest', 'wallets', 'w1'))
-        #assert os.path.exists(os.path.join(new_data_dir, 'regtest', 'wallets', 'w1'))
 
         # Ensure command line argument overrides datadir in conf
         os.mkdir(new_data_dir_2)