Merge bitcoin/bitcoin#21850: Remove GetDataDir(net_specific) function

aca0e5dcdb Remove `GetDataDir(bool fNetSpecific = true)` function (Kiminuo)
b3e67f20a0 scripted-diff: Replace `GetDataDir(true)` calls with `gArgs.GetDataDirNet()` calls (Kiminuo)
4c3a5dcbfc scripted-diff: Replace `GetDataDir()` calls with `gArgs.GetDataDirNet()` calls (Kiminuo)
13bd8bb053 Make `ArgsManager.GetDataDirPath` private and drop needless suffix (Kiminuo)
4d8189f620 scripted-diff: Change `ArgsManager.GetDataDirPath()` to `ArgsManager.GetDataDirBase()` in tests (Kiminuo)
0f53df47d5 Add `ArgsManager.GetDataDirBase()` and `ArgsManager.GetDataDirNet()` as an intended replacement for `ArgsManager.GetDataDirPath(net_identifier)` (Kiminuo)
716de29dd8 Make `m_cached_blocks_path` mutable. Make `ArgsManager::GetBlocksDirPath()` const. (Kiminuo)

Pull request description:

  This PR is a follow up PR to #21244. The PR attempts to move us an inch towards the [goal](https://github.com/bitcoin/bitcoin/pull/21244#discussion_r615307465) by removing `GetDataDir(net_specific)` and replacing it by `gArgs.GetDataDir(net_specific)` calls.

  The approach of this PR attempts to be similar to the one chosen in "De-globalize ChainstateManager" (#20158). The goal is to pass `ArgsManager` to functions (or ideally to have `ArgsManager` as a member of a class where needed; inspiration from here: #21789) instead of having it as a global variable (i.e. `gArgs`).

  **Notes:**
  * First commit makes `m_cached_blocks_path` `mutable` as was suggested [here](https://github.com/bitcoin/bitcoin/pull/21244#discussion_r615274095) but not fully applied in #21244. (`m_cached_datadir_path` and `m_cached_network_datadir_path` were marked as `mutable` in #21244) This commit can be in a separate PR too.
  * Other commits deal with removing of `GetDataDir(net_specific)` function.
      * This was originally part of #21244 but it was [left]((https://github.com/bitcoin/bitcoin/pull/21244#pullrequestreview-633779754)) for a follow up PR.
  * I think that the proposed changes show nicely where there is reliance on `gArgs` which is IMO a good thing.

  If you know about a better approach how to do this, please share it here.

ACKs for top commit:
  hebasto:
    ACK aca0e5dcdb
  MarcoFalke:
    re-ACK aca0e5dcdb 👃

Tree-SHA512: deec4d88edb32d7f4c818c3a74ffbb64709685819b88242dcf5dbaa1fb611f3ce2b29d2576ddb9e0dc5e75288e43538968224008c0a80e7149fc81c309f7c9da
This commit is contained in:
MarcoFalke
2021-05-24 11:05:48 +02:00
36 changed files with 101 additions and 91 deletions

View File

@@ -386,7 +386,7 @@ std::optional<unsigned int> ArgsManager::GetArgFlags(const std::string& name) co
return std::nullopt;
}
const fs::path& ArgsManager::GetBlocksDirPath()
const fs::path& ArgsManager::GetBlocksDirPath() const
{
LOCK(cs_args);
fs::path& path = m_cached_blocks_path;
@@ -402,7 +402,7 @@ const fs::path& ArgsManager::GetBlocksDirPath()
return path;
}
} else {
path = GetDataDirPath(false);
path = GetDataDirBase();
}
path /= BaseParams().DataDir();
@@ -412,7 +412,7 @@ const fs::path& ArgsManager::GetBlocksDirPath()
return path;
}
const fs::path& ArgsManager::GetDataDirPath(bool net_specific) const
const fs::path& ArgsManager::GetDataDir(bool net_specific) const
{
LOCK(cs_args);
fs::path& path = net_specific ? m_cached_network_datadir_path : m_cached_datadir_path;
@@ -511,7 +511,7 @@ bool ArgsManager::GetSettingsPath(fs::path* filepath, bool temp) const
}
if (filepath) {
std::string settings = GetArg("-settings", BITCOIN_SETTINGS_FILENAME);
*filepath = fsbridge::AbsPathJoin(GetDataDirPath(/* net_specific= */ true), temp ? settings + ".tmp" : settings);
*filepath = fsbridge::AbsPathJoin(GetDataDirNet(), temp ? settings + ".tmp" : settings);
}
return true;
}
@@ -800,11 +800,6 @@ fs::path GetDefaultDataDir()
#endif
}
const fs::path &GetDataDir(bool fNetSpecific)
{
return gArgs.GetDataDirPath(fNetSpecific);
}
bool CheckDataDirOption()
{
std::string datadir = gArgs.GetArg("-datadir", "");
@@ -1359,7 +1354,7 @@ fs::path AbsPathForConfigVal(const fs::path& path, bool net_specific)
if (path.is_absolute()) {
return path;
}
return fsbridge::AbsPathJoin(GetDataDir(net_specific), path);
return fsbridge::AbsPathJoin(net_specific ? gArgs.GetDataDirNet() : gArgs.GetDataDirBase(), path);
}
void ScheduleBatchPriority()

