aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorfanquake <fanquake@gmail.com>2022-02-17 16:28:31 +0000
committerfanquake <fanquake@gmail.com>2022-02-17 16:29:10 +0000
commit003523d239aa41533482313aa3fd97c00641d69e (patch)
treee03073e2c9a8aff42b562fedc8c27770206f6882
parent922c49a1389531d9fba30168257c466bd413f625 (diff)
parentb223c3c21e89f6af76b5401413880923f7c444d6 (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.h22
-rw-r--r--src/test/dbwrapper_tests.cpp4
-rw-r--r--src/test/fs_tests.cpp24
-rwxr-xr-xtest/functional/feature_dirsymlinks.py39
-rwxr-xr-xtest/functional/test_runner.py1
5 files changed, 88 insertions, 2 deletions
diff --git a/src/fs.h b/src/fs.h
index d2299db168..00b786453c 100644
--- a/src/fs.h
+++ b/src/fs.h
@@ -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',