Merge bitcoin/bitcoin#24251: Re-enable windows path tests disabled by #20744

d216bc8d76d7f4e9dce58b0bb732a2d4deaf23b6 Re-enable walletinit_verify_walletdir_no_trailing2 test disabled in #20744 (Ryan Ofsky)
80cd64e84296f1166e133c237fa0afc046b01ce2 Re-enable util_datadir check disabled in #20744 (Ryan Ofsky)

Pull request description:

  Reenable some broken tests as discussed https://github.com/bitcoin/bitcoin/pull/20744#discussion_r798651736 and https://github.com/bitcoin/bitcoin/pull/20744#discussion_r798678137

  Fix windows test cases broken in #20744, by passing normalized path arguments to fs::equivalent, fs::exists, and fs::is_directory, instead of non-normalized arguments. Also re-enable the tests.

  It is possible these changes also fix real init behavior on windows when -datadir or -walletdir paths with trailing dots or dashes are used, but it's not clear because I only tested on wine.

ACKs for top commit:
  hebasto:
    ACK d216bc8d76d7f4e9dce58b0bb732a2d4deaf23b6, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: 2099ddfa1a3ad70f7ac2ff413929414a1851d257b280da25c0f5cefb46fd1372b580a1f1ee5280681a1c16e6031f119185cadd4f7a6121298562cf001f711068
This commit is contained in:
fanquake 2022-02-05 20:01:03 +08:00 committed by pasta
parent f2f298286e
commit f3775d96b0
No known key found for this signature in database
GPG Key ID: 52527BEDABE87984
4 changed files with 5 additions and 9 deletions

View File

@ -73,12 +73,9 @@ BOOST_AUTO_TEST_CASE(util_datadir)
args.ClearPathCache(); args.ClearPathCache();
BOOST_CHECK_EQUAL(dd_norm, args.GetDataDirBase()); BOOST_CHECK_EQUAL(dd_norm, args.GetDataDirBase());
#ifndef WIN32
// Windows does not consider "datadir/.//" to be a valid directory path.
args.ForceSetArg("-datadir", fs::PathToString(dd_norm) + "/.//"); args.ForceSetArg("-datadir", fs::PathToString(dd_norm) + "/.//");
args.ClearPathCache(); args.ClearPathCache();
BOOST_CHECK_EQUAL(dd_norm, args.GetDataDirBase()); BOOST_CHECK_EQUAL(dd_norm, args.GetDataDirBase());
#endif
} }
namespace { namespace {

View File

@ -268,7 +268,7 @@ fs::path StripRedundantLastElementsOfPath(const fs::path& path)
result = result.parent_path(); result = result.parent_path();
} }
assert(fs::equivalent(result, path)); assert(fs::equivalent(result, path.lexically_normal()));
return result; return result;
} }
} // namespace } // namespace

View File

@ -27,11 +27,13 @@ bool VerifyWallets(interfaces::Chain& chain)
fs::path wallet_dir = fs::PathFromString(gArgs.GetArg("-walletdir", "")); fs::path wallet_dir = fs::PathFromString(gArgs.GetArg("-walletdir", ""));
std::error_code error; std::error_code error;
// The canonical path cleans the path, preventing >1 Berkeley environment instances for the same directory // The canonical path cleans the path, preventing >1 Berkeley environment instances for the same directory
// It also lets the fs::exists and fs::is_directory checks below pass on windows, since they return false
// if a path has trailing slashes, and it strips trailing slashes.
fs::path canonical_wallet_dir = fs::canonical(wallet_dir, error); fs::path canonical_wallet_dir = fs::canonical(wallet_dir, error);
if (error || !fs::exists(wallet_dir)) { if (error || !fs::exists(canonical_wallet_dir)) {
chain.initError(strprintf(_("Specified -walletdir \"%s\" does not exist"), fs::PathToString(wallet_dir))); chain.initError(strprintf(_("Specified -walletdir \"%s\" does not exist"), fs::PathToString(wallet_dir)));
return false; return false;
} else if (!fs::is_directory(wallet_dir)) { } else if (!fs::is_directory(canonical_wallet_dir)) {
chain.initError(strprintf(_("Specified -walletdir \"%s\" is not a directory"), fs::PathToString(wallet_dir))); chain.initError(strprintf(_("Specified -walletdir \"%s\" is not a directory"), fs::PathToString(wallet_dir)));
return false; return false;
// The canonical path transforms relative paths into absolute ones, so we check the non-canonical version // The canonical path transforms relative paths into absolute ones, so we check the non-canonical version

View File

@ -80,8 +80,6 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing)
BOOST_CHECK_EQUAL(walletdir, expected_path); BOOST_CHECK_EQUAL(walletdir, expected_path);
} }
#ifndef WIN32
// Windows does not consider "datadir/wallets//" to be a valid directory path.
BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing2) BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing2)
{ {
SetWalletDir(m_walletdir_path_cases["trailing2"]); SetWalletDir(m_walletdir_path_cases["trailing2"]);
@ -94,6 +92,5 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing2)
fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]); fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]);
BOOST_CHECK_EQUAL(walletdir, expected_path); BOOST_CHECK_EQUAL(walletdir, expected_path);
} }
#endif
BOOST_AUTO_TEST_SUITE_END() BOOST_AUTO_TEST_SUITE_END()