Merge bitcoin#21244: Move GetDataDir to ArgsManager

bb8d1c6 Change ClearDataDirPathCache() to ArgsManager.ClearPathCache(). (Kiminuo)
b4190ef Change GetBlocksDir() to ArgsManager.GetBlocksDirPath(). (Kiminuo)
83292e2 scripted-diff: Modify unit tests to use the ArgsManager in the BasicTestingSetup class instead of implicitly relying on gArgs. (Kiminuo)
55c68e6 scripted-diff: Replace m_args with m_local_args in getarg_tests.cpp (Kiminuo)
511ce3a BasicTestingSetup: Add ArgsManager. (Kiminuo)
1cb52ba Modify "util_datadir" unit test to not use gArgs. (Kiminuo)
1add318 Move GetDataDir(fNetSpecific) implementation to ArgsManager. (Kiminuo)
70cdf67 Move StripRedundantLastElementsOfPath before ArgsManager class. (Kiminuo)

Pull request description:

  This PR attempts to contribute to "Remove gArgs" (bitcoin#21005).

  Main changes:

  * `GetDataDir()` function is moved to `ArgsManager.GetDataDirPath()`.
  * `GetBlocksDir()` function is moved to `ArgsManager.GetBlocksDirPath()`.

ACKs for top commit:
  ryanofsky:
    Code review ACK bb8d1c6. Just minor const/naming changes and splitting/scripting commits since last review
  MarcoFalke:
    review ACK bb8d1c6 📓
  hebasto:
    re-ACK bb8d1c6, addressed comments, and two commits made scripted-diffs since my [previous](bitcoin#21244 (review)) review.

Tree-SHA512: ba9408c22129d6572beaa103dca0324131766f06d562bb7d6b9e214a0a4d40b0216ce861384562bde24b744003b3fbe6fac239061c8fd798abd3981ebc1b9019

src/flat-database.h

@@ -180,7 +180,7 @@ class CFlatDB
 public:
     CFlatDB(std::string strFilenameIn, std::string strMagicMessageIn)
     {
-        pathDB = GetDataDir() / strFilenameIn;
+        pathDB = m_args.GetDataDirPath() / strFilenameIn;
         strFilename = strFilenameIn;
         strMagicMessage = strMagicMessageIn;
     }

src/init.cpp

@@ -816,7 +816,7 @@ static void CleanupBlockRevFiles()
     // Remove the rev files immediately and insert the blk file paths into an
     // ordered map keyed by block file index.
     LogPrintf("Removing unusable blk?????.dat and rev?????.dat files for -reindex with -prune\n");
-    fs::path blocksdir = GetBlocksDir();
+    fs::path blocksdir = gArgs.GetBlocksDirPath();
     for (fs::directory_iterator it(blocksdir); it != fs::directory_iterator(); it++) {
         if (fs::is_regular_file(*it) &&
             it->path().filename().string().length() == 12 &&
@@ -1156,7 +1156,7 @@ bool AppInitParameterInteraction(const ArgsManager& args)
         InitWarning(warnings);
     }
 
-    if (!fs::is_directory(GetBlocksDir())) {
+    if (!fs::is_directory(gArgs.GetBlocksDirPath())) {
         return InitError(strprintf(_("Specified blocks directory \"%s\" does not exist."), args.GetArg("-blocksdir", "")));
     }
 
@@ -2255,8 +2255,8 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
         InitError(strprintf(_("Error: Disk space is low for %s"), GetDataDir()));
         return false;
     }
-    if (!CheckDiskSpace(GetBlocksDir())) {
-        InitError(strprintf(_("Error: Disk space is low for %s"), GetBlocksDir()));
+    if (!CheckDiskSpace(gArgs.GetBlocksDirPath())) {
+        InitError(strprintf(_("Error: Disk space is low for %s"), gArgs.GetBlocksDirPath()));
         return false;
     }
 

src/qt/clientmodel.cpp

@@ -251,7 +251,7 @@ QString ClientModel::dataDir() const
 
 QString ClientModel::blocksDir() const
 {
-    return GUIUtil::boostPathToQString(GetBlocksDir());
+    return GUIUtil::boostPathToQString(gArgs.GetBlocksDirPath());
 }
 
 void ClientModel::updateBanlist()

src/test/dbwrapper_tests.cpp

@@ -26,7 +26,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper)
 {
     // Perform tests both obfuscated and non-obfuscated.
     for (const bool obfuscate : {false, true}) {
-        fs::path ph = GetDataDir() / (obfuscate ? "dbwrapper_obfuscate_true" : "dbwrapper_obfuscate_false");
+        fs::path ph = m_args.GetDataDirPath() / (obfuscate ? "dbwrapper_obfuscate_true" : "dbwrapper_obfuscate_false");
         CDBWrapper dbw(ph, (1 << 20), true, false, obfuscate);
         char key = 'k';
         uint256 in = InsecureRand256();
@@ -45,7 +45,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_basic_data)
 {
     // Perform tests both obfuscated and non-obfuscated.
     for (bool obfuscate : {false, true}) {
-        fs::path ph = GetDataDir() / (obfuscate ? "dbwrapper_1_obfuscate_true" : "dbwrapper_1_obfuscate_false");
+        fs::path ph = m_args.GetDataDirPath() / (obfuscate ? "dbwrapper_1_obfuscate_true" : "dbwrapper_1_obfuscate_false");
         CDBWrapper dbw(ph, (1 << 20), false, true, obfuscate);
 
         uint256 res;
@@ -126,7 +126,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_batch)
 {
     // Perform tests both obfuscated and non-obfuscated.
     for (const bool obfuscate : {false, true}) {
-        fs::path ph = GetDataDir() / (obfuscate ? "dbwrapper_batch_obfuscate_true" : "dbwrapper_batch_obfuscate_false");
+        fs::path ph = m_args.GetDataDirPath() / (obfuscate ? "dbwrapper_batch_obfuscate_true" : "dbwrapper_batch_obfuscate_false");
         CDBWrapper dbw(ph, (1 << 20), true, false, obfuscate);
 
         char key = 'i';
@@ -162,7 +162,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_iterator)
 {
     // Perform tests both obfuscated and non-obfuscated.
     for (const bool obfuscate : {false, true}) {
-        fs::path ph = GetDataDir() / (obfuscate ? "dbwrapper_iterator_obfuscate_true" : "dbwrapper_iterator_obfuscate_false");
+        fs::path ph = m_args.GetDataDirPath() / (obfuscate ? "dbwrapper_iterator_obfuscate_true" : "dbwrapper_iterator_obfuscate_false");
         CDBWrapper dbw(ph, (1 << 20), true, false, obfuscate);
 
         // The two keys are intentionally chosen for ordering
@@ -202,7 +202,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_iterator)
 BOOST_AUTO_TEST_CASE(existing_data_no_obfuscate)
 {
     // We're going to share this fs::path between two wrappers
-    fs::path ph = GetDataDir() / "existing_data_no_obfuscate";
+    fs::path ph = m_args.GetDataDirPath() / "existing_data_no_obfuscate";
     create_directories(ph);
 
     // Set up a non-obfuscated wrapper to write some initial data.
@@ -243,7 +243,7 @@ BOOST_AUTO_TEST_CASE(existing_data_no_obfuscate)
 BOOST_AUTO_TEST_CASE(existing_data_reindex)
 {
     // We're going to share this fs::path between two wrappers
-    fs::path ph = GetDataDir() / "existing_data_reindex";
+    fs::path ph = m_args.GetDataDirPath() / "existing_data_reindex";
     create_directories(ph);
 
     // Set up a non-obfuscated wrapper to write some initial data.
@@ -278,7 +278,7 @@ BOOST_AUTO_TEST_CASE(existing_data_reindex)
 
 BOOST_AUTO_TEST_CASE(iterator_ordering)
 {
-    fs::path ph = GetDataDir() / "iterator_ordering";
+    fs::path ph = m_args.GetDataDirPath() / "iterator_ordering";
     CDBWrapper dbw(ph, (1 << 20), true, false, false);
     for (int x=0x00; x<256; ++x) {
         uint8_t key = x;
@@ -358,7 +358,7 @@ BOOST_AUTO_TEST_CASE(iterator_string_ordering)
 {
     char buf[10];
 
-    fs::path ph = GetDataDir() / "iterator_string_ordering";
+    fs::path ph = m_args.GetDataDirPath() / "iterator_string_ordering";
     CDBWrapper dbw(ph, (1 << 20), true, false, false);
     for (int x=0x00; x<10; ++x) {
         for (int y = 0; y < 10; y++) {
@@ -404,7 +404,7 @@ BOOST_AUTO_TEST_CASE(unicodepath)
     // On Windows this test will fail if the directory is created using
     // the ANSI CreateDirectoryA call and the code page isn't UTF8.
     // It will succeed if created with CreateDirectoryW.
-    fs::path ph = GetDataDir() / "test_runner_₿_🏃_20191128_104644";
+    fs::path ph = m_args.GetDataDirPath() / "test_runner_₿_🏃_20191128_104644";
     CDBWrapper dbw(ph, (1 << 20));
 
     fs::path lockPath = ph / "LOCK";

src/test/denialofservice_tests.cpp

@@ -222,7 +222,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management)
 BOOST_AUTO_TEST_CASE(DoS_banning)
 {
     const CChainParams& chainparams = Params();
-    auto banman = std::make_unique<BanMan>(GetDataDir() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME);
+    auto banman = std::make_unique<BanMan>(m_args.GetDataDirPath() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME);
     auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman);
     auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), *m_node.scheduler,
                                        *m_node.chainman, *m_node.mempool, *governance, m_node.cj_ctx,
@@ -268,7 +268,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
 BOOST_AUTO_TEST_CASE(DoS_banscore)
 {
     const CChainParams& chainparams = Params();
-    auto banman = std::make_unique<BanMan>(GetDataDir() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME);
+    auto banman = std::make_unique<BanMan>(m_args.GetDataDirPath() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME);
     auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman);
     auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), *m_node.scheduler,
                                        *m_node.chainman, *m_node.mempool, *governance, m_node.cj_ctx,
@@ -313,7 +313,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore)
 BOOST_AUTO_TEST_CASE(DoS_bantime)
 {
     const CChainParams& chainparams = Params();
-    auto banman = std::make_unique<BanMan>(GetDataDir() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME);
+    auto banman = std::make_unique<BanMan>(m_args.GetDataDirPath() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME);
     auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman);
     auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), *m_node.scheduler,
                                        *m_node.chainman, *m_node.mempool, *governance, m_node.cj_ctx,

src/test/flatfile_tests.cpp

@@ -14,7 +14,7 @@ BOOST_FIXTURE_TEST_SUITE(flatfile_tests, BasicTestingSetup)
 
 BOOST_AUTO_TEST_CASE(flatfile_filename)
 {
-    const auto data_dir = GetDataDir();
+    const auto data_dir = m_args.GetDataDirPath();
 
     FlatFilePos pos(456, 789);
 
@@ -27,7 +27,7 @@ BOOST_AUTO_TEST_CASE(flatfile_filename)
 
 BOOST_AUTO_TEST_CASE(flatfile_open)
 {
-    const auto data_dir = GetDataDir();
+    const auto data_dir = m_args.GetDataDirPath();
     FlatFileSeq seq(data_dir, "a", 16 * 1024);
 
     std::string line1("A purely peer-to-peer version of electronic cash would allow online "
@@ -88,7 +88,7 @@ BOOST_AUTO_TEST_CASE(flatfile_open)
 
 BOOST_AUTO_TEST_CASE(flatfile_allocate)
 {
-    const auto data_dir = GetDataDir();
+    const auto data_dir = m_args.GetDataDirPath();
     FlatFileSeq seq(data_dir, "a", 100);
 
     bool out_of_space;
@@ -108,7 +108,7 @@ BOOST_AUTO_TEST_CASE(flatfile_allocate)
 
 BOOST_AUTO_TEST_CASE(flatfile_flush)
 {
-    const auto data_dir = GetDataDir();
+    const auto data_dir = m_args.GetDataDirPath();
     FlatFileSeq seq(data_dir, "a", 100);
 
     bool out_of_space;

src/test/fs_tests.cpp

@@ -13,7 +13,7 @@ BOOST_FIXTURE_TEST_SUITE(fs_tests, BasicTestingSetup)
 
 BOOST_AUTO_TEST_CASE(fsbridge_fstream)
 {
-    fs::path tmpfolder = GetDataDir();
+    fs::path tmpfolder = m_args.GetDataDirPath();
     // tmpfile1 should be the same as tmpfile2
     fs::path tmpfile1 = tmpfolder / "fs_tests_₿_🏃";
     fs::path tmpfile2 = tmpfolder / "fs_tests_₿_🏃";