diff options
author | fanquake <fanquake@gmail.com> | 2022-02-05 20:01:03 +0800 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2022-02-05 20:02:41 +0800 |
commit | 1e7564eca8a688f39c75540877ec3bdfdde766b1 (patch) | |
tree | 62b5b333d1f6ff38e1aeea71825c4a776e2d455c | |
parent | b23e0379610dde49e18c7e3e66edd23ebbcdcb45 (diff) | |
parent | d216bc8d76d7f4e9dce58b0bb732a2d4deaf23b6 (diff) |
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
-rw-r--r-- | src/test/util_tests.cpp | 3 | ||||
-rw-r--r-- | src/util/system.cpp | 2 | ||||
-rw-r--r-- | src/wallet/load.cpp | 6 | ||||
-rw-r--r-- | src/wallet/test/init_tests.cpp | 3 |
4 files changed, 5 insertions, 9 deletions
diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index b6479efe7d..52b24327ec 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -71,12 +71,9 @@ BOOST_AUTO_TEST_CASE(util_datadir) args.ClearPathCache(); 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.ClearPathCache(); BOOST_CHECK_EQUAL(dd_norm, args.GetDataDirBase()); -#endif } BOOST_AUTO_TEST_CASE(util_check) diff --git a/src/util/system.cpp b/src/util/system.cpp index 0ee63f6381..8d0cec249d 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -253,7 +253,7 @@ fs::path StripRedundantLastElementsOfPath(const fs::path& path) result = result.parent_path(); } - assert(fs::equivalent(result, path)); + assert(fs::equivalent(result, path.lexically_normal())); return result; } } // namespace diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 4949ed7dc9..6a74f2eb84 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -31,11 +31,13 @@ bool VerifyWallets(WalletContext& context) fs::path wallet_dir = fs::PathFromString(args.GetArg("-walletdir", "")); std::error_code error; // 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); - 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))); 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))); return false; // The canonical path transforms relative paths into absolute ones, so we check the non-canonical version diff --git a/src/wallet/test/init_tests.cpp b/src/wallet/test/init_tests.cpp index d1df48a312..c1cae5c5f6 100644 --- a/src/wallet/test/init_tests.cpp +++ b/src/wallet/test/init_tests.cpp @@ -73,8 +73,6 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing) 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) { SetWalletDir(m_walletdir_path_cases["trailing2"]); @@ -84,7 +82,6 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing2) fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]); BOOST_CHECK_EQUAL(walletdir, expected_path); } -#endif BOOST_AUTO_TEST_SUITE_END() } // namespace wallet |