util: Work around libstdc++ create

laanwj commented at 12:55 pm on February 14, 2022: member

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, 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.

laanwj added the label Utils/log/libs on Feb 14, 2022

laanwj added this to the milestone 23.0 on Feb 14, 2022

MarcoFalke commented at 1:03 pm on February 14, 2022: member

Concept ACK.

Mind applying this diff: #24266 (comment) ?

Otherwise the blocks dir handling differs from the wallet dir handling (and root dir handling).

laanwj force-pushed on Feb 14, 2022

laanwj commented at 1:07 pm on February 14, 2022: member

Mind applying this diff: #24266 (comment) ?

~~Done!~~ Reverted for now (https://github.com/bitcoin/bitcoin/pull/24338#issuecomment-1039171253).

in src/fs.h:152 in e6fafbdfd0 outdated

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 (fs::is_symlink(p) && fs::is_directory(p)) {

jonatack commented at 1:34 pm on February 14, 2022:

I could be confused but it reads as though the comment says “or” and the logic says “and”

Edit: std::filesystem::create_directories returns false for an existing directory per https://en.cppreference.com/w/cpp/filesystem/create_directory, so this seems good.

03) Executes (1) for every element of p that does not already exist.
1   If p already exists, the function does nothing (this condition is not treated as an error).
2
3Return value
4true if a directory was created for the directory p resolves to, false otherwise.

laanwj commented at 2:00 pm on February 14, 2022:

Right, or would work, but it’s not necessary. This if statement is to catch the specific situation that is buggy, it doesn’t aim to implement anything further.

vasild commented at 2:46 pm on February 14, 2022:

Or just std::filesystem::exists() would also work, I guess but will have a wider scope.

catch the specific situation that is buggy

+1

laanwj commented at 2:54 pm on February 14, 2022:

What if it is a symlink to a file, a device, etc, instead of a directory? I think it should still fail in that case. That’s why I didn’t use the more general exists.

vasild commented at 2:31 pm on February 15, 2022:

By “fail” you mean to throw an exception or to return false? According to the spec, it should return false in that case.

laanwj commented at 3:07 pm on February 16, 2022:

OK. Yeah thanks, but I think it’s fine to deviate from the spec in this case, it’s fatal in our case.

laanwj commented at 2:03 pm on February 14, 2022: member

I’m really confused by the CI output:

 0Traceback (most recent call last):
 1  File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/combine_logs.py", line 201, in <module>
 2    main()
 3  File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/combine_logs.py", line 67, in main
 4    log_events = read_logs(testdir)
 5  File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/combine_logs.py", line 86, in read_logs
 6    assert next(glob, None) is None #  more than one debug.log, should never happen
 7  File "/usr/lib64/python3.6/pathlib.py", line 1100, in glob
 8    for p in selector.select_from(self):
 9  File "/usr/lib64/python3.6/pathlib.py", line 508, in _select_from
10    for p in self.successor._select_from(path, is_dir, exists, scandir):
11  File "/usr/lib64/python3.6/pathlib.py", line 559, in _select_from
12    for starting_point in self._iterate_directories(parent_path, is_dir, scandir):
13  File "/usr/lib64/python3.6/pathlib.py", line 549, in _iterate_directories
14    for p in self._iterate_directories(path, is_dir, scandir):
15  File "/usr/lib64/python3.6/pathlib.py", line 549, in _iterate_directories
16    for p in self._iterate_directories(path, is_dir, scandir):
17  File "/usr/lib64/python3.6/pathlib.py", line 549, in _iterate_directories
18    for p in self._iterate_directories(path, is_dir, scandir):
19  File "/usr/lib64/python3.6/pathlib.py", line 547, in _iterate_directories
20    if entry.is_dir() and not entry.is_symlink():
21OSError: [Errno 40] Too many levels of symbolic links: '/tmp/cirrus-ci-build/ci/scratch/test_runner/test_runner_\u20bf_🏃_20220214_135218/wallet_multiwallet_169/node0/regtest/walletdir/self_walletdat_symlink/wallet.dat'

This change doesn’t actually create any symbolic links, it just tolerates them, neither does it change any Python code, so I don’t understand.

jonatack commented at 2:28 pm on February 14, 2022: member

I’ve haven’t been following the issue, but the code makes sense, util and dbwrapper unit tests are green for me on Debian with clang 13.0.1, but wallet_multiwallet.py isn’t happy.

02022-02-14T14:25:38.701000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_5_zm4jhi
12022-02-14T14:25:44.906000Z TestFramework (INFO): Do not allow -upgradewallet with multiwallet
22022-02-14T14:25:45.837000Z TestFramework (ERROR): Unexpected exception caught during testing
3Traceback (most recent call last):
4  File "/home/jon/projects/bitcoin/bitcoin/test/functional/test_framework/test_framework.py", line 132, in main
5    self.run_test()
6  File "/home/jon/projects/bitcoin/bitcoin/test/functional/wallet_multiwallet.py", line 192, in run_test
7    os.rename(wallet_dir2, wallet_dir())
8OSError: [Errno 39] Directory not empty: '/tmp/bitcoin_func_test_5_zm4jhi/node0/regtest/walletdir' -> '/tmp/bitcoin_func_test_5_zm4jhi/node0/regtest/wallets'
9</p></details>

laanwj force-pushed on Feb 14, 2022

laanwj commented at 2:45 pm on February 14, 2022: member

I’ve reverted @MarcoFalke ’s proposed change for now. Let’s see if this passes CI.

in src/fs.h:150 in 01c5aefa03 outdated

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)

