qml: Stabilize onboarding settings and data directories

.github/workflows/artifacts.yml

@@ -44,7 +44,7 @@ jobs:
             build-essential ccache cmake pkgconf \
             libboost-dev libsqlite3-dev libgl-dev libqrencode-dev \
             qt6-base-dev qt6-tools-dev qt6-l10n-tools qt6-tools-dev-tools \
-            qt6-declarative-dev qml6-module-qt-labs-settings qml6-module-qtquick qml6-module-qtquick-controls qml6-module-qtquick-dialogs qml6-module-qtquick-layouts qml6-module-qtquick-templates qml6-module-qtqml
+            qt6-declarative-dev qml6-module-qt-labs-platform qml6-module-qt-labs-settings qml6-module-qtquick qml6-module-qtquick-controls qml6-module-qtquick-dialogs qml6-module-qtquick-layouts qml6-module-qtquick-templates qml6-module-qtqml
           echo "CCACHE_DIR=${{ runner.temp }}/ccache" >> "$GITHUB_ENV"
 
       - name: Restore Ccache cache

qml/bitcoin.cpp

@@ -69,6 +69,7 @@
 #endif
 #include <util/fs.h>
 #include <util/fs_helpers.h>
+#include <util/string.h>
 #include <util/threadnames.h>
 #include <util/translation.h>
 #ifdef ENABLE_WALLET
@@ -77,6 +78,7 @@
 
 #include <cassert>
 #include <memory>
+#include <optional>
 #include <tuple>
 #include <vector>
 
@@ -89,6 +91,7 @@
 #include <QPixmap>
 #include <QGuiApplication>
 #include <QJSEngine>
+#include <QMessageBox>
 #include <QPointer>
 #include <QQmlApplicationEngine>
 #include <QQmlContext>
@@ -300,8 +303,51 @@ enum class PreInitOnboardingStatus {
     FAILED,
 };
 
