Skip to content

Commit 003523d

Browse files
committed
Merge bitcoin/bitcoin#24338: util: Work around libstdc++ create_directories issue
b223c3c test: Add functional test for symlinked blocks directory (laanwj) ddb75c2 test: Add fs_tests/create_directories unit test (Hennadii Stepanov) 1f46b6e 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 b223c3c per `git range-diff df08250 67019cd b223c3c` hebasto: re-ACK b223c3c w0xlt: re-ACK b223c3c vasild: ACK b223c3c Tree-SHA512: 028321717c8b10d16185c3711b35da6b05fb7aa31cee1c8c7e754e92bf5a0b02719a3785cd0f6f8bf052b3bd759f644af212320672baabc9e44e0b93ba464abc
2 parents 922c49a + b223c3c commit 003523d

File tree

5 files changed

+88
-2
lines changed

5 files changed

+88
-2
lines changed

src/fs.h

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,28 @@ static inline path PathFromString(const std::string& string)
140140
return std::filesystem::path(string);
141141
#endif
142142
}
143+
144+
/**
145+
* Create directory (and if necessary its parents), unless the leaf directory
146+
* already exists or is a symlink to an existing directory.
147+
* This is a temporary workaround for an issue in libstdc++ that has been fixed
148+
* upstream [PR101510].
149+
*/
150+
static inline bool create_directories(const std::filesystem::path& p)
151+
{
152+
if (std::filesystem::is_symlink(p) && std::filesystem::is_directory(p)) {
153+
return false;
154+
}
155+
return std::filesystem::create_directories(p);
156+
}
157+
158+
/**
159+
* This variant is not used. Delete it to prevent it from accidentally working
160+
* around the workaround. If it is needed, add a workaround in the same pattern
161+
* as above.
162+
*/
163+
bool create_directories(const std::filesystem::path& p, std::error_code& ec) = delete;
164+
143165
} // namespace fs
144166

145167
/** Bridge operations to C stdio */

src/test/dbwrapper_tests.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ BOOST_AUTO_TEST_CASE(existing_data_no_obfuscate)
203203
{
204204
// We're going to share this fs::path between two wrappers
205205
fs::path ph = m_args.GetDataDirBase() / "existing_data_no_obfuscate";
206-
create_directories(ph);
206+
fs::create_directories(ph);
207207

208208
// Set up a non-obfuscated wrapper to write some initial data.
209209
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)
244244
{
245245
// We're going to share this fs::path between two wrappers
246246
fs::path ph = m_args.GetDataDirBase() / "existing_data_reindex";
247-
create_directories(ph);
247+
fs::create_directories(ph);
248248

249249
// Set up a non-obfuscated wrapper to write some initial data.
250250
std::unique_ptr<CDBWrapper> dbw = std::make_unique<CDBWrapper>(ph, (1 << 10), false, false, false);

src/test/fs_tests.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,4 +152,28 @@ BOOST_AUTO_TEST_CASE(rename)
152152
fs::remove(path2);
153153
}
154154

155+
#ifndef WIN32
156+
BOOST_AUTO_TEST_CASE(create_directories)
157+
{
158+
// Test fs::create_directories workaround.
159+
const fs::path tmpfolder{m_args.GetDataDirBase()};
160+
161+
const fs::path dir{GetUniquePath(tmpfolder)};
162+
fs::create_directory(dir);
163+
BOOST_CHECK(fs::exists(dir));
164+
BOOST_CHECK(fs::is_directory(dir));
165+
BOOST_CHECK(!fs::create_directories(dir));
166+
167+
const fs::path symlink{GetUniquePath(tmpfolder)};
168+
fs::create_directory_symlink(dir, symlink);
169+
BOOST_CHECK(fs::exists(symlink));
170+
BOOST_CHECK(fs::is_symlink(symlink));
171+
BOOST_CHECK(fs::is_directory(symlink));
172+
BOOST_CHECK(!fs::create_directories(symlink));
173+
174+
fs::remove(symlink);
175+
fs::remove(dir);
176+
}
177+
#endif // WIN32
178+
155179
BOOST_AUTO_TEST_SUITE_END()
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
#!/usr/bin/env python3
2+
# Copyright (c) 2022 The Bitcoin Core developers
3+
# Distributed under the MIT software license, see the accompanying
4+
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
5+
"""Test successful startup with symlinked directories.
6+
"""
7+
8+
import os
9+
import sys
10+
11+
from test_framework.test_framework import BitcoinTestFramework, SkipTest
12+
13+
14+
def rename_and_link(*, from_name, to_name):
15+
os.rename(from_name, to_name)
16+
os.symlink(to_name, from_name)
17+
assert os.path.islink(from_name) and os.path.isdir(from_name)
18+
19+
class SymlinkTest(BitcoinTestFramework):
20+
def skip_test_if_missing_module(self):
21+
if sys.platform == 'win32':
22+
raise SkipTest("Symlinks test skipped on Windows")
23+
24+
def set_test_params(self):
25+
self.num_nodes = 1
26+
27+
def run_test(self):
28+
self.stop_node(0)
29+
30+
rename_and_link(from_name=os.path.join(self.nodes[0].datadir, self.chain, "blocks"),
31+
to_name=os.path.join(self.nodes[0].datadir, self.chain, "newblocks"))
32+
rename_and_link(from_name=os.path.join(self.nodes[0].datadir, self.chain, "chainstate"),
33+
to_name=os.path.join(self.nodes[0].datadir, self.chain, "newchainstate"))
34+
35+
self.start_node(0)
36+
37+
38+
if __name__ == '__main__':
39+
SymlinkTest().main()

test/functional/test_runner.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,7 @@
321321
'rpc_getdescriptorinfo.py',
322322
'rpc_mempool_entry_fee_fields_deprecation.py',
323323
'rpc_help.py',
324+
'feature_dirsymlinks.py',
324325
'feature_help.py',
325326
'feature_shutdown.py',
326327
'p2p_ibd_txrelay.py',

0 commit comments

Comments
 (0)