Page MenuHomePhabricator

Drop StripRedundantLastElementsOfPath() function
ClosedPublic

Authored by PiRK on Apr 16 2023, 12:02.

Details

Reviewers
Fabien
sdulfari
Group Reviewers
Restricted Project
Commits
rABC19b61626b64b: Drop StripRedundantLastElementsOfPath() function
Summary

Switching to std::filesystems makes possible to leverage std::filesystem::path::lexically_normal and get rid of ugly StripRedundantLastElementsOfPath() crutch.

To make its usage simple and error-proof, a new ArgsManager::GetPathArg() member function introduced which guarantees to return a normalized with no trailing slashes paths provided via -datadir, -blocksdir or -walletdir command-line arguments or configure options.

Re-enable tests disabled in #20744

This should also fix an init error if a -walletdir with a trailing slash
is used on windows. This appears to be a real error and regression
introduced with #20744.

On windows (or at least wine), fs calls that actuallly access the
filesystem like fs::equivalent or fs::exists seem to treat directory
paths with trailing slashes as not existing, so it's necessary to
normalize these paths before using them. This change passes canonical
paths to fs calls validating the -walletdir path to fix this.

Backport of core#24265 and core#24251.

Depends on D11974.

Test Plan
ninja all check-all

Run the windows cross build tests.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Tail of the build log:

[428/486] bitcoin: testing checkpoints_tests
[429/486] bitcoin: testing inv_tests
[430/486] Running utility command for check-bitcoin-fs_tests
[431/486] Running utility command for check-bitcoin-checkpoints_tests
[432/486] Running utility command for check-bitcoin-inv_tests
[433/486] bitcoin: testing sigencoding_tests
[434/486] Running utility command for check-bitcoin-sigencoding_tests
[435/486] bitcoin: testing script_tests
[436/486] bitcoin: testing key_io_tests
[437/486] bitcoin: testing scheduler_tests
[438/486] bitcoin: testing blockfilter_tests
[439/486] Running utility command for check-bitcoin-script_tests
[440/486] Running utility command for check-bitcoin-key_io_tests
[441/486] Running utility command for check-bitcoin-blockfilter_tests
[442/486] Running utility command for check-bitcoin-scheduler_tests
[443/486] bitcoin: testing coinstatsindex_tests
[444/486] Running utility command for check-bitcoin-coinstatsindex_tests
[445/486] bitcoin: testing scriptpubkeyman_tests
[446/486] Running utility command for check-bitcoin-scriptpubkeyman_tests
[447/486] bitcoin: testing txvalidationcache_tests
[448/486] Running utility command for check-bitcoin-txvalidationcache_tests
[449/486] bitcoin: testing walletdb_tests
[450/486] Running utility command for check-bitcoin-walletdb_tests
[451/486] bitcoin: testing crypto_tests
[452/486] bitcoin: testing logging_tests
[453/486] Running utility command for check-bitcoin-crypto_tests
[454/486] bitcoin: testing psbt_wallet_tests
[455/486] Running utility command for check-bitcoin-logging_tests
[456/486] bitcoin: testing cuckoocache_tests
[457/486] Running utility command for check-bitcoin-psbt_wallet_tests
[458/486] bitcoin: testing merkle_tests
[459/486] Running utility command for check-bitcoin-cuckoocache_tests
[460/486] Running utility command for check-bitcoin-merkle_tests
[461/486] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/wallettests.cpp.o
[462/486] bitcoin: testing denialofservice_tests
[463/486] bitcoin: testing init_tests
[464/486] Running utility command for check-bitcoin-denialofservice_tests
[465/486] Running utility command for check-bitcoin-init_tests
[466/486] bitcoin: testing rcu_tests
[467/486] Running utility command for check-bitcoin-rcu_tests
[468/486] Linking CXX executable src/qt/test/test_bitcoin-qt
[469/486] bitcoin: testing wallet_crypto_tests
[470/486] Running utility command for check-bitcoin-wallet_crypto_tests
[471/486] bitcoin: testing txrequest_tests
[472/486] Running utility command for check-bitcoin-txrequest_tests
[473/486] bitcoin: testing blockcheck_tests
[474/486] Running utility command for check-bitcoin-blockcheck_tests
[475/486] bitcoin-qt: testing test_bitcoin-qt
[476/486] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[477/486] bitcoin: testing coinselector_tests
[478/486] Running utility command for check-bitcoin-coinselector_tests
[479/486] bitcoin: testing wallet_tests
[480/486] Running utility command for check-bitcoin-wallet_tests
[481/486] bitcoin: testing transaction_tests
[482/486] Running utility command for check-bitcoin-transaction_tests
[483/486] bitcoin: testing coins_tests
[484/486] Running utility command for check-bitcoin-coins_tests
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang failed with exit code 1

