Strip any trailing `/` in -datadir and -blocksdir paths #20080

hebasto commented at 6:03 PM on October 4, 2020: member

Wallet names in listwalletdir RPC are correct now, even if the -datadir path has any number of trailing /.

This PR is an alternative to #19933.

Fixes #19928.

in src/util/system.cpp:713 in de37a1de75 outdated

 709 | @@ -711,6 +710,7 @@ const fs::path &GetDataDir(bool fNetSpecific)
 710 |              path = "";
 711 |              return path;
 712 |          }
 713 | +        path = fs::canonical(path);

luke-jr commented at 6:34 PM on October 4, 2020:

This isn't in boost 1.47.0, which is our minimum required in older versions. Thus, it is ineligible for use in a bugfix.

hebasto commented at 6:40 PM on October 4, 2020:

Hmm,

$ git checkout v0.19.1
$ git grep -e 'fs::canonical' | wc -l
5

luke-jr commented at 6:49 PM on October 4, 2020:

lol, looks like #14146 was the culprit, released in v0.18.0.

Might as well just call it a bug in the dependency docs at this point I guess.

utACK

sipa commented at 6:53 PM on October 4, 2020:

According to doc/dependencies.md, our minimum supported Boost version is 1.58 ?

luke-jr commented at 6:55 PM on October 4, 2020:

@sipa that just changed in master (only).

sipa commented at 6:56 PM on October 4, 2020:

This PR is also fixing a bug that's only in master AFAIK?

luke-jr commented at 6:57 PM on October 4, 2020:

FWIW fs::canonical was introduced in Boost 1.48.

hebasto commented at 7:06 PM on October 4, 2020:

This PR is also fixing a bug that's only in master AFAIK?

Just tested 0.19.1. It has that bug too.

sipa commented at 7:08 PM on October 4, 2020:

Ah, my mistake.

luke-jr changes_requested

DrahtBot added the label Utils/log/libs on Oct 4, 2020

hebasto commented at 8:03 PM on October 4, 2020: member

@promag As an author of #14561, mind looking into his PR?

Saibato commented at 6:45 AM on October 5, 2020: contributor

from the other PR #19933 ... to strip correctly,without boost > 1.47 siggh.

So if we could really now use the 1.58 that approach here concept tACK, since i might have choose this also if i was sure we had 1.58 and after the comment from @promag

TyI: i still find in the compile cache after make distclean multiple lines like

/autom4te.cache/output.2:29476:  _AX_BOOST_BASE_TONUMERICVERSION_req="1.47.0"

But that's probably just trash form old version compiles?

hebasto commented at 8:12 AM on October 5, 2020: member

@Saibato

TyI: i still find in the compile cache after make distclean multiple lines like
/autom4te.cache/output.2:29476:  _AX_BOOST_BASE_TONUMERICVERSION_req="1.47.0"  

You need something like git clean -fdx to clean the autom4te.cache directory.

But that's probably just trash form old version compiles?

Yes, it is a cache from your old compiles.

MarcoFalke commented at 1:04 PM on October 5, 2020: member

review ACK de37a1de75859b85619382867a9755a8bb8ae457

hebasto commented at 1:24 PM on October 5, 2020: member

The utACK from @luke-jr is buried under "Show resolved":

lol, looks like #14146 was the culprit, released in v0.18.0.

Might as well just call it a bug in the dependency docs at this point I guess.

utACK

promag commented at 5:42 PM on October 5, 2020: member

ACK, Could add a test with trailing /?

MarcoFalke commented at 5:52 PM on October 5, 2020: member

I wrote a test when I reviewed this, btw:

diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
index 241c56934e..e9f2ba8e13 100644
--- a/src/test/util_tests.cpp
+++ b/src/test/util_tests.cpp
@@ -42,6 +42,17 @@ namespace BCLog {
 
 BOOST_FIXTURE_TEST_SUITE(util_tests, BasicTestingSetup)
 
+BOOST_AUTO_TEST_CASE(util_datadir)
+{
+    ClearDatadirCache();
+    const fs::path dd_norm = GetDataDir();
+    {
+        gArgs.ForceSetArg("-datadir", dd_norm.string() + "/");
+        ClearDatadirCache();
+        BOOST_CHECK_EQUAL(dd_norm, GetDataDir());
+    }
+}
+
 BOOST_AUTO_TEST_CASE(util_check)
 {
     // Check that Assert can forward

It fails on master:

test/util_tests.cpp(52): error: in "util_tests/util_datadir": check dd_norm == GetDataDir() has failed ["/tmp/test_common_Bitcoin Core/458e2f26ff0b2a2c4f007bdc719daaef01bf2da78133448fe78fddeb4f7f6c5d" != "/tmp/test_common_Bitcoin Core/458e2f26ff0b2a2c4f007bdc719daaef01bf2da78133448fe78fddeb4f7f6c5d/"]

Should pass on this branch.

hebasto commented at 6:20 PM on October 5, 2020: member

I wrote a test when I reviewed this, btw:

Thanks @MarcoFalke! Test added.

promag commented at 8:01 PM on October 5, 2020: member

Code review ACK b7c5a94ee1550f4f85b1889314f41b47fdf86cc2.

MarcoFalke commented at 5:50 AM on October 6, 2020: member

review ACK b7c5a94ee1550f4f85b1889314f41b47fdf86cc2 💥

<details><summary>Show signature and timestamp</summary>

Signature:

Timestamp of file with hash ddb64044682ce642ee34bc19a33fc5a043fa5665c44db97a466c620be17f3743 -

</details>

MarcoFalke added the label Needs backport (0.19) on Oct 6, 2020

MarcoFalke added the label Needs backport (0.18) on Oct 6, 2020

MarcoFalke added the label Needs backport (0.20) on Oct 6, 2020

in src/test/util_tests.cpp:50 in b7c5a94ee1 outdated

  41 | @@ -42,6 +42,17 @@ namespace BCLog {
  42 |  
  43 |  BOOST_FIXTURE_TEST_SUITE(util_tests, BasicTestingSetup)
  44 |  
  45 | +BOOST_AUTO_TEST_CASE(util_datadir)
  46 | +{
  47 | +    ClearDatadirCache();
  48 | +    const fs::path dd_norm = GetDataDir();
  49 | +    {
  50 | +        gArgs.ForceSetArg("-datadir", dd_norm.string() + "/");

promag commented at 7:50 AM on October 7, 2020:

nit, could add another case lile + "/./" to it's clear the result is canonical, not just without last /.

hebasto commented at 8:56 AM on October 7, 2020:

Thanks! Updated.

hebasto force-pushed on Oct 7, 2020

hebasto commented at 8:55 AM on October 7, 2020: member

Updated b7c5a94ee1550f4f85b1889314f41b47fdf86cc2 -> 3cd0a16d5a53e2e1d6aec30d67a4c449b5dd919c (pr20080.02 -> pr20080.03):

addressed @promag's comment:

nit, could add another case lile + "/./" to it's clear the result is canonical, not just without last /.

rebased to work around #20066

meshcollider commented at 9:18 AM on October 7, 2020: contributor

Sorry I may be confused about the boost versions that this requires, but if this needs boost 1.58, it can't actually be backported to the branches tagged can it? This works on master but only the alternative #19933 could be backported? EDIT: or does a boost version bump also apply to backport branches 🤔

hebasto commented at 9:47 AM on October 7, 2020: member

@meshcollider

Sorry I may be confused about the boost versions that this requires, but if this needs boost 1.58, it can't actually be backported to the branches tagged can it? This works on master but only the alternative #19933 could be backported? EDIT: or does a boost version bump also apply to backport branches thinking

There is a discussion above.

ghost commented at 8:51 PM on October 11, 2020: none

Concept ACK

While I was testing few things with wallet names on Windows and Ubuntu, I found that user can create wallets with names:

../testwallet for Linux ..\testwallet for Windows

It creates the wallet in testnet3 instead of testnet3/wallets in the case of Ubuntu and testnet3\wallets for Windows which is kind of directory traversal although this is not a web application however used by lot of web applications so maybe we can add some code to validate user's input for a wallet name before saving.

hebasto commented at 6:03 AM on October 12, 2020: member

@prayank23

... maybe we can add some code to validate user's input for a wallet name before saving.

Good idea for a new pr.

MarcoFalke commented at 11:22 AM on October 13, 2020: member

re-ACK 3cd0a16d5a , only change is adding even more tests 🦉

<details><summary>Show signature and timestamp</summary>

Signature:

Timestamp of file with hash 4e75eb321c4be66e5097c59196fb29d4285c9d8c96a0f6b0e3f80314a8e3612d -

</details>

MarcoFalke removed the label Needs backport (0.18) on Oct 13, 2020

MarcoFalke added the label Needs backport (0.18) on Oct 13, 2020

MarcoFalke referenced this in commit b9ac31f2d2 on Oct 13, 2020

hebasto commented at 4:01 PM on October 13, 2020: member

@meshcollider

Sorry I may be confused about the boost versions that this requires, but if this needs boost 1.58, it can't actually be backported to the branches tagged can it? This works on master but only the alternative #19933 could be backported? EDIT: or does a boost version bump also apply to backport branches thinking

Since #20142 has been merged, still you have concerns about backporting?

meshcollider commented at 7:25 AM on October 15, 2020: contributor

I really don't have a preference now for either this or #19933 but would like to see one of them picked so we can get the fix in soon.

laanwj added this to the milestone 0.21.0 on Oct 15, 2020

meshcollider commented at 7:23 PM on October 15, 2020: contributor

Code review ACK 3cd0a16d5a53e2e1d6aec30d67a4c449b5dd919c

MarcoFalke commented at 8:23 PM on October 15, 2020: member

Tested

master with trailing /

{
  "wallets": [
    {
      "name": "est_20080_wallet"
    }
  ]
}

this pull with trailing /:

{
  "wallets": [
    {
      "name": "test_20080_wallet"
    }
  ]
}

promag commented at 8:36 PM on October 15, 2020: member

Code review ACK 3cd0a16d5a53e2e1d6aec30d67a4c449b5dd919c.

Same canonical tests could be added for -blocksdir and -walletdir.

fanquake commented at 3:24 AM on October 16, 2020: member

When merged into master this currently breaks at least the feature_asmap.py & feature_filelock.py tests (and by the looks of it a few others others). Note the difference in file path /private/var/folders/z2/cn877.... vs /var/folders/z2/cn877....

1/1 - feature_filelock.py failed, Duration: 1 s

stdout:
2020-10-16T03:21:53.605000Z TestFramework (INFO): Initializing test directory /var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_112152/feature_filelock_0
2020-10-16T03:21:53.876000Z TestFramework (INFO): Using datadir /var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_112152/feature_filelock_0/node0/regtest
2020-10-16T03:21:53.876000Z TestFramework (INFO): Check that we can't start a second bitcoind instance using the same datadir
2020-10-16T03:21:54.132000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/Users/michael/github/bitcoin-merge-tree/test/functional/test_framework/test_node.py", line 485, in assert_start_raises_init_error
    self.wait_for_rpc_connection()
  File "/Users/michael/github/bitcoin-merge-tree/test/functional/test_framework/test_node.py", line 221, in wait_for_rpc_connection
    'bitcoind exited with status {} during initialization'.format(self.process.returncode)))
test_framework.test_node.FailedToStartError: [node 1] bitcoind exited with status 1 during initialization

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/michael/github/bitcoin-merge-tree/test/functional/test_framework/test_framework.py", line 126, in main
    self.run_test()
  File "/Users/michael/github/bitcoin-merge-tree/test/functional/feature_filelock.py", line 27, in run_test
    self.nodes[1].assert_start_raises_init_error(extra_args=['-datadir={}'.format(self.nodes[0].datadir), '-noserver'], expected_msg=expected_msg)
  File "/Users/michael/github/bitcoin-merge-tree/test/functional/test_framework/test_node.py", line 507, in assert_start_raises_init_error
    'Expected message "{}" does not fully match stderr:\n"{}"'.format(expected_msg, stderr))
  File "/Users/michael/github/bitcoin-merge-tree/test/functional/test_framework/test_node.py", line 162, in _raise_assertion_error
    raise AssertionError(self._node_msg(msg))
AssertionError: [node 1] Expected message "Error: Cannot obtain a lock on data directory /var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_112152/feature_filelock_0/node0/regtest. Bitcoin Core is probably already running." does not fully match stderr:
"Error: Cannot obtain a lock on data directory /private/var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_112152/feature_filelock_0/node0/regtest. Bitcoin Core is probably already running."
2020-10-16T03:21:54.189000Z TestFramework (INFO): Stopping nodes
2020-10-16T03:21:54.458000Z TestFramework (WARNING): Not cleaning up dir /var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_112152/feature_filelock_0
2020-10-16T03:21:54.458000Z TestFramework (ERROR): Test failed. Test logging available at /var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_112152/feature_filelock_0/test_framework.log
2020-10-16T03:21:54.458000Z TestFramework (ERROR): 
2020-10-16T03:21:54.459000Z TestFramework (ERROR): Hint: Call /Users/michael/github/bitcoin-merge-tree/test/functional/combine_logs.py '/var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_112152/feature_filelock_0' to consolidate all logs
2020-10-16T03:21:54.459000Z TestFramework (ERROR): 
2020-10-16T03:21:54.459000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2020-10-16T03:21:54.459000Z TestFramework (ERROR): https://github.com/bitcoin/bitcoin/issues
2020-10-16T03:21:54.459000Z TestFramework (ERROR):

and

Remaining jobs: [feature_asmap.py]
1/1 - feature_asmap.py failed, Duration: 5 s

stdout:
2020-10-16T03:17:37.222000Z TestFramework (INFO): Initializing test directory /var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_111736/feature_asmap_0
2020-10-16T03:17:37.781000Z TestFramework (INFO): Test bitcoind with no -asmap arg passed
2020-10-16T03:17:38.565000Z TestFramework (INFO): Test bitcoind -asmap=<absolute path>
2020-10-16T03:17:39.448000Z TestFramework (INFO): Test bitcoind -asmap=<relative path>
2020-10-16T03:17:41.854000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/Users/michael/github/bitcoin-merge-tree/test/functional/test_framework/test_framework.py", line 126, in main
    self.run_test()
  File "/Users/michael/github/bitcoin-merge-tree/test/functional/feature_asmap.py", line 99, in run_test
    self.test_asmap_with_relative_path()
  File "/Users/michael/github/bitcoin-merge-tree/test/functional/feature_asmap.py", line 64, in test_asmap_with_relative_path
    self.start_node(0, ['-asmap={}'.format(name)])
  File "/Users/michael/.pyenv/versions/3.5.6/lib/python3.5/contextlib.py", line 66, in __exit__
    next(self.gen)
  File "/Users/michael/github/bitcoin-merge-tree/test/functional/test_framework/test_node.py", line 393, in assert_debug_log
    self._raise_assertion_error('Expected messages "{}" does not partially match log:\n\n{}\n\n'.format(str(expected_msgs), print_log))
  File "/Users/michael/github/bitcoin-merge-tree/test/functional/test_framework/test_node.py", line 162, in _raise_assertion_error
    raise AssertionError(self._node_msg(msg))
AssertionError: [node 0] Expected messages "['Opened asmap file "/var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_\u20bf_🏃_20201016_111736/feature_asmap_0/node0/regtest/ASN_map" (59 bytes) from disk', 'Using asmap version fec61fa21a9f46f3b17bdcd660d7f4cd90b966aad3aec593c99b35f0aca15853 for IP bucketing']" does not partially match log:

 - 
 - 
 - 
 - 
 - 
 - 2020-10-16T03:17:39.947024Z [init] Bitcoin Core version v0.20.99.0-0627fcd97-dirty (release build)
 - 2020-10-16T03:17:39.947033Z [init] InitParameterInteraction: parameter interaction: -bind set -> setting -listen=1
 - 2020-10-16T03:17:39.947149Z [init] Validating signatures for all blocks.
 - 2020-10-16T03:17:39.947155Z [init] Setting nMinimumChainWork=0000000000000000000000000000000000000000000000000000000000000000
 - 2020-10-16T03:17:39.947225Z [init] Using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation
 - 2020-10-16T03:17:39.947232Z [init] Using RdSeed as additional entropy source
 - 2020-10-16T03:17:39.947235Z [init] Using RdRand as an additional entropy source
 - 2020-10-16T03:17:39.955613Z [init] Default data directory /Users/michael/Library/Application Support/Bitcoin
 - 2020-10-16T03:17:39.955630Z [init] Using data directory /private/var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_111736/feature_asmap_0/node0/regtest
 - 2020-10-16T03:17:39.955664Z [init] Config file: /private/var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_111736/feature_asmap_0/node0/bitcoin.conf
 - 2020-10-16T03:17:39.955685Z [init] Config file arg: regtest="1"
 - 2020-10-16T03:17:39.955699Z [init] Config file arg: [regtest] bind="127.0.0.1"
 - 2020-10-16T03:17:39.955711Z [init] Config file arg: [regtest] discover="0"
 - 2020-10-16T03:17:39.955721Z [init] Config file arg: [regtest] dnsseed="0"
 - 2020-10-16T03:17:39.955732Z [init] Config file arg: [regtest] fallbackfee="0.0002"
 - 2020-10-16T03:17:39.955743Z [init] Config file arg: [regtest] keypool="1"
 - 2020-10-16T03:17:39.955754Z [init] Config file arg: [regtest] listenonion="0"
 - 2020-10-16T03:17:39.955765Z [init] Config file arg: [regtest] port="11000"
 - 2020-10-16T03:17:39.955777Z [init] Config file arg: [regtest] printtoconsole="0"
 - 2020-10-16T03:17:39.955801Z [init] Config file arg: [regtest] rpcport="16000"
 - 2020-10-16T03:17:39.955811Z [init] Config file arg: [regtest] server="1"
 - 2020-10-16T03:17:39.955819Z [init] Config file arg: [regtest] shrinkdebugfile="0"
 - 2020-10-16T03:17:39.955826Z [init] Config file arg: [regtest] upnp="0"
 - 2020-10-16T03:17:39.955834Z [init] Setting file arg: wallet = [""]
 - 2020-10-16T03:17:39.955842Z [init] Command-line arg: asmap="ASN_map"
 - 2020-10-16T03:17:39.955852Z [init] Command-line arg: datadir="/var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_111736/feature_asmap_0/node0"
 - 2020-10-16T03:17:39.955861Z [init] Command-line arg: debug=""
 - 2020-10-16T03:17:39.955868Z [init] Command-line arg: debugexclude="libevent"
 - 2020-10-16T03:17:39.955875Z [init] Command-line arg: debugexclude="leveldb"
 - 2020-10-16T03:17:39.955883Z [init] Command-line arg: logthreadnames=""
 - 2020-10-16T03:17:39.955891Z [init] Command-line arg: logtimemicros=""
 - 2020-10-16T03:17:39.955898Z [init] Command-line arg: uacomment="testnode0"
 - 2020-10-16T03:17:39.955909Z [init] Using at most 125 automatic connections (2560 file descriptors available)
 - 2020-10-16T03:17:39.963108Z [init] Using 16 MiB out of 32/2 requested for signature cache, able to store 524288 elements
 - 2020-10-16T03:17:39.970015Z [init] Using 16 MiB out of 32/2 requested for script execution cache, able to store 524288 elements
 - 2020-10-16T03:17:39.970063Z [init] Script verification uses 7 additional threads
 - 2020-10-16T03:17:39.970285Z [scheduler] scheduler thread start
 - 2020-10-16T03:17:39.980399Z [init] Allowing HTTP connections from: 127.0.0.0/8 ::1/128 
 - 2020-10-16T03:17:39.980538Z [init] Binding RPC on address ::1 port 16000
 - 2020-10-16T03:17:39.982171Z [init] Binding RPC on address 127.0.0.1 port 16000
 - 2020-10-16T03:17:39.982232Z [init] Initialized HTTP server
 - 2020-10-16T03:17:39.982260Z [init] HTTP: creating work queue of depth 16
 - 2020-10-16T03:17:39.982280Z [init] Starting RPC
 - 2020-10-16T03:17:39.982300Z [init] Starting HTTP RPC server
 - 2020-10-16T03:17:39.982308Z [init] Using random cookie authentication.
 - 2020-10-16T03:17:39.982579Z [init] Generated RPC authentication cookie /private/var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_111736/feature_asmap_0/node0/regtest/.cookie
 - 2020-10-16T03:17:39.982615Z [init] Registering HTTP handler for / (exactmatch 1)
 - 2020-10-16T03:17:39.982626Z [init] Registering HTTP handler for /wallet/ (exactmatch 0)
 - 2020-10-16T03:17:39.982636Z [init] Starting HTTP server
 - 2020-10-16T03:17:39.982644Z [init] HTTP: starting 4 worker threads
 - 2020-10-16T03:17:39.982739Z [http] Entering http event loop
 - 2020-10-16T03:17:39.982763Z [init] Using wallet directory /private/var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_111736/feature_asmap_0/node0/regtest/wallets
 - 2020-10-16T03:17:39.982781Z [init] init message: Verifying wallet(s)...
 - 2020-10-16T03:17:39.983035Z [init] Using BerkeleyDB version Berkeley DB 4.8.30: (April  9, 2010)
 - 2020-10-16T03:17:39.983047Z [init] Using wallet /private/var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_111736/feature_asmap_0/node0/regtest/wallets/wallet.dat
 - 2020-10-16T03:17:39.983182Z [init] BerkeleyEnvironment::Open: LogDir=/private/var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_111736/feature_asmap_0/node0/regtest/wallets/database ErrorFile=/private/var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_111736/feature_asmap_0/node0/regtest/wallets/db.log
 - 2020-10-16T03:17:40.012525Z [init] init message: Loading banlist...
 - 2020-10-16T03:17:40.012608Z [init] Loaded 0 banned node ips/subnets from banlist.dat  0ms
 - 2020-10-16T03:17:40.012781Z [init] net: setting try another outbound peer=false
 - 2020-10-16T03:17:40.012796Z [init] SetNetworkActive: true
 - 2020-10-16T03:17:40.013676Z [init] Opened asmap file "/private/var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_111736/feature_asmap_0/node0/regtest/ASN_map" (59 bytes) from disk
 - 2020-10-16T03:17:40.013729Z [init] Using asmap version fec61fa21a9f46f3b17bdcd660d7f4cd90b966aad3aec593c99b35f0aca15853 for IP bucketing
 - 2020-10-16T03:17:40.013747Z [init] Cache configuration:
 - 2020-10-16T03:17:40.013760Z [init] * Using 2.0 MiB for block index database
 - 2020-10-16T03:17:40.013770Z [init] * Using 8.0 MiB for chain state database
 - 2020-10-16T03:17:40.013781Z [init] * Using 440.0 MiB for in-memory UTXO set (plus up to 286.1 MiB of unused mempool space)
 - 2020-10-16T03:17:40.013789Z [init] init message: Loading block index...
 - 2020-10-16T03:17:40.013807Z [init] Switching active chainstate to Chainstate [ibd] @ height -1 (null)
 - 2020-10-16T03:17:40.015353Z [init] Opening LevelDB in /private/var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_111736/feature_asmap_0/node0/regtest/blocks/index
 - 2020-10-16T03:17:40.044571Z [init] Opened LevelDB successfully
 - 2020-10-16T03:17:40.044647Z [init] Using obfuscation key for /private/var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_111736/feature_asmap_0/node0/regtest/blocks/index: 0000000000000000
 - 2020-10-16T03:17:40.044933Z [init] LoadBlockIndexDB: last block file = 0
 - 2020-10-16T03:17:40.044984Z [init] LoadBlockIndexDB: last block file info: CBlockFileInfo(blocks=1, size=293, heights=0...0, time=2011-02-02...2011-02-02)
 - 2020-10-16T03:17:40.045002Z [init] Checking all blk files are present...
 - 2020-10-16T03:17:40.045130Z [init] Opening LevelDB in /private/var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_111736/feature_asmap_0/node0/regtest/chainstate
 - 2020-10-16T03:17:40.073676Z [init] Opened LevelDB successfully
 - 2020-10-16T03:17:40.073830Z [init] Using obfuscation key for /private/var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_111736/feature_asmap_0/node0/regtest/chainstate: 02e5f93ad41deaad
 - 2020-10-16T03:17:40.074002Z [init] Loaded best chain: hashBestChain=0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206 height=0 date=2011-02-02T23:16:42Z progress=1.000000
 - 2020-10-16T03:17:40.074028Z [init] init message: Rewinding blocks...
 - 2020-10-16T03:17:40.074119Z [init] FlushStateToDisk: write block and undo data to disk started
 - 2020-10-16T03:17:40.077916Z [init] FlushStateToDisk: write block and undo data to disk completed (3.77ms)
 - 2020-10-16T03:17:40.077965Z [init] FlushStateToDisk: write block index to disk started
 - 2020-10-16T03:17:40.084112Z [init] FlushStateToDisk: write block index to disk completed (6.09ms)
 - 2020-10-16T03:17:40.084167Z [init] FlushStateToDisk: write coins cache to disk (0 coins, 0kB) started
 - 2020-10-16T03:17:40.084234Z [init] Writing final batch of 0.00 MiB
 - 2020-10-16T03:17:40.086971Z [init] Committed 0 changed transaction outputs (out of 0) to coin database...
 - 2020-10-16T03:17:40.087000Z [init] FlushStateToDisk: write coins cache to disk (0 coins, 0kB) completed (0.00s)
 - 2020-10-16T03:17:40.087025Z [init] Enqueuing ChainStateFlushed: block hash=0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206
 - 2020-10-16T03:17:40.087052Z [init] init message: Verifying blocks...
 - 2020-10-16T03:17:40.087083Z [init]  block index              74ms
 - 2020-10-16T03:17:40.087103Z [scheduler] ChainStateFlushed: block hash=0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206
 - 2020-10-16T03:17:40.088023Z [http] Received a POST request for / from 127.0.0.1:58861
 - 2020-10-16T03:17:40.088094Z [httpworker.0] ThreadRPCServer method=getblockcount user=__cookie__
 - 2020-10-16T03:17:40.088600Z [init] Reading estimates: 190 buckets counting confirms up to 48 blocks
 - 2020-10-16T03:17:40.088966Z [init] Reading estimates: 190 buckets counting confirms up to 12 blocks
 - 2020-10-16T03:17:40.090128Z [init] Reading estimates: 190 buckets counting confirms up to 1008 blocks
 - 2020-10-16T03:17:40.090506Z [init] init message: Loading wallet...
 - 2020-10-16T03:17:40.090642Z [init] BerkeleyEnvironment::Open: LogDir=/private/var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_111736/feature_asmap_0/node0/regtest/wallets/database ErrorFile=/private/var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_111736/feature_asmap_0/node0/regtest/wallets/db.log
 - 2020-10-16T03:17:40.103438Z [init] [default wallet] Wallet File Version = 169900
 - 2020-10-16T03:17:40.103476Z [init] [default wallet] Keys: 4 plaintext, 0 encrypted, 4 w/ metadata, 4 total. Unknown wallet records: 0
 - 2020-10-16T03:17:40.119166Z [init] [default wallet] Wallet completed loading in              29ms
 - 2020-10-16T03:17:40.119293Z [init] [default wallet] setKeyPool.size() = 2
 - 2020-10-16T03:17:40.119320Z [init] [default wallet] mapWallet.size() = 0
 - 2020-10-16T03:17:40.119335Z [init] [default wallet] m_address_book.size() = 3
 - 2020-10-16T03:17:40.119457Z [init] block tree size = 1
 - 2020-10-16T03:17:40.119477Z [init] nBestHeight = 0
 - 2020-10-16T03:17:40.119525Z [loadblk] loadblk thread start
 - 2020-10-16T03:17:40.119610Z [init] Bound to 127.0.0.1:11000
 - 2020-10-16T03:17:40.119673Z [init] Bound to 127.0.0.1:18445
 - 2020-10-16T03:17:40.119710Z [loadblk] Imported mempool transactions from disk: 0 succeeded, 0 failed, 0 expired, 0 already there, 0 waiting for initial broadcast
 - 2020-10-16T03:17:40.119766Z [loadblk] loadblk thread exit
 - 2020-10-16T03:17:40.119790Z [init] init message: Loading P2P addresses...
 - 2020-10-16T03:17:40.120132Z [init] Loaded 0 addresses from peers.dat  1ms
 - 2020-10-16T03:17:40.120225Z [init] Loaded 0 addresses from "anchors.dat"
 - 2020-10-16T03:17:40.122585Z [init] 0 block-relay-only anchors will be tried for connections.
 - 2020-10-16T03:17:40.122630Z [init] init message: Starting network threads...
 - 2020-10-16T03:17:40.122700Z [init] DNS seeding disabled
 - 2020-10-16T03:17:40.122763Z [net] net thread start
 - 2020-10-16T03:17:40.122810Z [opencon] opencon thread start
 - 2020-10-16T03:17:40.122870Z [msghand] msghand thread start
 - 2020-10-16T03:17:40.122917Z [addcon] addcon thread start
 - 2020-10-16T03:17:40.122980Z [init] init message: Done loading
 - 2020-10-16T03:17:40.345445Z [http] Received a POST request for / from 127.0.0.1:58862
 - 2020-10-16T03:17:40.345547Z [httpworker.3] ThreadRPCServer method=getblockcount user=__cookie__
 - 2020-10-16T03:17:40.346413Z [http] Received a POST request for / from 127.0.0.1:58862
 - 2020-10-16T03:17:40.346489Z [httpworker.1] ThreadRPCServer method=getmempoolinfo user=__cookie__


2020-10-16T03:17:41.910000Z TestFramework (INFO): Stopping nodes
2020-10-16T03:17:42.120000Z TestFramework (WARNING): Not cleaning up dir /var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_111736/feature_asmap_0
2020-10-16T03:17:42.121000Z TestFramework (ERROR): Test failed. Test logging available at /var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_111736/feature_asmap_0/test_framework.log
2020-10-16T03:17:42.121000Z TestFramework (ERROR): 
2020-10-16T03:17:42.122000Z TestFramework (ERROR): Hint: Call /Users/michael/github/bitcoin-merge-tree/test/functional/combine_logs.py '/var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20201016_111736/feature_asmap_0' to consolidate all logs
2020-10-16T03:17:42.122000Z TestFramework (ERROR): 
2020-10-16T03:17:42.122000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2020-10-16T03:17:42.122000Z TestFramework (ERROR): https://github.com/bitcoin/bitcoin/issues
2020-10-16T03:17:42.122000Z TestFramework (ERROR):

hebasto commented at 9:19 AM on October 16, 2020: member

A canonical path:

has no elements which are symbolic links

And I agree with @luke-jr's comment:

Does this resolve symbolic links? (I don't think that is proper behaviour, if so)

I think that we should handle symbolic links properly, at least for -datadir and -blocksdir. That's why I've change my mind, and I believe now this PR should be closed in favor of #19933. However, the latter is not ideal, it cannot cope with trailing //.

After 0.21 branching off and switching to C++17, we could use a path::lexically_normal.

So, please re-open #19933.

P.S.: It is not a big deal to fix functional tests on macOS, but this PR is wrong conceptually :)

MarcoFalke commented at 9:47 AM on October 16, 2020: member

I don't think we are going to switch to std::fs any time soon, at least doing so would mean dropping support for major OSs.

If the concept from #19933 is preferred, then at least put the sanitizing workaround here, not at the call site. Otherwise we'll have to copy-paste the workaround everywhere it is needed, instead of having it run once right after the datadir is parsed.

hebasto commented at 9:50 AM on October 16, 2020: member

I don't think we are going to switch to std::fs any time soon, at least doing so would mean dropping support for major OSs.

How is so?

If the concept from #19933 is preferred, then at least put the sanitizing workaround here, not at the call site. Otherwise we'll have to copy-paste the workaround everywhere it is needed, instead of having it run once right after the datadir is parsed.

Agree.

MarcoFalke commented at 9:52 AM on October 16, 2020: member

Agree

Would you mind doing here (and attributing @Saibato for the idea on the fix)?

hebasto commented at 9:55 AM on October 16, 2020: member

Agree

Would you mind doing here (and attributing @Saibato for the idea on the fix)?

With my pleasure!

hebasto force-pushed on Oct 16, 2020

hebasto renamed this:
~~Strip any trailing `/` in -datadir path~~
Strip any trailing `/` in -datadir and -blocksdir paths
on Oct 16, 2020

hebasto commented at 3:45 PM on October 16, 2020: member

Updated 3cd0a16d5a53e2e1d6aec30d67a4c449b5dd919c -> 2ed6c43cf662590358bb44d582940baee2df9ba5 (pr20080.03 -> pr20080.04):

reworked from scratch to keep symlinks unresolved @promag

Same canonical tests could be added for -blocksdir and -walletdir.

-blocksdir path is also stripped now.

hebasto force-pushed on Oct 16, 2020

DrahtBot commented at 9:11 PM on October 16, 2020: 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:

#19213 (refactor: Replace RecursiveMutex with Mutex in Get{Data,Blocks}Dir() by hebasto)

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.

MarcoFalke commented at 7:03 PM on October 27, 2020: member

review ACK 1c55140c3950a9c0c250f21e0a45f77e04efc5ec

util: Add StripRedundantLastElementsOfPath function

Co-authored-by: saibato <saibato.naga@pm.me>
Co-authored-by: MarcoFalke <falke.marco@gmail.com>

b19e88230f

doc: Update data directory path comments ad5cef5dfd

in src/util/system.cpp:682 in 1c55140c39 outdated

 679 | +
 680 | +fs::path StripRedundantLastElementsOfPath(const fs::path& path)
 681 | +{
 682 | +    auto result = path;
 683 | +    while (result.filename().string() == ".") {
 684 | +        result = StripLastElementOfPath(result);

MarcoFalke commented at 7:15 PM on October 27, 2020:

nit: Can be written shorter

        result = result.parent_path();

hebasto commented at 7:55 PM on October 27, 2020:

Thanks! Updated.

hebasto force-pushed on Oct 27, 2020

hebasto commented at 7:54 PM on October 27, 2020: member

Updated 1c55140c3950a9c0c250f21e0a45f77e04efc5ec -> ad5cef5dfdd5802fc187a52e74d940a52f420a51 (pr20080.06 -> pr20080.07, diff):

addressed @MarcoFalke's comment

MarcoFalke commented at 12:59 PM on October 28, 2020: member

review ACK ad5cef5dfd 🔙

<details><summary>Show signature and timestamp</summary>

Signature:

Timestamp of file with hash 9a7e9ae08d0fbf3f86d95725fe0b1a1d791fdc2c42f0f77c5e92653f4a6141d2 -

</details>

promag commented at 1:20 PM on October 28, 2020: member

Code review ACK ad5cef5dfdd5802fc187a52e74d940a52f420a51.

meshcollider commented at 10:40 PM on November 1, 2020: contributor

Code review + test run ACK ad5cef5dfdd5802fc187a52e74d940a52f420a51

meshcollider merged this on Nov 1, 2020

meshcollider closed this on Nov 1, 2020

sidhujag referenced this in commit 4477249b3e on Nov 2, 2020

hebasto deleted the branch on Nov 2, 2020

fanquake referenced this in commit 7566af419f on Nov 6, 2020

fanquake removed the label Needs backport (0.20) on Nov 6, 2020

fanquake commented at 8:05 AM on November 6, 2020: member

Added to #20166 for backporting to 0.20.

MarkLTZ referenced this in commit ed3cc9c4c3 on Nov 11, 2020

MarcoFalke referenced this in commit a2fa11f9de on Nov 18, 2020

MarkLTZ referenced this in commit ab145efc35 on Nov 21, 2020

MarkLTZ referenced this in commit c61e643b75 on Nov 21, 2020

fanquake removed the label Needs backport (0.18) on May 31, 2021

fanquake removed the label Needs backport (0.19) on May 31, 2021

meshcollider referenced this in commit 69577a27ab on Jun 9, 2021

sidhujag referenced this in commit a150948159 on Jun 9, 2021

deadalnix referenced this in commit 8172699324 on Dec 21, 2021

PastaPastaPasta referenced this in commit 436245d732 on Apr 15, 2023

PastaPastaPasta referenced this in commit 165ca7a39b on Apr 15, 2023

PastaPastaPasta referenced this in commit e2203f9470 on Apr 16, 2023

PastaPastaPasta referenced this in commit 0cd4553927 on Apr 16, 2023

UdjinM6 referenced this in commit 3aab3efaa5 on Apr 16, 2023

DrahtBot locked this on Jun 17, 2023