Merge pull request #2174 from jamescowens/fix_assert_nonexistent_datadir

gui: Fix assert on non-existent data directory and GUI datadir chooser corner case issues

src/qt/bitcoin.cpp

@@ -274,20 +274,10 @@ int main(int argc, char *argv[])
         tfm::format(std::cerr, "Error parsing command line arguments: %s\n", error);
         return EXIT_FAILURE;
     }
+
     /** Check mainnet config file first in case testnet is set there and not in command line args **/
     SelectParams(CBaseChainParams::MAIN);
 
-    // Currently unused.
-    std::string error_msg;
-
-    if (!gArgs.ReadConfigFiles(error_msg, true)) {
-        ThreadSafeMessageBox(strprintf("Error reading configuration file.\n"),
-                "", CClientUIInterface::ICON_ERROR | CClientUIInterface::OK | CClientUIInterface::MODAL);
-        QMessageBox::critical(nullptr, PACKAGE_NAME,
-            QObject::tr("Error: Cannot parse configuration file."));
-        return EXIT_FAILURE;
-    }
-
 #ifdef Q_OS_WIN
     // Use Qt's built-in FreeType rendering engine to display text on Windows.
     // We use the Inter font's OpenType format which doesn't render clearly on
@@ -389,6 +379,17 @@ int main(int argc, char *argv[])
     // Gracefully exit if the user cancels
     if (!Intro::showIfNeeded(did_show_intro)) return EXIT_SUCCESS;
 
+    // Not currently useful.
+    std::string error_msg;
+
+    if (!gArgs.ReadConfigFiles(error_msg, true)) {
+        ThreadSafeMessageBox(strprintf("Error reading configuration file.\n"),
+                "", CClientUIInterface::ICON_ERROR | CClientUIInterface::OK | CClientUIInterface::MODAL);
+        QMessageBox::critical(nullptr, PACKAGE_NAME, QObject::tr("Error: Cannot read configuration file. Please check the "
+                                                                 "path and format of the file."));
+        return EXIT_FAILURE;
+    }
+
     // Do this to pickup -testnet from the command line.
     SelectParams(gArgs.IsArgSet("-testnet") ? CBaseChainParams::TESTNET : CBaseChainParams::MAIN);
 

src/qt/intro.cpp

@@ -166,19 +166,25 @@ void Intro::setDataDirectory(const QString &dataDir)
 
 bool Intro::showIfNeeded(bool& did_show_intro)
 {
-    did_show_intro = false;
-
     QSettings settings;
-    /* If data directory provided on command line, no need to look at settings
-       or show a picking dialog */
-    if(!gArgs.GetArg("-datadir", "").empty())
+    // If data directory provided on command line AND -choosedatadir is not specified, no need to look at settings or
+    // show a picking dialog. The -choosedatadir test is required because showIfNeeded is called after the initialization
+    // of the optionsModel, which will populate the datadir from the GUI settings if present. Without it, you would then
+    // not be able to get the chooser dialog to come up again once a datadir is stored in the GUI settings config file.
+    if (!gArgs.GetArg("-datadir", "").empty() && !gArgs.GetBoolArg("-choosedatadir", DEFAULT_CHOOSE_DATADIR)) {
         return true;
+    }
     /* 1) Default data directory for operating system */
     QString dataDir = GUIUtil::getDefaultDataDirectory();
     /* 2) Allow QSettings to override default dir */
     dataDir = settings.value("dataDir", dataDir).toString();
 
-    if(!fs::exists(GUIUtil::qstringToBoostPath(dataDir))
+    // This is necessary to handle the case where a user has changed the name of the data directory from a non-default
+    // back to the default, and wants to use the chooser via -choosedatadir to rechoose the data directory and have it
+    // properly respected without having to restart after the choosing.
+    bool originally_not_default_datadir = (dataDir != GUIUtil::getDefaultDataDirectory());
+
+    if (!fs::exists(GUIUtil::qstringToBoostPath(dataDir))
             || gArgs.GetBoolArg("-choosedatadir", DEFAULT_CHOOSE_DATADIR)
             || settings.value("fReset", false).toBool()
             || gArgs.GetBoolArg("-resetguisettings", false))
@@ -197,9 +203,9 @@ bool Intro::showIfNeeded(bool& did_show_intro)
         intro.setWindowIcon(QIcon(":/images/gridcoin"));
         did_show_intro = true;
 
-        while(true)
+        while (true)
         {
-            if(!intro.exec())
+            if (!intro.exec())
             {
                 /* Cancel clicked */
                 return false;
@@ -222,8 +228,10 @@ bool Intro::showIfNeeded(bool& did_show_intro)
      * override -datadir in the bitcoin.conf file in the default data directory
      * (to be consistent with bitcoind behavior)
      */
-    if (dataDir != GUIUtil::getDefaultDataDirectory()) {
-        gArgs.SoftSetArg("-datadir", GUIUtil::qstringToBoostPath(dataDir).string()); // use OS locale for path setting
+    if (dataDir != GUIUtil::getDefaultDataDirectory() || originally_not_default_datadir) {
+        // This must be a ForceSetArg, because the optionsModel has already loaded the datadir argument if it exists in
+        // the Qt settings file prior to this.
+        gArgs.ForceSetArg("-datadir", GUIUtil::qstringToBoostPath(dataDir).string()); // use OS locale for path setting
     }
 
     return true;

src/util/system.cpp

@@ -339,12 +339,11 @@ const fs::path& ArgsManager::GetDataDirPath(bool net_specific) const
     if (net_specific)
         path /= BaseParams().DataDir();
 
-    /* Reserved for future Bitcoin backport functionality with wallets.
     if (fs::create_directories(path)) {
         // This is the first run, create wallets subdirectory too
-        fs::create_directories(path / "wallets");
+        // Reserved for when we move wallets to a subdir like Bitcoin
+        //fs::create_directories(path / "wallets");
     }
-    */
 
     path = StripRedundantLastElementsOfPath(path);
     return path;
@@ -712,7 +711,14 @@ fs::path AbsPathForConfigVal(const fs::path& path, bool net_specific)
     if (path.is_absolute()) {
         return path;
     }
-    return fsbridge::AbsPathJoin(GetDataDir(net_specific), path);
+
+    fs::path data_dir = GetDataDir(net_specific);
+
+    if (data_dir.empty()) {
+        return fs::path {};
+    } else {
+    return fsbridge::AbsPathJoin(data_dir, path);
+    }
 }
 
 fs::path GetDefaultDataDir()
@@ -869,7 +875,10 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
 
     const std::string confPath = GetArg("-conf", GRIDCOIN_CONF_FILENAME);
 
-    fsbridge::ifstream stream(GetConfigFile(confPath));
+    fs::path config_file_spec = GetConfigFile(confPath);
+    if (config_file_spec.empty()) return false;
+
+    fsbridge::ifstream stream(config_file_spec);
 
     // ok to not have a config file
     if (stream.good()) {