vasild commented at 2:49 pm on February 14, 2022:

There is also another function:

0bool create_directories(const std::filesystem::path& p, std::error_code& ec);

Is it worth hijacking that too? In case somebody in the future decides to use that one instead?

laanwj commented at 2:52 pm on February 14, 2022:

I thought about it, but defining a function that’s not used felt wrong, somehow. Is there a way to un-define it? (I’m not exactly sure how the overloading rules work here)

Edit: this seems to work?

0static inline bool create_directories(const std::filesystem::path& p, std::error_code& ec) = delete;

vasild commented at 3:08 pm on February 14, 2022:

Cool, I did not know = delete could be used on non-member functions. It seems it can - https://en.cppreference.com/w/cpp/language/function#Deleted_functions. static inline can be skipped.

laanwj commented at 3:28 pm on February 14, 2022:

Thanks, will remove the static inline.

hebasto commented at 11:30 pm on February 16, 2022:

static inline can be skipped.

Thanks, will remove the static inline.

:smile:

MarcoFalke commented at 3:14 pm on February 14, 2022: member

I’ve reverted @MarcoFalke ’s proposed change for now. Let’s see if this passes CI.

Interesting. For reference, the failed CI is here: https://cirrus-ci.com/build/6366611337641984

It would be good to know why it failed. Otherwise it seems that the same failure that happened on the wallet dir is going to happen on the blocks dir as well, with this patch.

laanwj commented at 3:17 pm on February 14, 2022: member

Otherwise it seems that the same failure that happened on the wallet dir is going to happen on the blocks dir as well, with this patch.

This was the first thing I checked (as it’s the problem I’d been suffering from), and it worked fine.

Edit: could it be a problem with making multiple directories at once? I mean, this looks if a path exists before making a subdirectory of it:

0    if (!fs::exists(path)) {
1        fs::create_directories(path / "wallets");
2    }

It’s not the same as fs::create_directories(path / "wallets");, which will always create a wallets subdirectory, even if the parent path already exists.

jonatack commented at 3:17 pm on February 14, 2022: member

01c5aefa032f1f7d28c132481624e2d74aa1d14b solves the wallet_multiwallet.py error in #24338 (comment) for me locally.

MarcoFalke commented at 3:46 pm on February 14, 2022: member

Ugh, my bad. :man_facepalming:

in src/test/dbwrapper_tests.cpp:206 in 01c5aefa03 outdated

