From c361df90b9fd34e50bbf1db43b866930e5498357 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Thu, 23 Feb 2023 14:58:49 -0500 Subject: [PATCH 1/4] scripted-diff: Remove double newlines after some init errors Some InitError calls had trailing \n characters, causing double newlines in error output. After this change InitError calls consistently output one newline instead of two. Appearance of messages in the GUI does not seem to be affected. Can be tested with: src/bitcoind -regtest -datadir=noexist src/qt/bitcoin-qt -regtest -datadir=noexist -BEGIN VERIFY SCRIPT- git grep -l InitError src/ | xargs sed -i 's/\(InitError(.*\)\\n"/\1"/' -END VERIFY SCRIPT- --- src/bitcoind.cpp | 14 +++++++------- src/qt/bitcoin.cpp | 12 ++++++------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index 6851f862976..4836b916331 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -120,7 +120,7 @@ static bool AppInit(NodeContext& node, int argc, char* argv[]) SetupServerArgs(args); std::string error; if (!args.ParseParameters(argc, argv, error)) { - return InitError(Untranslated(strprintf("Error parsing command line arguments: %s\n", error))); + return InitError(Untranslated(strprintf("Error parsing command line arguments: %s", error))); } // Process help and version before taking care about datadir @@ -151,22 +151,22 @@ static bool AppInit(NodeContext& node, int argc, char* argv[]) try { if (!CheckDataDirOption(args)) { - return InitError(Untranslated(strprintf("Specified data directory \"%s\" does not exist.\n", args.GetArg("-datadir", "")))); + return InitError(Untranslated(strprintf("Specified data directory \"%s\" does not exist.", args.GetArg("-datadir", "")))); } if (!args.ReadConfigFiles(error, true)) { - return InitError(Untranslated(strprintf("Error reading configuration file: %s\n", error))); + return InitError(Untranslated(strprintf("Error reading configuration file: %s", error))); } // Check for chain settings (Params() calls are only valid after this clause) try { SelectParams(args.GetChainName()); } catch (const std::exception& e) { - return InitError(Untranslated(strprintf("%s\n", e.what()))); + return InitError(Untranslated(strprintf("%s", e.what()))); } // Error out when loose non-argument tokens are encountered on command line for (int i = 1; i < argc; i++) { if (!IsSwitchChar(argv[i][0])) { - return InitError(Untranslated(strprintf("Command line contains unexpected token '%s', see bitcoind -h for a list of options.\n", argv[i]))); + return InitError(Untranslated(strprintf("Command line contains unexpected token '%s', see bitcoind -h for a list of options.", argv[i]))); } } @@ -210,7 +210,7 @@ static bool AppInit(NodeContext& node, int argc, char* argv[]) } break; case -1: // Error happened. - return InitError(Untranslated(strprintf("fork_daemon() failed: %s\n", SysErrorString(errno)))); + return InitError(Untranslated(strprintf("fork_daemon() failed: %s", SysErrorString(errno)))); default: { // Parent: wait and exit. int token = daemon_ep.TokenRead(); if (token) { // Success @@ -222,7 +222,7 @@ static bool AppInit(NodeContext& node, int argc, char* argv[]) } } #else - return InitError(Untranslated("-daemon is not supported on this operating system\n")); + return InitError(Untranslated("-daemon is not supported on this operating system")); #endif // HAVE_DECL_FORK } // Lock data directory after daemonization diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 99faa51ea0d..440f475757c 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -176,7 +176,7 @@ static bool InitSettings() if (!gArgs.ReadSettingsFile(&errors)) { std::string error = QT_TRANSLATE_NOOP("bitcoin-core", "Settings file could not be read"); std::string error_translated = QCoreApplication::translate("bitcoin-core", error.c_str()).toStdString(); - InitError(Untranslated(strprintf("%s:\n%s\n", error, MakeUnorderedList(errors)))); + InitError(Untranslated(strprintf("%s:\n%s", error, MakeUnorderedList(errors)))); QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error_translated)), QMessageBox::Reset | QMessageBox::Abort); /*: Explanatory text shown on startup when the settings file cannot be read. @@ -199,7 +199,7 @@ static bool InitSettings() if (!gArgs.WriteSettingsFile(&errors)) { std::string error = QT_TRANSLATE_NOOP("bitcoin-core", "Settings file could not be written"); std::string error_translated = QCoreApplication::translate("bitcoin-core", error.c_str()).toStdString(); - InitError(Untranslated(strprintf("%s:\n%s\n", error, MakeUnorderedList(errors)))); + InitError(Untranslated(strprintf("%s:\n%s", error, MakeUnorderedList(errors)))); QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error_translated)), QMessageBox::Ok); /*: Explanatory text shown on startup when the settings file could not be written. @@ -546,7 +546,7 @@ int GuiMain(int argc, char* argv[]) SetupUIArgs(gArgs); std::string error; if (!gArgs.ParseParameters(argc, argv, error)) { - InitError(strprintf(Untranslated("Error parsing command line arguments: %s\n"), error)); + InitError(strprintf(Untranslated("Error parsing command line arguments: %s"), error)); // Create a message box, because the gui has neither been created nor has subscribed to core signals QMessageBox::critical(nullptr, PACKAGE_NAME, // message cannot be translated because translations have not been initialized @@ -589,7 +589,7 @@ int GuiMain(int argc, char* argv[]) /// 6a. Determine availability of data directory if (!CheckDataDirOption(gArgs)) { - InitError(strprintf(Untranslated("Specified data directory \"%s\" does not exist.\n"), gArgs.GetArg("-datadir", ""))); + InitError(strprintf(Untranslated("Specified data directory \"%s\" does not exist."), gArgs.GetArg("-datadir", ""))); QMessageBox::critical(nullptr, PACKAGE_NAME, QObject::tr("Error: Specified data directory \"%1\" does not exist.").arg(QString::fromStdString(gArgs.GetArg("-datadir", "")))); return EXIT_FAILURE; @@ -598,7 +598,7 @@ int GuiMain(int argc, char* argv[]) /// 6b. Parse bitcoin.conf /// - Do not call gArgs.GetDataDirNet() before this step finishes if (!gArgs.ReadConfigFiles(error, true)) { - InitError(strprintf(Untranslated("Error reading configuration file: %s\n"), error)); + InitError(strprintf(Untranslated("Error reading configuration file: %s"), error)); QMessageBox::critical(nullptr, PACKAGE_NAME, QObject::tr("Error: Cannot parse configuration file: %1.").arg(QString::fromStdString(error))); return EXIT_FAILURE; @@ -613,7 +613,7 @@ int GuiMain(int argc, char* argv[]) // Check for chain settings (Params() calls are only valid after this clause) SelectParams(gArgs.GetChainName()); } catch(std::exception &e) { - InitError(Untranslated(strprintf("%s\n", e.what()))); + InitError(Untranslated(strprintf("%s", e.what()))); QMessageBox::critical(nullptr, PACKAGE_NAME, QObject::tr("Error: %1").arg(e.what())); return EXIT_FAILURE; } From 3db2874bd71d2391747b7385cabcbfef67218c4c Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Thu, 23 Feb 2023 15:29:19 -0500 Subject: [PATCH 2/4] Extend bilingual_str support for tinyformat Previous bilingual_str tinyformat::format accepted bilingual format strings, but not bilingual arguments. Extend it to accept both. This is useful when embedding one translated string inside another translated string, for example: `strprintf(_("Error: %s"), message)` which would fail previously if `message` was a bilingual_str. --- src/Makefile.test.include | 1 + src/test/translation_tests.cpp | 21 +++++++++++++++++++++ src/util/translation.h | 15 ++++++++++++++- test/lint/run-lint-format-strings.py | 1 + 4 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 src/test/translation_tests.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index fa77e287360..bac132767d7 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -147,6 +147,7 @@ BITCOIN_TESTS =\ test/timedata_tests.cpp \ test/torcontrol_tests.cpp \ test/transaction_tests.cpp \ + test/translation_tests.cpp \ test/txindex_tests.cpp \ test/txpackage_tests.cpp \ test/txreconciliation_tests.cpp \ diff --git a/src/test/translation_tests.cpp b/src/test/translation_tests.cpp new file mode 100644 index 00000000000..bda5dfd0994 --- /dev/null +++ b/src/test/translation_tests.cpp @@ -0,0 +1,21 @@ +// Copyright (c) 2023 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include + +#include + +BOOST_AUTO_TEST_SUITE(translation_tests) + +BOOST_AUTO_TEST_CASE(translation_namedparams) +{ + bilingual_str arg{"original", "translated"}; + bilingual_str format{"original [%s]", "translated [%s]"}; + bilingual_str result{strprintf(format, arg)}; + BOOST_CHECK_EQUAL(result.original, "original [original]"); + BOOST_CHECK_EQUAL(result.translated, "translated [translated]"); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/util/translation.h b/src/util/translation.h index 05e7da0b5a6..d2b49d00b0f 100644 --- a/src/util/translation.h +++ b/src/util/translation.h @@ -47,11 +47,24 @@ inline bilingual_str operator+(bilingual_str lhs, const bilingual_str& rhs) /** Mark a bilingual_str as untranslated */ inline bilingual_str Untranslated(std::string original) { return {original, original}; } +// Provide an overload of tinyformat::format which can take bilingual_str arguments. namespace tinyformat { +inline std::string TranslateArg(const bilingual_str& arg, bool translated) +{ + return translated ? arg.translated : arg.original; +} + +template +inline T const& TranslateArg(const T& arg, bool translated) +{ + return arg; +} + template bilingual_str format(const bilingual_str& fmt, const Args&... args) { - return bilingual_str{format(fmt.original, args...), format(fmt.translated, args...)}; + return bilingual_str{format(fmt.original, TranslateArg(args, false)...), + format(fmt.translated, TranslateArg(args, true)...)}; } } // namespace tinyformat diff --git a/test/lint/run-lint-format-strings.py b/test/lint/run-lint-format-strings.py index 57eefb00f2c..91915f05f9f 100755 --- a/test/lint/run-lint-format-strings.py +++ b/test/lint/run-lint-format-strings.py @@ -17,6 +17,7 @@ FALSE_POSITIVES = [ ("src/index/base.cpp", "FatalError(const char* fmt, const Args&... args)"), ("src/netbase.cpp", "LogConnectFailure(bool manual_connection, const char* fmt, const Args&... args)"), ("src/clientversion.cpp", "strprintf(_(COPYRIGHT_HOLDERS).translated, COPYRIGHT_HOLDERS_SUBSTITUTION)"), + ("src/test/translation_tests.cpp", "strprintf(format, arg)"), ("src/validationinterface.cpp", "LogPrint(BCLog::VALIDATION, fmt \"\\n\", __VA_ARGS__)"), ("src/wallet/wallet.h", "WalletLogPrintf(std::string fmt, Params... parameters)"), ("src/wallet/wallet.h", "LogPrintf((\"%s \" + fmt).c_str(), GetDisplayName(), parameters...)"), From d172b5c6718b69200c8ad211fe709860081bd692 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 24 Feb 2023 13:44:07 -0500 Subject: [PATCH 3/4] Add InitError(error, details) overload This is only used in the current PR to avoid ugly `strprintf(Untranslated("%s:\n%s"), str, MakeUnorderedList(details)` boilerplate in init code. But in the future the function could be extended and more widely used to include more details in GUI error messages or display them in a more readable way, see code comment. --- src/index/base.cpp | 2 +- src/node/interface_ui.cpp | 13 +++++++++++++ src/node/interface_ui.h | 2 +- src/shutdown.cpp | 2 +- 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index 6f2ce2efe4a..7c570d4534f 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -35,7 +35,7 @@ static void FatalError(const char* fmt, const Args&... args) std::string strMessage = tfm::format(fmt, args...); SetMiscWarning(Untranslated(strMessage)); LogPrintf("*** %s\n", strMessage); - AbortError(_("A fatal internal error occurred, see debug.log for details")); + InitError(_("A fatal internal error occurred, see debug.log for details")); StartShutdown(); } diff --git a/src/node/interface_ui.cpp b/src/node/interface_ui.cpp index 08d1e03541b..9dd1e7d9cf9 100644 --- a/src/node/interface_ui.cpp +++ b/src/node/interface_ui.cpp @@ -4,6 +4,7 @@ #include +#include #include #include @@ -62,6 +63,18 @@ bool InitError(const bilingual_str& str) return false; } +bool InitError(const bilingual_str& str, const std::vector& details) +{ + // For now just flatten the list of error details into a string to pass to + // the base InitError overload. In the future, if more init code provides + // error details, the details could be passed separately from the main + // message for rich display in the GUI. But currently the only init + // functions which provide error details are ones that run during early init + // before the GUI uiInterface is registered, so there's no point passing + // main messages and details separately to uiInterface yet. + return InitError(details.empty() ? str : strprintf(Untranslated("%s:\n%s"), str, MakeUnorderedList(details))); +} + void InitWarning(const bilingual_str& str) { uiInterface.ThreadSafeMessageBox(str, "", CClientUIInterface::MSG_WARNING); diff --git a/src/node/interface_ui.h b/src/node/interface_ui.h index 9f6503b4a14..22c241cb78e 100644 --- a/src/node/interface_ui.h +++ b/src/node/interface_ui.h @@ -116,7 +116,7 @@ void InitWarning(const bilingual_str& str); /** Show error message **/ bool InitError(const bilingual_str& str); -constexpr auto AbortError = InitError; +bool InitError(const bilingual_str& str, const std::vector& details); extern CClientUIInterface uiInterface; diff --git a/src/shutdown.cpp b/src/shutdown.cpp index 57d6d2325de..2fffc0663c7 100644 --- a/src/shutdown.cpp +++ b/src/shutdown.cpp @@ -27,7 +27,7 @@ bool AbortNode(const std::string& strMessage, bilingual_str user_message) if (user_message.empty()) { user_message = _("A fatal internal error occurred, see debug.log for details"); } - AbortError(user_message); + InitError(user_message); StartShutdown(); return false; } From 802cc1ef536e11944608fe9ab782d3e962037703 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Thu, 23 Feb 2023 15:56:15 -0500 Subject: [PATCH 4/4] Deduplicate bitcoind and bitcoin-qt init code Add common InitConfig function to deduplicate bitcoind and bitcoin-qt code reading config files and creating the datadir. There are a few minor changes in behavior: - In bitcoin-qt, when there is a problem reading the configuration file, the GUI error text has changed from "Error: Cannot parse configuration file:" to "Error reading configuration file:" to be consistent with bitcoind. - In bitcoind, when there is a problem reading the settings.json file, the error text has changed from "Failed loading settings file" to "Settings file could not be read" to be consistent with bitcoin-qt. - In bitcoind, when there is a problem writing the settings.json file, the error text has changed from "Failed saving settings file" to "Settings file could not be written" to be consistent with bitcoin-qt. - In bitcoin-qt, if there datadir is not accessible (e.g. no permission to read), there is an normal error dialog showing "Error: filesystem error: status: Permission denied [.../settings.json]", instead of an uncaught exception --- ci/test/06_script_b.sh | 1 + src/Makefile.am | 2 + src/bitcoind.cpp | 19 ++----- src/common/init.cpp | 74 +++++++++++++++++++++++++ src/common/init.h | 39 ++++++++++++++ src/qt/bitcoin.cpp | 120 +++++++++++++++-------------------------- src/util/system.cpp | 40 -------------- src/util/system.h | 13 ----- 8 files changed, 163 insertions(+), 145 deletions(-) create mode 100644 src/common/init.cpp create mode 100644 src/common/init.h diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh index 115d727ca30..e73ba6d31d3 100755 --- a/ci/test/06_script_b.sh +++ b/ci/test/06_script_b.sh @@ -42,6 +42,7 @@ if [ "${RUN_TIDY}" = "true" ]; then ( CI_EXEC run-clang-tidy -quiet "${MAKEJOBS}" ) | grep -C5 "error" export P_CI_DIR="${BASE_BUILD_DIR}/bitcoin-$HOST/" CI_EXEC "python3 ${DIR_IWYU}/include-what-you-use/iwyu_tool.py"\ + " src/common/init.cpp"\ " src/common/url.cpp"\ " src/compat"\ " src/dbwrapper.cpp"\ diff --git a/src/Makefile.am b/src/Makefile.am index 72e7db53348..7dc5594cf2c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -134,6 +134,7 @@ BITCOIN_CORE_H = \ clientversion.h \ coins.h \ common/bloom.h \ + common/init.h \ common/run_command.h \ common/url.h \ compat/assumptions.h \ @@ -640,6 +641,7 @@ libbitcoin_common_a_SOURCES = \ chainparams.cpp \ coins.cpp \ common/bloom.cpp \ + common/init.cpp \ common/interfaces.cpp \ common/run_command.cpp \ compressor.cpp \ diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index 4836b916331..b69913dddbb 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -150,17 +151,8 @@ static bool AppInit(NodeContext& node, int argc, char* argv[]) std::any context{&node}; try { - if (!CheckDataDirOption(args)) { - return InitError(Untranslated(strprintf("Specified data directory \"%s\" does not exist.", args.GetArg("-datadir", "")))); - } - if (!args.ReadConfigFiles(error, true)) { - return InitError(Untranslated(strprintf("Error reading configuration file: %s", error))); - } - // Check for chain settings (Params() calls are only valid after this clause) - try { - SelectParams(args.GetChainName()); - } catch (const std::exception& e) { - return InitError(Untranslated(strprintf("%s", e.what()))); + if (auto error = common::InitConfig(args)) { + return InitError(error->message, error->details); } // Error out when loose non-argument tokens are encountered on command line @@ -170,11 +162,6 @@ static bool AppInit(NodeContext& node, int argc, char* argv[]) } } - if (!args.InitSettings(error)) { - InitError(Untranslated(error)); - return false; - } - // -server defaults to true for bitcoind but not for the GUI so do this here args.SoftSetBoolArg("-server", true); // Set this early so that parameter interactions go to console diff --git a/src/common/init.cpp b/src/common/init.cpp new file mode 100644 index 00000000000..159eb7e2ef5 --- /dev/null +++ b/src/common/init.cpp @@ -0,0 +1,74 @@ +// Copyright (c) 2023 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +namespace common { +std::optional InitConfig(ArgsManager& args, SettingsAbortFn settings_abort_fn) +{ + try { + if (!CheckDataDirOption(args)) { + return ConfigError{ConfigStatus::FAILED, strprintf(_("Specified data directory \"%s\" does not exist."), args.GetArg("-datadir", ""))}; + } + std::string error; + if (!args.ReadConfigFiles(error, true)) { + return ConfigError{ConfigStatus::FAILED, strprintf(_("Error reading configuration file: %s"), error)}; + } + + // Check for chain settings (Params() calls are only valid after this clause) + SelectParams(args.GetChainName()); + + // Create datadir if it does not exist. + const auto base_path{args.GetDataDirBase()}; + if (!fs::exists(base_path)) { + // When creating a *new* datadir, also create a "wallets" subdirectory, + // whether or not the wallet is enabled now, so if the wallet is enabled + // in the future, it will use the "wallets" subdirectory for creating + // and listing wallets, rather than the top-level directory where + // wallets could be mixed up with other files. For backwards + // compatibility, wallet code will use the "wallets" subdirectory only + // if it already exists, but never create it itself. There is discussion + // in https://github.com/bitcoin/bitcoin/issues/16220 about ways to + // change wallet code so it would no longer be necessary to create + // "wallets" subdirectories here. + fs::create_directories(base_path / "wallets"); + } + const auto net_path{args.GetDataDirNet()}; + if (!fs::exists(net_path)) { + fs::create_directories(net_path / "wallets"); + } + + // Create settings.json if -nosettings was not specified. + if (args.GetSettingsPath()) { + std::vector details; + if (!args.ReadSettingsFile(&details)) { + const bilingual_str& message = _("Settings file could not be read"); + if (!settings_abort_fn) { + return ConfigError{ConfigStatus::FAILED, message, details}; + } else if (settings_abort_fn(message, details)) { + return ConfigError{ConfigStatus::ABORTED, message, details}; + } else { + details.clear(); // User chose to ignore the error and proceed. + } + } + if (!args.WriteSettingsFile(&details)) { + const bilingual_str& message = _("Settings file could not be written"); + return ConfigError{ConfigStatus::FAILED_WRITE, message, details}; + } + } + } catch (const std::exception& e) { + return ConfigError{ConfigStatus::FAILED, Untranslated(e.what())}; + } + return {}; +} +} // namespace common diff --git a/src/common/init.h b/src/common/init.h new file mode 100644 index 00000000000..380ac3ac7e7 --- /dev/null +++ b/src/common/init.h @@ -0,0 +1,39 @@ +// Copyright (c) 2023 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_COMMON_INIT_H +#define BITCOIN_COMMON_INIT_H + +#include + +#include +#include +#include +#include + +class ArgsManager; + +namespace common { +enum class ConfigStatus { + FAILED, //!< Failed generically. + FAILED_WRITE, //!< Failed to write settings.json + ABORTED, //!< Aborted by user +}; + +struct ConfigError { + ConfigStatus status; + bilingual_str message{}; + std::vector details{}; +}; + +//! Callback function to let the user decide whether to abort loading if +//! settings.json file exists and can't be parsed, or to ignore the error and +//! overwrite the file. +using SettingsAbortFn = std::function& details)>; + +/* Read config files, and create datadir and settings.json if they don't exist. */ +std::optional InitConfig(ArgsManager& args, SettingsAbortFn settings_abort_fn = nullptr); +} // namespace common + +#endif // BITCOIN_COMMON_INIT_H diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 440f475757c..5244b72689b 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -165,54 +166,36 @@ static void initTranslations(QTranslator &qtTranslatorBase, QTranslator &qtTrans } } -static bool InitSettings() +static bool ErrorSettingsRead(const bilingual_str& error, const std::vector& details) { - gArgs.EnsureDataDir(); - if (!gArgs.GetSettingsPath()) { - return true; // Do nothing if settings file disabled. - } - - std::vector errors; - if (!gArgs.ReadSettingsFile(&errors)) { - std::string error = QT_TRANSLATE_NOOP("bitcoin-core", "Settings file could not be read"); - std::string error_translated = QCoreApplication::translate("bitcoin-core", error.c_str()).toStdString(); - InitError(Untranslated(strprintf("%s:\n%s", error, MakeUnorderedList(errors)))); - - QMessageBox messagebox(QMessageBox::Critical, PACKAGE_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. */ - messagebox.setInformativeText(QObject::tr("Do you want to reset settings to default values, or to abort without making changes?")); - messagebox.setDetailedText(QString::fromStdString(MakeUnorderedList(errors))); - messagebox.setTextFormat(Qt::PlainText); - messagebox.setDefaultButton(QMessageBox::Reset); - switch (messagebox.exec()) { - case QMessageBox::Reset: - break; - case QMessageBox::Abort: - return false; - default: - assert(false); - } - } - - errors.clear(); - if (!gArgs.WriteSettingsFile(&errors)) { - std::string error = QT_TRANSLATE_NOOP("bitcoin-core", "Settings file could not be written"); - std::string error_translated = QCoreApplication::translate("bitcoin-core", error.c_str()).toStdString(); - InitError(Untranslated(strprintf("%s:\n%s", error, MakeUnorderedList(errors)))); - - QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error_translated)), QMessageBox::Ok); - /*: Explanatory text shown on startup when the settings file could not be written. - Prompts user to check that we have the ability to write to the file. - Explains that the user has the option of running without a settings file.*/ - messagebox.setInformativeText(QObject::tr("A fatal error occurred. Check that settings file is writable, or try running with -nosettings.")); - messagebox.setDetailedText(QString::fromStdString(MakeUnorderedList(errors))); - messagebox.setTextFormat(Qt::PlainText); - messagebox.setDefaultButton(QMessageBox::Ok); - messagebox.exec(); + QMessageBox messagebox(QMessageBox::Critical, PACKAGE_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. */ + messagebox.setInformativeText(QObject::tr("Do you want to reset settings to default values, or to abort without making changes?")); + messagebox.setDetailedText(QString::fromStdString(MakeUnorderedList(details))); + messagebox.setTextFormat(Qt::PlainText); + messagebox.setDefaultButton(QMessageBox::Reset); + switch (messagebox.exec()) { + case QMessageBox::Reset: return false; + case QMessageBox::Abort: + return true; + default: + assert(false); } - return true; +} + +static void ErrorSettingsWrite(const bilingual_str& error, const std::vector& details) +{ + QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error.translated)), QMessageBox::Ok); + /*: Explanatory text shown on startup when the settings file could not be written. + Prompts user to check that we have the ability to write to the file. + Explains that the user has the option of running without a settings file.*/ + messagebox.setInformativeText(QObject::tr("A fatal error occurred. Check that settings file is writable, or try running with -nosettings.")); + messagebox.setDetailedText(QString::fromStdString(MakeUnorderedList(details))); + messagebox.setTextFormat(Qt::PlainText); + messagebox.setDefaultButton(QMessageBox::Ok); + messagebox.exec(); } /* qDebug() message handler --> debug.log */ @@ -587,34 +570,23 @@ int GuiMain(int argc, char* argv[]) // Gracefully exit if the user cancels if (!Intro::showIfNeeded(did_show_intro, prune_MiB)) return EXIT_SUCCESS; - /// 6a. Determine availability of data directory - if (!CheckDataDirOption(gArgs)) { - InitError(strprintf(Untranslated("Specified data directory \"%s\" does not exist."), gArgs.GetArg("-datadir", ""))); - QMessageBox::critical(nullptr, PACKAGE_NAME, - QObject::tr("Error: Specified data directory \"%1\" does not exist.").arg(QString::fromStdString(gArgs.GetArg("-datadir", "")))); - return EXIT_FAILURE; - } - try { - /// 6b. Parse bitcoin.conf - /// - Do not call gArgs.GetDataDirNet() before this step finishes - if (!gArgs.ReadConfigFiles(error, true)) { - InitError(strprintf(Untranslated("Error reading configuration file: %s"), error)); - QMessageBox::critical(nullptr, PACKAGE_NAME, - QObject::tr("Error: Cannot parse configuration file: %1.").arg(QString::fromStdString(error))); - return EXIT_FAILURE; + /// 6-7. Parse bitcoin.conf, determine network, switch to network specific + /// options, and create datadir and settings.json. + // - Do not call gArgs.GetDataDirNet() before this step finishes + // - Do not call Params() before this step + // - QSettings() will use the new application name after this, resulting in network-specific settings + // - Needs to be done before createOptionsModel + if (auto error = common::InitConfig(gArgs, ErrorSettingsRead)) { + InitError(error->message, error->details); + if (error->status == common::ConfigStatus::FAILED_WRITE) { + // Show a custom error message to provide more information in the + // case of a datadir write error. + ErrorSettingsWrite(error->message, error->details); + } else if (error->status != common::ConfigStatus::ABORTED) { + // Show a generic message in other cases, and no additional error + // message in the case of a read error if the user decided to abort. + QMessageBox::critical(nullptr, PACKAGE_NAME, QObject::tr("Error: %1").arg(QString::fromStdString(error->message.translated))); } - - /// 7. Determine network (and switch to network specific options) - // - Do not call Params() before this step - // - Do this after parsing the configuration file, as the network can be switched there - // - QSettings() will use the new application name after this, resulting in network-specific settings - // - Needs to be done before createOptionsModel - - // Check for chain settings (Params() calls are only valid after this clause) - SelectParams(gArgs.GetChainName()); - } catch(std::exception &e) { - InitError(Untranslated(strprintf("%s", e.what()))); - QMessageBox::critical(nullptr, PACKAGE_NAME, QObject::tr("Error: %1").arg(e.what())); return EXIT_FAILURE; } #ifdef ENABLE_WALLET @@ -622,10 +594,6 @@ int GuiMain(int argc, char* argv[]) PaymentServer::ipcParseCommandLine(argc, argv); #endif - if (!InitSettings()) { - return EXIT_FAILURE; - } - QScopedPointer networkStyle(NetworkStyle::instantiate(Params().NetworkIDString())); assert(!networkStyle.isNull()); // Allow for separate UI settings for testnets diff --git a/src/util/system.cpp b/src/util/system.cpp index 58afd264ae0..5b1a1659bf6 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -438,27 +438,6 @@ const fs::path& ArgsManager::GetDataDir(bool net_specific) const return path; } -void ArgsManager::EnsureDataDir() const -{ - /** - * "/wallets" subdirectories are created in all **new** - * datadirs, because wallet code will create new wallets in the "wallets" - * subdirectory only if exists already, otherwise it will create them in - * the top-level datadir where they could interfere with other files. - * Wallet init code currently avoids creating "wallets" directories itself - * for backwards compatibility, but this be changed in the future and - * wallet code here could go away (#16220). - */ - auto path{GetDataDir(false)}; - if (!fs::exists(path)) { - fs::create_directories(path / "wallets"); - } - path = GetDataDir(true); - if (!fs::exists(path)) { - fs::create_directories(path / "wallets"); - } -} - void ArgsManager::ClearPathCache() { LOCK(cs_args); @@ -502,25 +481,6 @@ bool ArgsManager::IsArgSet(const std::string& strArg) const return !GetSetting(strArg).isNull(); } -bool ArgsManager::InitSettings(std::string& error) -{ - EnsureDataDir(); - if (!GetSettingsPath()) { - return true; // Do nothing if settings file disabled. - } - - std::vector errors; - if (!ReadSettingsFile(&errors)) { - error = strprintf("Failed loading settings file:\n%s\n", MakeUnorderedList(errors)); - return false; - } - if (!WriteSettingsFile(&errors)) { - error = strprintf("Failed saving settings file:\n%s\n", MakeUnorderedList(errors)); - return false; - } - return true; -} - bool ArgsManager::GetSettingsPath(fs::path* filepath, bool temp, bool backup) const { fs::path settings = GetPathArg("-settings", BITCOIN_SETTINGS_FILENAME); diff --git a/src/util/system.h b/src/util/system.h index 3eb0a0f2b87..f7bebe1f2aa 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -434,13 +434,6 @@ protected: */ std::optional GetArgFlags(const std::string& name) const; - /** - * Read and update settings file with saved settings. This needs to be - * called after SelectParams() because the settings file location is - * network-specific. - */ - bool InitSettings(std::string& error); - /** * Get settings file path, or return false if read-write settings were * disabled with -nosettings. @@ -480,12 +473,6 @@ protected: */ void LogArgs() const; - /** - * If datadir does not exist, create it along with wallets/ - * subdirectory(s). - */ - void EnsureDataDir() const; - private: /** * Get data directory path