+bool ErrorSettingsRead(
+    const bilingual_str& error,
+    const std::vector<std::string>& details,
+    std::optional<QmlOnboardingSettings::SettingsFileBackup>& settings_file_backup,
+    QString& settings_file_backup_error)
+{
+    const auto capture_backup = [&] {
+        QmlOnboardingSettings::SettingsFileBackup backup;
+        if (!QmlOnboardingSettings::CaptureSettingsFileBackup(
+                gArgs,
+                backup,
+                &settings_file_backup_error)) {
+            return false;
+        }
+        settings_file_backup = std::move(backup);
+        return true;
+    };
+
+    if (gArgs.GetBoolArg("-resetguisettings", false)) return !capture_backup();
+
+    QMessageBox message_box{
+        QMessageBox::Critical,
+        CLIENT_NAME,
+        QString::fromStdString(strprintf("%s.", error.translated)),
+        QMessageBox::Reset | QMessageBox::Abort,
+    };
+    /*: Explanatory text shown on startup when the settings file cannot be read.
+      Prompts user to make a choice between resetting or aborting. */
+    message_box.setInformativeText(QObject::tr("Do you want to reset settings to default values, or to abort without making changes?"));
+    message_box.setDetailedText(QString::fromStdString(util::MakeUnorderedList(details)));
+    message_box.setTextFormat(Qt::PlainText);
+    message_box.setDefaultButton(QMessageBox::Reset);
+    switch (message_box.exec()) {
+    case QMessageBox::Reset:
+        return !capture_backup();
+    case QMessageBox::Abort:
+        return true;
+    default:
+        assert(false);
+    }
+}
+
 struct PreInitOnboardingContext {
     std::unique_ptr<OnboardingOptionsModel> onboarding_options_model;
+    std::optional<QmlOnboardingSettings::PendingApply> pending_apply;
     QScopedPointer<const NetworkStyle> network_style;
     std::unique_ptr<QQmlApplicationEngine> engine;
 #ifdef ENABLE_TEST_AUTOMATION
@@ -328,11 +374,20 @@ bool ShouldShowPreInitOnboarding(const std::vector<std::string>& argv, bool can_
     const QmlOnboardingSettings::OnboardingStartupStatus status{
         QmlOnboardingSettings::ResolveOnboardingStartupStatus(argv, can_listen_ipc)
     };
+    if (status.settings_file_unreadable) {
+        return false;
+    }
     return !status.ok || status.should_show_onboarding;
 }
 
 PreInitOnboardingStatus RunPreInitOnboarding(PreInitOnboardingContext& context, const std::vector<std::string>& argv, bool can_listen_ipc)
 {
+    // Let InitConfig report invalid explicit datadirs with Core's standard
+    // error instead of entering an onboarding preview that cannot override
+    // the command line.
+    if (!QmlDataDir::ValidateExplicitDataDir(gArgs).isEmpty()) {
+        return PreInitOnboardingStatus::NOT_SHOWN;
+    }
     if (!ShouldShowPreInitOnboarding(argv, can_listen_ipc)) {
         QmlDataDir::ApplyGuiDataDirSetting(gArgs);
         return PreInitOnboardingStatus::NOT_SHOWN;
@@ -384,11 +439,13 @@ PreInitOnboardingStatus RunPreInitOnboarding(PreInitOnboardingContext& context,
     }
 
     QString error;
-    if (!context.onboarding_options_model->applyToArgs(gArgs, &error)) {
+    QmlOnboardingSettings::PendingApply pending_apply;
+    if (!context.onboarding_options_model->prepareApplyToArgs(gArgs, pending_apply, &error)) {
         InitError(Untranslated(error.toStdString()));
         context.close();
         return PreInitOnboardingStatus::FAILED;
     }
+    context.pending_apply = std::move(pending_apply);
     return PreInitOnboardingStatus::COMPLETED;
 }
 } // namespace
@@ -469,13 +526,9 @@ int QmlGuiMain(int argc, char* argv[])
 
     app.setQuitOnLastWindowClosed(false);
     setupChainQSettings(&app, QString::fromStdString(gArgs.GetChainTypeString()).toUpper());
-    if (gArgs.GetBoolArg("-resetguisettings", false)) {
-        QString reset_error;
-        if (!QmlDataDir::ResetGuiSettings(gArgs, &reset_error)) {
-            InitError(Untranslated(reset_error.toStdString()));
-            return EXIT_FAILURE;
-        }
-    }
+    const QmlOnboardingSettings::GuiSettingsStore bootstrap_gui_settings{
+        QmlOnboardingSettings::CurrentGuiSettingsStore()
+    };
 
     LoadFontResource(":/fonts/bitcoincoresans/regular");
     LoadFontResource(":/fonts/bitcoincoresans/semibold");
@@ -514,28 +567,37 @@ int QmlGuiMain(int argc, char* argv[])
         break;
     }
 
+    std::optional<QmlOnboardingSettings::SettingsFileBackup> settings_file_backup;
+    QString settings_file_backup_error;
     if (auto error = common::InitConfig(
             gArgs,
-            [](const bilingual_str& msg, const std::vector<std::string>& details) {
-                return InitError(msg, details);
+            [&](const bilingual_str& message, const std::vector<std::string>& details) {
+                return ErrorSettingsRead(
+                    message,
+                    details,
+                    settings_file_backup,
+                    settings_file_backup_error);
             })) {
+        if (!settings_file_backup_error.isEmpty()) {
+            InitError(Untranslated(settings_file_backup_error.toStdString()));
+        } else if (error->status != common::ConfigStatus::ABORTED) {
+            InitError(error->message, error->details);
+        }
         return EXIT_FAILURE;
     }
 
-    const QmlLegacySettings::MigrationResult legacy_migration{
-        QmlLegacySettings::MigrateCoreSettings(gArgs, QmlLegacySettings::MigrationMode::Persist)
-    };
-    if (!legacy_migration.error.isEmpty()) {
-        InitError(Untranslated(legacy_migration.error.toStdString()));
+    setupChainQSettings(&app, QString::fromStdString(gArgs.GetChainTypeString()).toUpper());
+    QString finalize_settings_error;
+    if (!QmlOnboardingSettings::FinalizeStartupSettings(
+            gArgs,
+            bootstrap_gui_settings,
+            pre_init_onboarding_context.pending_apply ? &*pre_init_onboarding_context.pending_apply : nullptr,
+            /*result=*/nullptr,
+            &finalize_settings_error,
+            settings_file_backup ? &*settings_file_backup : nullptr)) {
+        InitError(Untranslated(finalize_settings_error.toStdString()));
         return EXIT_FAILURE;
     }
-    if (legacy_migration.settings_changed) {
-        std::vector<std::string> settings_errors;
-        if (!gArgs.WriteSettingsFile(&settings_errors)) {
-            InitError(_("Settings file could not be written"), settings_errors);
-            return EXIT_FAILURE;
-        }
-    }
 
     // legacy GUI: parameterSetup()
     // Default printtoconsole to false for the GUI. GUI programs should not
@@ -604,15 +666,6 @@ int QmlGuiMain(int argc, char* argv[])
 
     ChainModel chain_model{*chain};
     chain_model.setCurrentNetworkName(QString::fromStdString(gArgs.GetChainTypeString()));
-    setupChainQSettings(&app, chain_model.currentNetworkName());
-    // Settings reset must happen before model instantiation so the models
-    // read clean defaults from QSettings.
-    if (gArgs.IsArgSet("-resetguisettings")) {
-        QSettings settings;
-        settings.remove(QStringLiteral("fHideTrayIcon"));
-        settings.remove(QStringLiteral("fMinimizeToTray"));
-        settings.remove(QStringLiteral("fMinimizeOnClose"));
-    }
 
     QObject::connect(&node_model, &NodeModel::setTimeRatioList, &chain_model, &ChainModel::setTimeRatioList);
     QObject::connect(&node_model, &NodeModel::setTimeRatioListInitial, &chain_model, &ChainModel::setTimeRatioListInitial);

qml/controls/qmldir

@@ -1 +1,3 @@
 singleton Theme 1.0 Theme.qml
+AppSettings 1.0 AppSettings.qml
+AppFolderDialog 1.0 AppFolderDialog.qml

qml/datadir.cpp

@@ -19,6 +19,7 @@
 #include <QSettings>
 #include <QUrl>
 
+#include <exception>
 #include <string>
 #include <vector>
 
@@ -108,11 +109,21 @@ QString ValidateCustomDataDir(const QString& path)
     }
 
     QFileInfo target_info(local_path);
-    if (target_info.exists() && !target_info.isDir()) {
-        return DataDirTr("The selected path exists and is not a directory.");
-    }
-    if (target_info.exists() && !target_info.isWritable()) {
-        return DataDirTr("The selected directory is not writable.");
+    if (target_info.exists()) {
+        if (!target_info.isDir()) {
+            return DataDirTr("The selected path exists and is not a directory.");
+        }
+        bool accessible{target_info.isReadable()};
+#ifndef Q_OS_WIN
+        accessible = accessible && target_info.isExecutable();
+#endif
+        if (!accessible) {
+            //: Error shown during onboarding when the selected data directory cannot be read or entered.
+            return DataDirTr("The selected directory is not accessible.");
+        }
+        if (!target_info.isWritable()) {
+            return DataDirTr("The selected directory is not writable.");
+        }
     }
 
     QString parent_path = target_info.absoluteDir().absolutePath();
@@ -225,51 +236,23 @@ void PersistDefaultDataDirSelection()
     QmlLegacySettings::ClearLegacyGuiSettings(QString::fromStdString(Params().GetChainTypeString()));
 }
 
-bool ResetGuiSettings(ArgsManager& args, QString* error)
+bool HasExplicitDataDirArg(const ArgsManager& args)
 {
-    if (error) error->clear();
-
-    QSettings settings;
-    settings.clear();
-    settings.setValue(RESET_GUI_SETTINGS_KEY, false);
+    return args.IsArgSet("-datadir") && !args.GetPathArg("-datadir").empty();
+}
 
+QString ValidateExplicitDataDir(const ArgsManager& args)
+{
+    if (!HasExplicitDataDirArg(args)) return {};
     try {
-        SelectParams(args.GetChainType());
-        args.SelectConfigNetwork(args.GetChainTypeString());
-        QmlLegacySettings::ClearLegacyGuiSettings(QString::fromStdString(args.GetChainTypeString()));
+        if (CheckDataDirOption(args)) return {};
     } catch (const std::exception& e) {
-        if (error) *error = QString::fromStdString(e.what());
-        return false;
+        return QString::fromStdString(e.what());
     }
 
-    fs::path settings_path;
-    if (!args.GetSettingsPath(&settings_path) || !fs::exists(settings_path)) {
-        return true;
-    }
-
-    std::vector<std::string> settings_errors;
-    if (!args.ReadSettingsFile(&settings_errors)) {
-        if (error) *error = QString::fromStdString(settings_errors.empty() ? std::string{"Settings file could not be read."} : settings_errors.front());
-        return false;
-    }
-    if (!args.WriteSettingsFile(&settings_errors, /*backup=*/true)) {
-        if (error) *error = QString::fromStdString(settings_errors.empty() ? std::string{"Settings file backup could not be written."} : settings_errors.front());
-        return false;
-    }
-    args.LockSettings([](common::Settings& settings) {
-        settings.rw_settings.clear();
-    });
-    settings_errors.clear();
-    if (!args.WriteSettingsFile(&settings_errors)) {
-        if (error) *error = QString::fromStdString(settings_errors.empty() ? std::string{"Settings file could not be written."} : settings_errors.front());
-        return false;
-    }
-    return true;
-}
-
-bool HasExplicitDataDirArg(const ArgsManager& args)
-{
-    return args.IsArgSet("-datadir") && !args.GetPathArg("-datadir").empty();
+    //: Startup error shown when a data directory explicitly supplied on the command line does not exist. %1 is the supplied path.
+    return DataDirTr("Specified data directory \"%1\" does not exist.")
+        .arg(QString::fromStdString(args.GetArg("-datadir", "")));
 }
 
 bool ShouldShowDataDirChooser(const ArgsManager& args)

qml/datadir.h

@@ -49,9 +49,9 @@ StorageSpaceResult CheckStorageSpace(const QString& path);
 bool EnsureDataDir(const QString& path, QString* error = nullptr);
 bool PersistGuiDataDirSelection(const QString& path, QString* error = nullptr);
 void PersistDefaultDataDirSelection();
-bool ResetGuiSettings(ArgsManager& args, QString* error = nullptr);
 
 bool HasExplicitDataDirArg(const ArgsManager& args);
+QString ValidateExplicitDataDir(const ArgsManager& args);
 bool ShouldShowDataDirChooser(const ArgsManager& args);
 bool ApplyGuiDataDirSetting(ArgsManager& args);
 bool ApplyDataDirArg(ArgsManager& args, const QString& path);