Tail of the build log:

[421/479] Running utility command for check-bitcoin-script_tests
[422/479] bitcoin: testing coinstatsindex_tests
[423/479] bitcoin: testing fs_tests
[424/479] Running utility command for check-bitcoin-coinstatsindex_tests
[425/479] bitcoin: testing inv_tests
[426/479] Running utility command for check-bitcoin-fs_tests
[427/479] Running utility command for check-bitcoin-inv_tests
[428/479] bitcoin: testing checkpoints_tests
[429/479] Running utility command for check-bitcoin-checkpoints_tests
[430/479] bitcoin: testing txvalidationcache_tests
[431/479] Running utility command for check-bitcoin-txvalidationcache_tests
[432/479] bitcoin: testing blockfilter_tests
[433/479] bitcoin: testing key_io_tests
[434/479] bitcoin: testing scheduler_tests
[435/479] Running utility command for check-bitcoin-blockfilter_tests
[436/479] Running utility command for check-bitcoin-key_io_tests
[437/479] bitcoin: testing crypto_tests
[438/479] bitcoin: testing cuckoocache_tests
[439/479] Running utility command for check-bitcoin-scheduler_tests
[440/479] Running utility command for check-bitcoin-crypto_tests
[441/479] bitcoin: testing scriptpubkeyman_tests
[442/479] Running utility command for check-bitcoin-cuckoocache_tests
[443/479] Running utility command for check-bitcoin-scriptpubkeyman_tests
[444/479] bitcoin: testing walletdb_tests
[445/479] Running utility command for check-bitcoin-walletdb_tests
[446/479] bitcoin: testing merkle_tests
[447/479] Running utility command for check-bitcoin-merkle_tests
[448/479] bitcoin: testing psbt_wallet_tests
[449/479] Running utility command for check-bitcoin-psbt_wallet_tests
[450/479] bitcoin: testing miner_tests
[451/479] Running utility command for check-bitcoin-miner_tests
[452/479] bitcoin: testing denialofservice_tests
[453/479] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/paymentservertests.cpp.o
[454/479] bitcoin: testing init_tests
[455/479] Running utility command for check-bitcoin-denialofservice_tests
[456/479] Running utility command for check-bitcoin-init_tests
[457/479] bitcoin: testing rcu_tests
[458/479] Running utility command for check-bitcoin-rcu_tests
[459/479] bitcoin: testing wallet_crypto_tests
[460/479] Running utility command for check-bitcoin-wallet_crypto_tests
[461/479] bitcoin: testing txrequest_tests
[462/479] Running utility command for check-bitcoin-txrequest_tests
[463/479] bitcoin: testing coinselector_tests
[464/479] Running utility command for check-bitcoin-coinselector_tests
[465/479] bitcoin: testing blockcheck_tests
[466/479] Running utility command for check-bitcoin-blockcheck_tests
[467/479] bitcoin: testing wallet_tests
[468/479] Running utility command for check-bitcoin-wallet_tests
[469/479] bitcoin: testing transaction_tests
[470/479] Running utility command for check-bitcoin-transaction_tests
[471/479] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/addressbooktests.cpp.o
[472/479] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/wallettests.cpp.o
[473/479] Linking CXX executable src/qt/test/test_bitcoin-qt
[474/479] bitcoin: testing coins_tests
[475/479] Running utility command for check-bitcoin-coins_tests
[476/479] bitcoin-qt: testing test_bitcoin-qt
[477/479] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1