202@@ -203,7 +203,7 @@ BOOST_AUTO_TEST_CASE(existing_data_no_obfuscate)
203 {
204     // We're going to share this fs::path between two wrappers
205     fs::path ph = m_args.GetDataDirBase() / "existing_data_no_obfuscate";
206-    create_directories(ph);

MarcoFalke commented at 3:57 pm on February 14, 2022:

Any idea why this even compiles?

laanwj commented at 4:15 pm on February 14, 2022:

No, no idea, I was surprised too. It started giving an error about an ambiguous overload with this patch.

vasild commented at 2:28 pm on February 15, 2022:

My guess, without checking, is that some of the boost headers do using std::filesystem. boost/dll/config.hpp looks suspicious, it contains /// \brief Imports filesystem, ....

MarcoFalke commented at 2:56 pm on February 15, 2022:

The following compiles:

0#include <filesystem>
1int main() {
2    create_directories(std::filesystem::path{"/"});
3}

vasild commented at 3:14 pm on February 15, 2022:

Wow! I confirm it compiles with clang 14. But just create_directories("/"); does not compile: error: use of undeclared identifier 'create_directories'; did you mean 'std::filesystem::create_directories'? :mage:

ryanofsky commented at 6:13 pm on February 16, 2022:

Wow! I confirm it compiles with clang 14.

https://en.wikipedia.org/wiki/Argument-dependent_name_lookup

laanwj force-pushed on Feb 14, 2022

jonatack commented at 5:03 pm on February 14, 2022: member

utACK b9dff100ff2e7dfb9f216e98964fe28ef5bac3a9

The disabled function corresponds to the second create_directories of 3 in https://en.cppreference.com/w/cpp/filesystem/create_directory

w0xlt commented at 1:39 am on February 15, 2022: contributor

If the blocks directory is a symlink, as mentioned in #24257 (comment), the error below still occurs.

0************************
1EXCEPTION: NSt10filesystem7__cxx1116filesystem_errorE       
2filesystem error: cannot create directories: Not a directory [/home/.../.bitcoin/signet/blocks]       
3bitcoin in AppInit()

MarcoFalke commented at 7:37 am on February 15, 2022: member

Is it possible to write a test for this with https://docs.python.org/3.8/library/os.html#os.symlink ? (Like the multiwallet test)

laanwj commented at 9:10 am on February 15, 2022: member

If the blocks directory is a symlink, as mentioned in #24257 (comment), the error below still occurs.

Strange, it works here on Ubuntu 20.04. The blocks directory is a symlink.

0$ stat ~/.bitcoin/blocks
1  File: /home/…/.bitcoin/blocks -> /…/bitcoin/blocks

Without this patch I get

0************************
1EXCEPTION: NSt10filesystem7__cxx1116filesystem_errorE       
2filesystem error: cannot create directories: Not a directory [/home/…/.bitcoin/blocks]       
3bitcoin in AppInit()

With this patch, it works:

0$ src/bitcoind                                                        
12022-02-15T08:59:11Z Bitcoin Core version v22.99.0-b9dff100ff2e (release build)
2…

I have checked the same for signet. Both making the signet directory a link, and signet/blocks.

Is it possible to write a test for this with https://docs.python.org/3.8/library/os.html#os.symlink ? (Like the multiwallet test)

Sure. I’ll look into it.

in src/fs.h:152 in b9dff100ff outdated

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 (fs::is_symlink(p) && fs::is_directory(p)) {

vasild commented at 2:47 pm on February 15, 2022:

nit (feel free to ignore): this is inside the fs namespace, so maybe the fs:: prefix is not necessary.

laanwj commented at 9:13 am on February 16, 2022:

I prefer to keep this fully qualified, just to be sure, and entirely clear what these refer to in case other functions are wrapped.

hebasto commented at 11:16 am on February 16, 2022:

I prefer to keep this fully qualified, just to be sure, and entirely clear what these refer to in case other functions are wrapped.

Then, maybe

0    if (std::filesystem::is_symlink(p) && std::filesystem::is_directory(p)) {

?

laanwj commented at 3:02 pm on February 16, 2022:

Well, if we override these, we’ll likely want to use the overridden versions, right? (at least, it’s something to seriously consider because we’d be overriding them for a good reason). I don’t think it makes sense to change these around.

Edit: never mind, I’ve updated it to say std::filesystem::

vasild commented at 3:02 pm on February 15, 2022: member

@w0xlt, @laanwj, I don’t have a buggy gcc to experiment, but my guess is that if signet is a symlink and inside blocks is an ordinary directory, then is_symlink("signet/blocks") will return false. In this case our condition will be false and we will call the buggy/system create_directories(). It should return false in that case but maybe it throws instead because of the bug?

w0xlt commented at 4:22 pm on February 15, 2022: contributor

My bad. The symlink was created using a relative path instead of an absolute one. Tested with absolute path on Ubuntu 20.04 and this patch worked.

vasild commented at 4:43 pm on February 15, 2022: member

@w0xlt, so the link was broken, pointing to a nonexistent file/directory?

w0xlt commented at 4:54 pm on February 15, 2022: contributor

Yes, the link was broken. The relative path did not exist in the bitcoind execution context.

laanwj commented at 9:14 am on February 16, 2022: member

Thanks for double-checking @w0xlt.

hebasto commented at 9:54 am on February 16, 2022: member

Concept ACK, reviewing…

hebasto commented at 11:17 am on February 16, 2022: member

Approach ACK b9dff100ff2e7dfb9f216e98964fe28ef5bac3a9.

Maybe add a unit test which being applied to the master branch passes on systems that have libstdc++ already being patched?

vasild approved

vasild commented at 1:51 pm on February 16, 2022: member

ACK b9dff100ff2e7dfb9f216e98964fe28ef5bac3a9

I managed to reproduce this and played a bit with it. This PR will fix the problem where the last component of the path is a symlink to a directory (instead of just directory).

Assuming buggy system create_directories(), then in some cases our newly introduced create_directories() still deviates from the standard - e.g. if the last component exists but is not a directory, then we would call the system function which will throw, whereas, according to the standard it should just return false. I think this is ok.

laanwj commented at 3:04 pm on February 16, 2022: member

Added @hebasto’s unit test (and using fully qualified std::filesystem::is_symlink(p) && std::filesystem::is_directory(p)). Thanks!

laanwj force-pushed on Feb 16, 2022

vasild approved

vasild commented at 3:22 pm on February 16, 2022: member

ACK d035b64aa0d1b23f34698cc20df65dd613b4adf7

in src/test/fs_tests.cpp:158 in d035b64aa0 outdated

117@@ -118,4 +118,26 @@ BOOST_AUTO_TEST_CASE(fsbridge_fstream)
118     }
119 }
120 
121-BOOST_AUTO_TEST_SUITE_END()
122\ No newline at end of file
123+BOOST_AUTO_TEST_CASE(create_directories)
124+{
125+    // Test fs::create_directories workaround.

laanwj commented at 3:26 pm on February 16, 2022:

I wonder if this works on windows? If not, we may want to disable this test there.

Edit: apparently not, looking at CI, will add guard

laanwj force-pushed on Feb 16, 2022

laanwj commented at 4:48 pm on February 16, 2022: member

New update:

Guard symlink test to not run on Windows.
Add feature_dirsymlinks functional test that tests running with symlinked blocks and chainstate directories (might also want to test individual symlinked block files, but out of scope for this PR).

w0xlt approved

w0xlt commented at 8:10 pm on February 16, 2022: contributor

tACK 67019cd on Ubuntu 20.04

in test/functional/feature_dirsymlinks.py:16 in 67019cde22 outdated

11+from test_framework.test_framework import BitcoinTestFramework, SkipTest
12+
13+
14+def rename_and_link(fromname, toname):
15+    os.rename(fromname, toname)
16+    os.symlink(toname, fromname)

jonatack commented at 9:54 pm on February 16, 2022:

pico nit if need to retouch, snakecase from_name, to_name and use named arguments in the callers (feel free to ignore)

laanwj commented at 8:41 am on February 17, 2022:

Yea, that’d be slightly better.

jonatack commented at 9:56 pm on February 16, 2022: member

Wasn’t able to make the tests fail using gcc 8.4 without the fix but am on debian testing and turns out it has libstdc++ patched. Hopefully another reviewer can see them fail without the fix (thanks for adding them).

re-ACK 67019cde228feef8c41e6c685d62a21e85a2cfd9

in src/test/fs_tests.cpp:155 in 67019cde22 outdated

117@@ -118,4 +118,28 @@ BOOST_AUTO_TEST_CASE(fsbridge_fstream)
118     }
119 }
120 
121-BOOST_AUTO_TEST_SUITE_END()
122\ No newline at end of file
123+#ifndef WIN32