View File

@@ -90,7 +90,6 @@ void ReleaseDirectoryLocks();
bool TryCreateDirectories(const fs::path& p);
fs::path GetDefaultDataDir();
const fs::path &GetDataDir(bool fNetSpecific = true);
// Return true if -datadir option points to a valid directory or is not specified.
bool CheckDataDirOption();
fs::path GetConfigFile(const std::string& confPath);
@@ -119,7 +118,7 @@ UniValue RunCommandParseJSON(const std::string& str_command, const std::string&
* the datadir if they are not absolute.
*
* @param path The path to be conditionally prefixed with datadir.
* @param net_specific Forwarded to GetDataDir().
* @param net_specific Use network specific datadir variant
* @return The normalized path.
*/
fs::path AbsPathForConfigVal(const fs::path& path, bool net_specific = true);
@@ -195,7 +194,7 @@ protected:
std::map<OptionsCategory, std::map<std::string, Arg>> m_available_args GUARDED_BY(cs_args);
bool m_accept_any_command GUARDED_BY(cs_args){true};
std::list<SectionInfo> m_config_sections GUARDED_BY(cs_args);
fs::path m_cached_blocks_path GUARDED_BY(cs_args);
mutable fs::path m_cached_blocks_path GUARDED_BY(cs_args);
mutable fs::path m_cached_datadir_path GUARDED_BY(cs_args);
mutable fs::path m_cached_network_datadir_path GUARDED_BY(cs_args);
@@ -266,16 +265,23 @@ public:
*
* @return Blocks path which is network specific
*/
const fs::path& GetBlocksDirPath();
const fs::path& GetBlocksDirPath() const;
/**
* Get data directory path
*
* @param net_specific Append network identifier to the returned path
* @return Absolute path on success, otherwise an empty path when a non-directory path would be returned
* @post Returned directory path is created unless it is empty
*/
const fs::path& GetDataDirPath(bool net_specific = true) const;
const fs::path& GetDataDirBase() const { return GetDataDir(false); }
/**
* Get data directory path with appended network identifier
*
* @return Absolute path on success, otherwise an empty path when a non-directory path would be returned
* @post Returned directory path is created unless it is empty
*/
const fs::path& GetDataDirNet() const { return GetDataDir(true); }
/**
* Clear cached directory paths
@@ -437,6 +443,15 @@ public:
void LogArgs() const;
private:
/**
* Get data directory path
*
* @param net_specific Append network identifier to the returned path
* @return Absolute path on success, otherwise an empty path when a non-directory path would be returned
* @post Returned directory path is created unless it is empty
*/
const fs::path& GetDataDir(bool net_specific) const;
// Helper function for LogArgs().
void logArgsPrefix(
const std::string& prefix,