Tail of the build log:

wallet_multiwallet.py --usecli            | ✓ Passed  | 15 s
wallet_reorgsrestore.py                   | ✓ Passed  | 5 s
wallet_resendwallettransactions.py        | ✓ Passed  | 6 s
wallet_send.py                            | ✓ Passed  | 7 s
wallet_startup.py                         | ✓ Passed  | 3 s
wallet_timelock.py                        | ✓ Passed  | 1 s
wallet_txn_clone.py                       | ✓ Passed  | 2 s
wallet_txn_clone.py --mineblock           | ✓ Passed  | 4 s
wallet_txn_doublespend.py                 | ✓ Passed  | 2 s
wallet_txn_doublespend.py --mineblock     | ✓ Passed  | 4 s
wallet_watchonly.py                       | ✓ Passed  | 1 s
wallet_watchonly.py --usecli              | ✓ Passed  | 1 s
chronik_avalanche.py                      | ○ Skipped | 0 s
chronik_block.py                          | ○ Skipped | 0 s
chronik_disallow_prune.py                 | ○ Skipped | 0 s
chronik_resync.py                         | ○ Skipped | 0 s
chronik_script_confirmed_txs.py           | ○ Skipped | 0 s
chronik_script_history.py                 | ○ Skipped | 0 s
chronik_script_unconfirmed_txs.py         | ○ Skipped | 0 s
chronik_serve.py                          | ○ Skipped | 0 s
chronik_tx.py                             | ○ Skipped | 0 s
interface_usdt_net.py                     | ○ Skipped | 0 s
interface_usdt_utxocache.py               | ○ Skipped | 0 s
interface_usdt_validation.py              | ○ Skipped | 0 s

ALL                                       | ✓ Passed  | 1967 s (accumulated) 
Runtime: 394 s

[174/487] Test Bitcoin RPC authentication...
...
----------------------------------------------------------------------
Ran 3 tests in 0.020s

OK
[178/487] cd /work/contrib/devtools/chainparams && /usr/bin/python3.9 ./test_make_chainparams.py
.....
----------------------------------------------------------------------
Ran 5 tests in 0.001s

OK
[210/487] Running seeder test suite
PASSED: seeder test suite
[360/487] bitcoin: testing db_tests
FAILED: src/test/CMakeFiles/check-bitcoin-db_tests 
cd /work/abc-ci-builds/build-debug/src/test && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-debug/test/junit && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-debug/test/log && /usr/bin/cmake -E env /work/cmake/utils/log-and-print-on-failure.sh /work/abc-ci-builds/build-debug/test/log/bitcoin-db_tests.log /work/abc-ci-builds/build-debug/src/test/test_bitcoin --run_test=db_tests --logger=HRF,message:JUNIT,message,bitcoin-db_tests.xml --catch_system_errors=no
Running 4 test cases...
../../src/wallet/test/db_tests.cpp(30): error: in "db_tests/getwalletenv_file": check env->Directory() == datadir has failed

*** 1 failure is detected in the test module "Bitcoin ABC unit tests"
[455/487] Running avalanche test suite
PASSED: avalanche test suite
[458/487] Running pow test suite
PASSED: pow test suite
[459/487] Running secp256k1 test suite
PASSED: secp256k1 test suite
[470/487] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[484/487] Running utility command for check-bitcoin-coins_tests
ninja: build stopped: cannot make progress due to previous errors.
Build build-debug failed with exit code 1

Tail of the build log:

wallet_coinbase_category.py               | ✓ Passed  | 1 s
wallet_create_tx.py                       | ✓ Passed  | 5 s
wallet_createwallet.py                    | ✓ Passed  | 2 s
wallet_createwallet.py --descriptors      | ✓ Passed  | 2 s
wallet_createwallet.py --usecli           | ✓ Passed  | 3 s
wallet_descriptor.py                      | ✓ Passed  | 6 s
wallet_disable.py                         | ✓ Passed  | 1 s
wallet_dump.py                            | ✓ Passed  | 4 s
wallet_encryption.py                      | ✓ Passed  | 5 s
wallet_encryption.py --descriptors        | ✓ Passed  | 5 s
wallet_groups.py                          | ✓ Passed  | 11 s
wallet_hd.py                              | ✓ Passed  | 6 s
wallet_hd.py --descriptors                | ✓ Passed  | 5 s
wallet_import_rescan.py                   | ✓ Passed  | 5 s
wallet_import_with_label.py               | ✓ Passed  | 1 s
wallet_importdescriptors.py               | ✓ Passed  | 4 s
wallet_importmulti.py                     | ✓ Passed  | 3 s
wallet_importprunedfunds.py               | ✓ Passed  | 2 s
wallet_importprunedfunds.py --descriptors | ✓ Passed  | 1 s
wallet_keypool.py                         | ✓ Passed  | 2 s
wallet_keypool_topup.py                   | ✓ Passed  | 3 s
wallet_keypool_topup.py --descriptors     | ✓ Passed  | 3 s
wallet_labels.py                          | ✓ Passed  | 1 s
wallet_labels.py --descriptors            | ✓ Passed  | 2 s
wallet_listreceivedby.py                  | ✓ Passed  | 5 s
wallet_listsinceblock.py                  | ✓ Passed  | 5 s
wallet_listsinceblock.py --descriptors    | ✓ Passed  | 2 s
wallet_listtransactions.py                | ✓ Passed  | 4 s
wallet_listtransactions.py --descriptors  | ✓ Passed  | 3 s
wallet_multiwallet.py                     | ✓ Passed  | 40 s
wallet_multiwallet.py --usecli            | ✓ Passed  | 9 s
wallet_reorgsrestore.py                   | ✓ Passed  | 3 s
wallet_resendwallettransactions.py        | ✓ Passed  | 4 s
wallet_send.py                            | ✓ Passed  | 7 s
wallet_startup.py                         | ✓ Passed  | 2 s
wallet_timelock.py                        | ✓ Passed  | 1 s
wallet_txn_clone.py                       | ✓ Passed  | 1 s
wallet_txn_clone.py --mineblock           | ✓ Passed  | 2 s
wallet_txn_doublespend.py                 | ✓ Passed  | 1 s
wallet_txn_doublespend.py --mineblock     | ✓ Passed  | 2 s
wallet_watchonly.py                       | ✓ Passed  | 1 s
wallet_watchonly.py --usecli              | ✓ Passed  | 1 s
chronik_avalanche.py                      | ○ Skipped | 0 s
chronik_block.py                          | ○ Skipped | 0 s
chronik_disallow_prune.py                 | ○ Skipped | 0 s
chronik_resync.py                         | ○ Skipped | 0 s
chronik_script_confirmed_txs.py           | ○ Skipped | 0 s
chronik_script_history.py                 | ○ Skipped | 0 s
chronik_script_unconfirmed_txs.py         | ○ Skipped | 0 s
chronik_serve.py                          | ○ Skipped | 0 s
chronik_tx.py                             | ○ Skipped | 0 s
interface_usdt_net.py                     | ○ Skipped | 0 s
interface_usdt_utxocache.py               | ○ Skipped | 0 s
interface_usdt_validation.py              | ○ Skipped | 0 s

ALL                                       | ✓ Passed  | 1229 s (accumulated) 
Runtime: 246 s

ninja: build stopped: cannot make progress due to previous errors.
Build build-diff failed with exit code 1
PiRK added a reviewer: Fabien.

rebase
D13735 should fix the remaining redundant /

rebase and enable test that is now disabled in D11974

squash with core#24251 (additional fixes related to trailing slashes on windows)

@bot build-win64

PiRK published this revision for review.Apr 19 2023, 10:22
This revision is now accepted and ready to land.Apr 19 2023, 23:24