hebasto commented at 11:29 pm on February 16, 2022:

Could be narrowed to __MINGW64__, but WIN32 is ok as it is coordinated with a new feature_dirsymlinks.py test.

laanwj commented at 8:40 am on February 17, 2022:

WIN32 is “all windows” so I think this is correct. No need to narrow it if we know it’s true for all.

hebasto commented at 8:46 am on February 17, 2022:

I mean, this test, being applied on top of the master branch, passes for MSVC build.

hebasto approved

hebasto commented at 11:30 pm on February 16, 2022: member

ACK 67019cde228feef8c41e6c685d62a21e85a2cfd9

laanwj commented at 8:39 am on February 17, 2022: member

Haven’t been able to make the new unit or functional test fail without the fix commit. With debian testing, tried compiling with an older gcc 8 (Debian 8.4.0-7). Any tips?

Ubuntu 20.04 is still unpatched, seems the best OS to test this on. Haven’t tried it on 22.04 yet (edit: just did—also not patched yet). It makes sense for debian testing to be ahead.

It would be kind of funny if by the time we release, all distros have been patched. That said, the tests are useful to keep.

DrahtBot commented at 10:00 am on February 17, 2022: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#24331 (util: Revert back MoveFileExW call for MinGW-w64 by hebasto)
#24169 (build: Add –enable-c++20 option by MarcoFalke)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

