diff options
author | fanquake <fanquake@gmail.com> | 2022-02-17 16:28:31 +0000 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2022-02-17 16:29:10 +0000 |
commit | 003523d239aa41533482313aa3fd97c00641d69e (patch) | |
tree | e03073e2c9a8aff42b562fedc8c27770206f6882 | |
parent | 922c49a1389531d9fba30168257c466bd413f625 (diff) | |
parent | b223c3c21e89f6af76b5401413880923f7c444d6 (diff) |
Merge bitcoin/bitcoin#24338: util: Work around libstdc++ create_directories issue
b223c3c21e89f6af76b5401413880923f7c444d6 test: Add functional test for symlinked blocks directory (laanwj)
ddb75c2e87a60ed24065bdf0c3bfabf4e058cef1 test: Add fs_tests/create_directories unit test (Hennadii Stepanov)
1f46b6e46e1454b91ff7ceb31853bc440952f8eb util: Work around libstdc++ create_directories issue (laanwj)
Pull request description:
Work around libstdc++ issue [PR101510] with create_directories where the leaf already exists as a symlink. Fixes #24257, introduced by the switch to `std::filesystem`. It is meant to be more thorough than #24266, which worked around one instance of the problem.
The issue was [fixed upstream](https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=124eaa50e0a34f5f89572c1aa812c50979da58fc), but unfortunately we'll have to carry a fix for it for a while.
This introduces a function `fs::create_directories` which wraps
`std::filesystem::create_directories`. This allows easiliy reverting the
workaround when it is no longer necessary.
ACKs for top commit:
jonatack:
re-ACK b223c3c21e89f6af76b5401413880923f7c444d6 per `git range-diff df08250 67019cd b223c3c`
hebasto:
re-ACK b223c3c21e89f6af76b5401413880923f7c444d6
w0xlt:
re-ACK b223c3c
vasild:
ACK b223c3c21e89f6af76b5401413880923f7c444d6
Tree-SHA512: 028321717c8b10d16185c3711b35da6b05fb7aa31cee1c8c7e754e92bf5a0b02719a3785cd0f6f8bf052b3bd759f644af212320672baabc9e44e0b93ba464abc
-rw-r--r-- | src/fs.h | 22 | ||||
-rw-r--r-- | src/test/dbwrapper_tests.cpp | 4 | ||||
-rw-r--r-- | src/test/fs_tests.cpp | 24 | ||||
-rwxr-xr-x | test/functional/feature_dirsymlinks.py | 39 | ||||
-rwxr-xr-x | test/functional/test_runner.py | 1 |
5 files changed, 88 insertions, 2 deletions
@@ -140,6 +140,28 @@ static inline path PathFromString(const std::string& string) return std::filesystem::path(string); #endif } + +/** + * Create directory (and if necessary its parents), unless the leaf directory + * already exists or is a symlink to an existing directory. + * This is a temporary workaround for an issue in libstdc++ that has been fixed + * upstream [PR101510]. + */ +static inline bool create_directories(const std::filesystem::path& p) +{ + if (std::filesystem::is_symlink(p) && std::filesystem::is_directory(p)) { + return false; + } + return std::filesystem::create_directories(p); +} + +/** + * This variant is not used. Delete it to prevent it from accidentally working + * around the workaround. If it is needed, add a workaround in the same pattern + * as above. + */ +bool create_directories(const std::filesystem::path& p, std::error_code& ec) = delete; + } // namespace fs /** Bridge operations to C stdio */ diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp index 2f95cc27a3..fc89fe1450 100644 --- a/src/test/dbwrapper_tests.cpp +++ b/src/test/dbwrapper_tests.cpp @@ -203,7 +203,7 @@ BOOST_AUTO_TEST_CASE(existing_data_no_obfuscate) { // We're going to share this fs::path between two wrappers fs::path ph = m_args.GetDataDirBase() / "existing_data_no_obfuscate"; - create_directories(ph); + fs::create_directories(ph); // Set up a non-obfuscated wrapper to write some initial data. std::unique_ptr<CDBWrapper> dbw = std::make_unique<CDBWrapper>(ph, (1 << 10), false, false, false); @@ -244,7 +244,7 @@ BOOST_AUTO_TEST_CASE(existing_data_reindex) { // We're going to share this fs::path between two wrappers fs::path ph = m_args.GetDataDirBase() / "existing_data_reindex"; - create_directories(ph); + fs::create_directories(ph); // Set up a non-obfuscated wrapper to write some initial data. std::unique_ptr<CDBWrapper> dbw = std::make_unique<CDBWrapper>(ph, (1 << 10), false, false, false); diff --git a/src/test/fs_tests.cpp b/src/test/fs_tests.cpp index 4fb2c23d98..313064b294 100644 --- a/src/test/fs_tests.cpp +++ b/src/test/fs_tests.cpp @@ -152,4 +152,28 @@ BOOST_AUTO_TEST_CASE(rename) fs::remove(path2); } +#ifndef WIN32 +BOOST_AUTO_TEST_CASE(create_directories) +{ + // Test fs::create_directories workaround. + const fs::path tmpfolder{m_args.GetDataDirBase()}; + + const fs::path dir{GetUniquePath(tmpfolder)}; + fs::create_directory(dir); + BOOST_CHECK(fs::exists(dir)); + BOOST_CHECK(fs::is_directory(dir)); + BOOST_CHECK(!fs::create_directories(dir)); + + const fs::path symlink{GetUniquePath(tmpfolder)}; + fs::create_directory_symlink(dir, symlink); + BOOST_CHECK(fs::exists(symlink)); + BOOST_CHECK(fs::is_symlink(symlink)); + BOOST_CHECK(fs::is_directory(symlink)); + BOOST_CHECK(!fs::create_directories(symlink)); + + fs::remove(symlink); + fs::remove(dir); +} +#endif // WIN32 + BOOST_AUTO_TEST_SUITE_END() diff --git a/test/functional/feature_dirsymlinks.py b/test/functional/feature_dirsymlinks.py new file mode 100755 index 0000000000..85c8e27600 --- /dev/null +++ b/test/functional/feature_dirsymlinks.py @@ -0,0 +1,39 @@ +#!/usr/bin/env python3 +# Copyright (c) 2022 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test successful startup with symlinked directories. +""" + +import os +import sys + +from test_framework.test_framework import BitcoinTestFramework, SkipTest + + +def rename_and_link(*, from_name, to_name): + os.rename(from_name, to_name) + os.symlink(to_name, from_name) + assert os.path.islink(from_name) and os.path.isdir(from_name) + +class SymlinkTest(BitcoinTestFramework): + def skip_test_if_missing_module(self): + if sys.platform == 'win32': + raise SkipTest("Symlinks test skipped on Windows") + + def set_test_params(self): + self.num_nodes = 1 + + def run_test(self): + self.stop_node(0) + + rename_and_link(from_name=os.path.join(self.nodes[0].datadir, self.chain, "blocks"), + to_name=os.path.join(self.nodes[0].datadir, self.chain, "newblocks")) + rename_and_link(from_name=os.path.join(self.nodes[0].datadir, self.chain, "chainstate"), + to_name=os.path.join(self.nodes[0].datadir, self.chain, "newchainstate")) + + self.start_node(0) + + +if __name__ == '__main__': + SymlinkTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 1a0d62aa8f..516e8be638 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -321,6 +321,7 @@ BASE_SCRIPTS = [ 'rpc_getdescriptorinfo.py', 'rpc_mempool_entry_fee_fields_deprecation.py', 'rpc_help.py', + 'feature_dirsymlinks.py', 'feature_help.py', 'feature_shutdown.py', 'p2p_ibd_txrelay.py', |