util: Work around libstdc++ create_directories issue

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
only worked around one instance of the problem.

The issue was fixed upstream in
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.

1f46b6e46e

test: Add fs_tests/create_directories unit test ddb75c2e87

test: Add functional test for symlinked blocks directory b223c3c21e

DrahtBot added the label Needs rebase on Feb 17, 2022

laanwj force-pushed on Feb 17, 2022

laanwj commented at 11:52 am on February 17, 2022: member

Rebased for #24331 and addressed @jonatack’s python comment.

hebasto approved

hebasto commented at 12:08 pm on February 17, 2022: member

re-ACK b223c3c21e89f6af76b5401413880923f7c444d6

jonatack commented at 1:05 pm on February 17, 2022: member

Ubuntu 20.04 is still unpatched, seems the best OS to test this on. Haven’t tried it on 22.04 yet (edit: just did—also not patched yet). It makes sense for debian testing to be ahead.

It would be kind of funny if by the time we release, all distros have been patched. That said, the tests are useful to keep.

:+1:

re-ACK b223c3c21e89f6af76b5401413880923f7c444d6 per git range-diff df08250 67019cd b223c3c

Nice touch adding the splat to rename_and_link() to enforce the named args.

fanquake commented at 1:08 pm on February 17, 2022: member

Guix build:

 0bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
 1075732c9a4af5d0c3cc09bc7bf8c15ef41c62d0984c4556c2756fb1d684ddc11  guix-build-b223c3c21e89/output/aarch64-linux-gnu/SHA256SUMS.part
 2fd13a2477eb6400aa00200e46df3dd3734e54f19f156807ba8e0fe2d1659dafb  guix-build-b223c3c21e89/output/aarch64-linux-gnu/bitcoin-b223c3c21e89-aarch64-linux-gnu-debug.tar.gz
 38c1c6cd750fdf1cc6f3990a1d15a03076fcdb3fef76916359d76b11370f11e73  guix-build-b223c3c21e89/output/aarch64-linux-gnu/bitcoin-b223c3c21e89-aarch64-linux-gnu.tar.gz
 40d957e31ab5645703be7fc62d09c1096677c25b395f97bed11ae7bf1b4da0a44  guix-build-b223c3c21e89/output/arm-linux-gnueabihf/SHA256SUMS.part
 544fdc0ddfc86eab2a747f4087bbcf79b532b5d7dd5a77b1ef33b49f5c65a95b9  guix-build-b223c3c21e89/output/arm-linux-gnueabihf/bitcoin-b223c3c21e89-arm-linux-gnueabihf-debug.tar.gz
 6456b5b91302568e59ee8d5329d065efffcbd7fd76a064cd9c67e7f404dc7249d  guix-build-b223c3c21e89/output/arm-linux-gnueabihf/bitcoin-b223c3c21e89-arm-linux-gnueabihf.tar.gz
 73a0c3420152e95c88ac67ecf114e88a43af2b82b17cf5778b66645b4273bf995  guix-build-b223c3c21e89/output/arm64-apple-darwin/SHA256SUMS.part
 8c2f03f7866bdb1455a277dd2c2539facbf0a64ed256ab226e61422dec60098eb  guix-build-b223c3c21e89/output/arm64-apple-darwin/bitcoin-b223c3c21e89-arm64-apple-darwin.tar.gz
 9a2093272f16783e68a207fe7ece4f7159dea079c29315f72d4a995201542b514  guix-build-b223c3c21e89/output/arm64-apple-darwin/bitcoin-b223c3c21e89-osx-unsigned.dmg
107c98dcd13c45af6e4039a0cde255818552981c5690dedb764b89bc84c9b09d97  guix-build-b223c3c21e89/output/arm64-apple-darwin/bitcoin-b223c3c21e89-osx-unsigned.tar.gz
11c5e1be7e7fd68fc9b0cc87829c94c12dd00386905961b012f778cd1a2eaca1bc  guix-build-b223c3c21e89/output/dist-archive/bitcoin-b223c3c21e89.tar.gz
12b6044165141e6dbbf8a2deac093b46bc7216a8f8027a41d551eb6adeb6b35caa  guix-build-b223c3c21e89/output/powerpc64-linux-gnu/SHA256SUMS.part
13db025c0407b73aa6ba385d508d3cb9fc5483271fa92bc0cc09065f10230c5d3d  guix-build-b223c3c21e89/output/powerpc64-linux-gnu/bitcoin-b223c3c21e89-powerpc64-linux-gnu-debug.tar.gz
14ca4285de33997bd7e6cd3a9ce41038221c488d86c6dcd082c2479d8c18cd0c09  guix-build-b223c3c21e89/output/powerpc64-linux-gnu/bitcoin-b223c3c21e89-powerpc64-linux-gnu.tar.gz
15e12d1e7864f79a37639156751b3b03c4e2f9bf9ca53ac9ae279aeabaa1f692ba  guix-build-b223c3c21e89/output/powerpc64le-linux-gnu/SHA256SUMS.part
1694b7da0b74207974acb5a3d614397c04acb56a07ae1a97d6e782760ff24271e2  guix-build-b223c3c21e89/output/powerpc64le-linux-gnu/bitcoin-b223c3c21e89-powerpc64le-linux-gnu-debug.tar.gz
177e64a5d5a7f586244684a4c7b275640fb09b82b9591c48b0440b7e5ca16b9781  guix-build-b223c3c21e89/output/powerpc64le-linux-gnu/bitcoin-b223c3c21e89-powerpc64le-linux-gnu.tar.gz
1838277de2c4cc5c539796a92e4c97f4dec4eb8a39aee32af97a21fca02e0a3fa7  guix-build-b223c3c21e89/output/riscv64-linux-gnu/SHA256SUMS.part
19d89076a6d7c7d693aea0a3ca2ac49e76f2c72ae61261ba1623b34b2d03f4937f  guix-build-b223c3c21e89/output/riscv64-linux-gnu/bitcoin-b223c3c21e89-riscv64-linux-gnu-debug.tar.gz
2057741dadfbe3c8b744e3e700a1b8c314d442d99ef3aff281efe4428e5938e8f0  guix-build-b223c3c21e89/output/riscv64-linux-gnu/bitcoin-b223c3c21e89-riscv64-linux-gnu.tar.gz
21762aa6260628afbaf5cee1b60a4fcecc7de5453b1b526bbba222c85b33e1ac7f  guix-build-b223c3c21e89/output/x86_64-apple-darwin/SHA256SUMS.part
2221a5d675de49c2cc2c94406e1d5b5565c0c04d8c7f21b7d8fde111dd68ffcb9f  guix-build-b223c3c21e89/output/x86_64-apple-darwin/bitcoin-b223c3c21e89-osx-unsigned.dmg
231caf7a2115fc0aac3216cd06cdb4eb3c715299f6e707eb6ebe0cd62acffa2621  guix-build-b223c3c21e89/output/x86_64-apple-darwin/bitcoin-b223c3c21e89-osx-unsigned.tar.gz
2481b0cc3cbc308ebc6b778fdfc2f38c550e273883c47f909d12b7e096d3d950ca  guix-build-b223c3c21e89/output/x86_64-apple-darwin/bitcoin-b223c3c21e89-osx64.tar.gz
250a3b41c634a9952c0084fd43f757971e9b55dd099b850b4585621be263ed01cf  guix-build-b223c3c21e89/output/x86_64-linux-gnu/SHA256SUMS.part
26c0d461b1006201f2ae202019f06cb4709a2627f0e28a8cd9ebafed6ee8bd8231  guix-build-b223c3c21e89/output/x86_64-linux-gnu/bitcoin-b223c3c21e89-x86_64-linux-gnu-debug.tar.gz
2731bcf08af8012bfbff3e417c784ee0d546f916d24495143cff581a23e0bb9690  guix-build-b223c3c21e89/output/x86_64-linux-gnu/bitcoin-b223c3c21e89-x86_64-linux-gnu.tar.gz
28c9bb21542cdb876ef9e7dd4b4d18dda2955d56a003b5e06ff171f8480d52bb15  guix-build-b223c3c21e89/output/x86_64-w64-mingw32/SHA256SUMS.part
29ccf12d1929669f1edbe80496e7cd48b96eb336de596c46b666802c784aec9808  guix-build-b223c3c21e89/output/x86_64-w64-mingw32/bitcoin-b223c3c21e89-win-unsigned.tar.gz
304f91e09b2ba0b130e101ce3e8616463d8f4a72098c9adc1ac46fa7aa5bf92188  guix-build-b223c3c21e89/output/x86_64-w64-mingw32/bitcoin-b223c3c21e89-win64-debug.zip
316d40204e81732857cf28a32262e9ae4170714627efc81ebe7ddad7b50faa45b1  guix-build-b223c3c21e89/output/x86_64-w64-mingw32/bitcoin-b223c3c21e89-win64-setup-unsigned.exe
329c6881f8734ffa8b0765aa791b10c28e8ce0f983ca35b3ba2ddf2577173cdd58  guix-build-b223c3c21e89/output/x86_64-w64-mingw32/bitcoin-b223c3c21e89-win64.zip

DrahtBot removed the label Needs rebase on Feb 17, 2022

w0xlt approved

w0xlt commented at 3:00 pm on February 17, 2022: contributor

re-ACK b223c3c

in test/functional/feature_dirsymlinks.py:17 in b223c3c21e

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)

vasild commented at 3:10 pm on February 17, 2022:

This will only work with absolute paths.

0$ cd /tmp
1$ mkdir -p datadir/regtest/blocks
2
3$ mv datadir/regtest/blocks datadir/regtest/newblocks
4$ ln -s datadir/regtest/newblocks datadir/regtest/blocks
5$ ls -l datadir/regtest/blocks
6lrwxr-xr-x   datadir/regtest/blocks -> datadir/regtest/newblocks
7# now /tmp/datadir/regtest/blocks is a symlink to /tmp/datadir/regtest/datadir/regtest/newblocks

I think it is ok, coz self.nodes[0].datadir is probably an absolute path now, given that the test passes.

vasild commented at 3:27 pm on February 17, 2022:

I verified self.nodes[0].datadir is absolute even if a relative one is given on the command line, like: ./test/functional/feature_dirsymlinks.py --tmpdir=foo

laanwj commented at 8:03 am on February 18, 2022:

Right. It’s a function for the scope of this test, it doesn’t aim nor need to work for the general case.

vasild approved

vasild commented at 3:11 pm on February 17, 2022: member

ACK b223c3c21e89f6af76b5401413880923f7c444d6

fanquake merged this on Feb 17, 2022

fanquake closed this on Feb 17, 2022

sidhujag referenced this in commit fd145549cb on Feb 18, 2022

DrahtBot locked this on Feb 18, 2023

util: Work around libstdc++ create_directories issue #24338

Conflicts