Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use std::filesystem. Remove Boost Filesystem & System #20744

Merged
merged 4 commits into from Feb 3, 2022

Conversation

fanquake
Copy link
Member

@fanquake fanquake commented Dec 22, 2020

This PR replaces our Boost Filesystem usage with std::filesystem and includes dropping Boost System as a dependency. It includes a squashed down version of the changes from #19245.

A macro has been added, modelling how we check for -latomic to facilitate linking with -lstdc++fs when required. This is ~GCC < 9.1 & ~Clang < 9.0, however not always. i.e you could be using Clang 7 on top of a GCC 9 installation (i.e Ubuntu Focal) and use <filesystem> without passing any additional arguments. I've tested this with GCC 8 on Bionic, Clang 7 on Focal & with Apple Clang 12.0.0 on macOS.

Guix build:

bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
c1f9b326f9be4140f00cebeae5ff8de428a2fb8ecced539fcc36c53f53bfecf4  guix-build-07269321f38e/output/aarch64-linux-gnu/SHA256SUMS.part
b44aca3bcf5ea92a3a6c48c24d6f85576f425f59b73528d4d00c20e950cf2128  guix-build-07269321f38e/output/aarch64-linux-gnu/bitcoin-07269321f38e-aarch64-linux-gnu-debug.tar.gz
27a5553f7bd14797293fc40c5fb131c91e98a61d5481a283f13a1d0497eb5ed8  guix-build-07269321f38e/output/aarch64-linux-gnu/bitcoin-07269321f38e-aarch64-linux-gnu.tar.gz
99e55a88823f6095864a09c9eaa824e24d9ec527380eb394f751c7205b930f69  guix-build-07269321f38e/output/arm-linux-gnueabihf/SHA256SUMS.part
b720b2724fa47fde584f58ed3b587f1d1183523540777fd367ab7e582605cfea  guix-build-07269321f38e/output/arm-linux-gnueabihf/bitcoin-07269321f38e-arm-linux-gnueabihf-debug.tar.gz
c19c247f4e9e0d7f888ac8ba9de1c12d382f48fa828a685d4fe02818a18abd1f  guix-build-07269321f38e/output/arm-linux-gnueabihf/bitcoin-07269321f38e-arm-linux-gnueabihf.tar.gz
55b49ccb38de03bb95101354a16fd8d2190abede5ccc0d9b00b40c0cd526a2f6  guix-build-07269321f38e/output/arm64-apple-darwin/SHA256SUMS.part
baa44752470a6be9acae1c2f8fd1b9bc37afb00971787ea11fbaeddc9ab7c4aa  guix-build-07269321f38e/output/arm64-apple-darwin/bitcoin-07269321f38e-arm64-apple-darwin.tar.gz
ad7df4d8026d5bcce1321cdccc2e1820e8a8bb7e1064ed16e20a7ea354057fd2  guix-build-07269321f38e/output/arm64-apple-darwin/bitcoin-07269321f38e-osx-unsigned.dmg
f342066dc34a14d67c47779a2413a14633a996e8e7ddca89ae0184e23ef99efd  guix-build-07269321f38e/output/arm64-apple-darwin/bitcoin-07269321f38e-osx-unsigned.tar.gz
f6905346a5d48f57805fb062d0247ab5007c89047070a0b3125941dd1a2b8aa6  guix-build-07269321f38e/output/dist-archive/bitcoin-07269321f38e.tar.gz
a1f6c4b2b118dbd89770801f0bcffd2218b82df408cd227e34c40493469bb7a2  guix-build-07269321f38e/output/powerpc64-linux-gnu/SHA256SUMS.part
ba8359426e523bf013d93579c1bedc57380214c8170a9743b64ec1a8a3bbccbf  guix-build-07269321f38e/output/powerpc64-linux-gnu/bitcoin-07269321f38e-powerpc64-linux-gnu-debug.tar.gz
b0bb500c274a683ea28ecbc1e8f18c618a9f8acb00045f80ae43c515288402c0  guix-build-07269321f38e/output/powerpc64-linux-gnu/bitcoin-07269321f38e-powerpc64-linux-gnu.tar.gz
38c85e9589e092cd3aa08996aa383c0ccd5c73208943389741355a6eb7f72537  guix-build-07269321f38e/output/powerpc64le-linux-gnu/SHA256SUMS.part
50fcba7942ff48d91e84c093fda0affc17e46167fe1d5137c6e14c5c41f289d1  guix-build-07269321f38e/output/powerpc64le-linux-gnu/bitcoin-07269321f38e-powerpc64le-linux-gnu-debug.tar.gz
fa08ef1ceca072e014faa95ffee945954b2976fa28f90926b87ab0e5f15f8ca5  guix-build-07269321f38e/output/powerpc64le-linux-gnu/bitcoin-07269321f38e-powerpc64le-linux-gnu.tar.gz
e52dd80a9c306d6aeb544acaa1f4ed560b6b92b5184764a05026d45451aa2e94  guix-build-07269321f38e/output/riscv64-linux-gnu/SHA256SUMS.part
864e0a16c485b4159cec3ee0a83b046f1b1c3bc821670011c5ac5cd09ddfb91f  guix-build-07269321f38e/output/riscv64-linux-gnu/bitcoin-07269321f38e-riscv64-linux-gnu-debug.tar.gz
4a061172181322e7ad0cf06405bf74f4c8683eaba3a67ecfd46158cba7627f62  guix-build-07269321f38e/output/riscv64-linux-gnu/bitcoin-07269321f38e-riscv64-linux-gnu.tar.gz
876d82251853205420dffe7237523fc6ee3d09f78bf74cc03dc71f548446f335  guix-build-07269321f38e/output/x86_64-apple-darwin/SHA256SUMS.part
3f82b2e62c60eee68e7b8fc28e4792e069e3c2cd780ee2d67290ca422cdbc47c  guix-build-07269321f38e/output/x86_64-apple-darwin/bitcoin-07269321f38e-osx-unsigned.dmg
4ccdd4c410cac9d627e54ce83ee4816608681735da3cb93c60c5eb70ca86337a  guix-build-07269321f38e/output/x86_64-apple-darwin/bitcoin-07269321f38e-osx-unsigned.tar.gz
2179d36b2f60e28c15262d4e51f27465b5ae077f60e550345e125683ca611066  guix-build-07269321f38e/output/x86_64-apple-darwin/bitcoin-07269321f38e-osx64.tar.gz
b377e72fe84b6a982b8d414d60c85e6319523dff50dc852a0ba907f6d850ddd0  guix-build-07269321f38e/output/x86_64-linux-gnu/SHA256SUMS.part
8547e2f582ce05ae6a6224793b64efb2eb63f2816bf0bed5d53fcc4786274597  guix-build-07269321f38e/output/x86_64-linux-gnu/bitcoin-07269321f38e-x86_64-linux-gnu-debug.tar.gz
83b64805aa39f31a6fa4c2ed41e029c3be084e6dea06b90fac1ebca5c95bce29  guix-build-07269321f38e/output/x86_64-linux-gnu/bitcoin-07269321f38e-x86_64-linux-gnu.tar.gz

Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK for 0.23 (earliest), given that this again requires a bunch of aggressive bumps.

ci/test/00_setup_env_native_nowallet.sh Outdated Show resolved Hide resolved
ci/test/00_setup_env_win64.sh Outdated Show resolved Hide resolved
configure.ac Outdated Show resolved Hide resolved
doc/dependencies.md Outdated Show resolved Hide resolved
src/rpc/blockchain.cpp Outdated Show resolved Hide resolved
src/test/util_tests.cpp Outdated Show resolved Hide resolved
src/wallet/wallettool.cpp Outdated Show resolved Hide resolved
@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 22, 2020

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #23931 (create bitcoin.conf on first run with template by prayank23)
  • #22472 (fuzz: Add environment option to keep /tmp/ clean by agroce)
  • #22380 (build: add and use C_STANDARD and CXX_STANDARD in depends by fanquake)
  • #17127 (util: Correct permissions for datadir and wallets subdir 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.

Copy link
Contributor

@mjdietzx mjdietzx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice‼️ Concept Ack

I didn't have any problems building and running some tests locally on my machine. Also, there's a minor TODO that kind of got lost in the mix here. I had a PR for it when we were on boost #20265

I rebased on your branch and tested it out, seems to work: mjdietzx@55bafb9 - not sure if you want to pull it into this PR, or once this get merged I'll re-open the PR

src/wallet/wallet.cpp Outdated Show resolved Hide resolved
Comment on lines 742 to 793
if (!BaseParams().DataDir().empty()) {
path /= BaseParams().DataDir();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar to "Why not just GetWalletDir() / name even if name is empty?", is another if branch really needed here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In commit "refactor: replace boost::filesystem with std::filesystem" (f449c26)

re: #20744 (comment)

similar to "Why not just GetWalletDir() / name even if name is empty?", is another if branch really needed here?

See #20744 (comment), this avoids adding an empty path component and trailing slash if the base datadir is empty

Copy link
Contributor

@kiminuo kiminuo Jan 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fanquake I believe this one should be:

path = fsbridge::AbsPathJoin(path, BaseParams().DataDir());

Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems good, but it would be nice to move code changes to their own PR separate from the build changes, so they can get better review and so the final code is simpler. (It would be nice if final code wasn't checking for empty strings multiple places and wasn't calling fs::absolute on paths already known to be absolute.)

I think an initial PR simplifying this one would just replace fs::system_complete(x) with fs::absolute(x) and replace fs::absolute(x, y) with AbsPathJoin(y, x) everywhere. AbsPathJoin would be a util function defined like:

fs::path AbsPathJoin(fs::path p1, fs::path p2)
{
    assert(p1.is_absolute());
    return boost::filesystem::absolute(p2, p1);
}

Then I think this PR wouldn't need to touch any meaningful code except one line in AbsPathJoin:

fs::path AbsPathJoin(fs::path p1, fs::path p2)
{
    assert(p1.is_absolute());
    return p2.empty() ? p1 : p1 / p2;
}

src/wallet/load.cpp Outdated Show resolved Hide resolved
@kiminuo
Copy link
Contributor

kiminuo commented Dec 23, 2020

@ryanofsky It is maybe worth reading "Differences between filesystem implementations that affects Bitcoin code" in #19245.

@ryanofsky
Copy link
Contributor

ryanofsky commented Dec 23, 2020

@ryanofsky It is maybe worth reading "Differences between filesystem implementations that affects Bitcoin code" in #19245.

Thanks, I saw it but don't think it takes into account the "appends path::preferred_separator" behavior described https://en.cppreference.com/w/cpp/filesystem/path/append which can result in trailing slashes and is different from boost::absolute behavior https://www.boost.org/doc/libs/1_66_0/libs/filesystem/doc/reference.html#absolute.

Also I think it is clearer and better to avoid calling fs::absolute on paths that we already know are absolute. Ideally we would go further and never call fs::absolute or fs::system_complete after daemonization. Behavior of code shouldn't change depending on the process working directory, and in most cases it makes sense for usability to interpret paths relative to the data directory or wallet directory, not the process working directory.

@practicalswift
Copy link
Contributor

Concept ACK

@DrahtBot
Copy link
Contributor

🕵️ @laanwj @harding have been requested to review this pull request as specified in the REVIEWERS file.

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 6, 2022
This should also fix an assert error if a -datadir with a trailing slash
is used on windows. This appears to be a real error and regression
introduced with bitcoin#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 fix adds a
path::lexically_normal() call to the failing assert so it passes.
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 6, 2022
…std::string

824e1ff bench: Represents paths with fs::path instead of std::string (Ryan Ofsky)

Pull request description:

  Suggested bitcoin#20744 (comment)

ACKs for top commit:
  fanquake:
    untested ACK 824e1ff
  hebasto:
    ACK 824e1ff, tested on Linux Mint 20.2 (x86_64).

Tree-SHA512: 348fc189f30b5ad9a8e49e95e535d2c044462a9d534c3f34d887fbde0c05c41e88e02b4fc340709e6395a1188496a8333eb9e734310aa4c41755ec080e53c06e
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 6, 2022
…#20744

d216bc8 Re-enable walletinit_verify_walletdir_no_trailing2 test disabled in bitcoin#20744 (Ryan Ofsky)
80cd64e Re-enable util_datadir check disabled in bitcoin#20744 (Ryan Ofsky)

Pull request description:

  Reenable some broken tests as discussed bitcoin#20744 (comment) and bitcoin#20744 (comment)

  Fix windows test cases broken in bitcoin#20744, by passing normalized path arguments to fs::equivalent, fs::exists, and fs::is_directory, instead of non-normalized arguments. Also re-enable the tests.

  It is possible these changes also fix real init behavior on windows when -datadir or -walletdir paths with trailing dots or dashes are used, but it's not clear because I only tested on wine.

ACKs for top commit:
  hebasto:
    ACK d216bc8, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: 2099ddfa1a3ad70f7ac2ff413929414a1851d257b280da25c0f5cefb46fd1372b580a1f1ee5280681a1c16e6031f119185cadd4f7a6121298562cf001f711068
domob1812 added a commit to domob1812/namecoin-core that referenced this pull request Feb 7, 2022
It seems bitcoin/bitcoin#20744 broke the build,
thus we are including a temporary partial rollback (still linking against
Boost system & filesystem in configure.ac).  Hopefully this will get
fixed upstream, then we can remove the hack.
@maflcko
Copy link
Member

maflcko commented Feb 7, 2022

@DrahtBot
Copy link
Contributor

DrahtBot commented Feb 8, 2022

Guix builds

File commit 133f73e
(master)
commit c22ce78
(master and this pull)
SHA256SUMS.part 4d29ebeb3309d60d... fe3150bbd0109d5f...
*-aarch64-linux-gnu-debug.tar.gz c29ba0d0426063e8... e16ef933ef09c924...
*-aarch64-linux-gnu.tar.gz e52c846b1841b3eb... 89b610a00c36b994...
*-arm-linux-gnueabihf-debug.tar.gz 5d3f9731cf88da5b... 4c1e672b25015b27...
*-arm-linux-gnueabihf.tar.gz 282b33b13dd1f8a0... 16dfc5343616a8f2...
*-arm64-apple-darwin.tar.gz 3f0dba0a6549c410... 68cf545be72ba21c...
*-osx-unsigned.dmg 6692809452b6cb62... 1d7e80f446d77db4...
*-osx-unsigned.tar.gz 4366f453672f800d... 135c65dd5f337523...
*-osx64.tar.gz 5387d0cdc36be37a... aa75f3413a4a724d...
*-powerpc64-linux-gnu-debug.tar.gz 1ed57908059941ef... efa6d04faf2798ae...
*-powerpc64-linux-gnu.tar.gz 21079e1bae0f459b... adcc9fe3a4e926d6...
*-powerpc64le-linux-gnu-debug.tar.gz 6b441c519f2d3185... d6f4c9fb1386b4e5...
*-powerpc64le-linux-gnu.tar.gz 132f95573d0fd005... affab9079ecc51be...
*-riscv64-linux-gnu-debug.tar.gz 3c5e1f8e3d9aa92a... 4acf9cee4026d4a8...
*-riscv64-linux-gnu.tar.gz b17efbca76425fc5... 0a8151ca3ddd5261...
*-x86_64-linux-gnu-debug.tar.gz 9f61cd7fca8d6425... 19538169ad68c373...
*-x86_64-linux-gnu.tar.gz 323dc8d7d0aa8290... 4642e4b995756947...
*.tar.gz 5887839fbd29cd1c... a83d8979a6d86f22...
guix_build.log 53868781dafe6675... 6a54af53a5cc64da...
guix_build.log.diff 3887c19da2411792...

fanquake added a commit that referenced this pull request Feb 9, 2022
ebda2b8 util: Drop no longer needed StripRedundantLastElementsOfPath() function (Hennadii Stepanov)
ecd094e Use ArgsManager::GetPathArg() for "-walletdir" option (Hennadii Stepanov)
06fed4c Use ArgsManager::GetPathArg() for "-blocksdir" option (Hennadii Stepanov)
15b632b Use ArgsManager::GetPathArg() for "-datadir" option (Hennadii Stepanov)
540ca51 util: Add ArgsManager::GetPathArg() function (Hennadii Stepanov)

Pull request description:

  [Switching](#20744) to `std::filesystems` makes possible to leverage [`std::filesystem::path::lexically_normal`](https://en.cppreference.com/w/cpp/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.

ACKs for top commit:
  ryanofsky:
    Code review ACK ebda2b8. Only change since last review is rebase which simplified the last commit

Tree-SHA512: ed86959b6038b7152b5a1d473478667b72caab1716cc9149e1a75833d50511f22157e4e5e55a9465d1fa76b90bce5e5286f4e4f0d1ae65ebd9c012fae19d835f
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 10, 2022
ebda2b8 util: Drop no longer needed StripRedundantLastElementsOfPath() function (Hennadii Stepanov)
ecd094e Use ArgsManager::GetPathArg() for "-walletdir" option (Hennadii Stepanov)
06fed4c Use ArgsManager::GetPathArg() for "-blocksdir" option (Hennadii Stepanov)
15b632b Use ArgsManager::GetPathArg() for "-datadir" option (Hennadii Stepanov)
540ca51 util: Add ArgsManager::GetPathArg() function (Hennadii Stepanov)

Pull request description:

  [Switching](bitcoin#20744) to `std::filesystems` makes possible to leverage [`std::filesystem::path::lexically_normal`](https://en.cppreference.com/w/cpp/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.

ACKs for top commit:
  ryanofsky:
    Code review ACK ebda2b8. Only change since last review is rebase which simplified the last commit

Tree-SHA512: ed86959b6038b7152b5a1d473478667b72caab1716cc9149e1a75833d50511f22157e4e5e55a9465d1fa76b90bce5e5286f4e4f0d1ae65ebd9c012fae19d835f
sh15h4nk pushed a commit to sh15h4nk/bitcoin that referenced this pull request Feb 13, 2022
Part of bitcoin#20744, but this can be done now, and will simplify the diff.
sh15h4nk pushed a commit to sh15h4nk/bitcoin that referenced this pull request Feb 13, 2022
This is needed to prevent compilation failures once boost is removed,
however is still correct to include now, and reduces the diff in bitcoin#20744.

<string> is extracted from the defines because it is used for Windows
and non-Windows code, i.e get_filesystem_error_message().
sh15h4nk pushed a commit to sh15h4nk/bitcoin that referenced this pull request Feb 13, 2022
Also uses fs::path quoting in bench printed strings and fixes a
misleading error message.

Originally suggested bitcoin#20744 (comment)

Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
sh15h4nk pushed a commit to sh15h4nk/bitcoin that referenced this pull request Feb 13, 2022
This should also fix an assert error if a -datadir with a trailing slash
is used on windows. This appears to be a real error and regression
introduced with bitcoin#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 fix adds a
path::lexically_normal() call to the failing assert so it passes.
sh15h4nk pushed a commit to sh15h4nk/bitcoin that referenced this pull request Feb 13, 2022
…itcoin#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 bitcoin#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.
rebroad pushed a commit to rebroad/bitcoin that referenced this pull request Feb 15, 2022
Also uses fs::path quoting in bench printed strings and fixes a
misleading error message.

Originally suggested bitcoin#20744 (comment)

Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
rebroad pushed a commit to rebroad/bitcoin that referenced this pull request Feb 15, 2022
This should also fix an assert error if a -datadir with a trailing slash
is used on windows. This appears to be a real error and regression
introduced with bitcoin#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 fix adds a
path::lexically_normal() call to the failing assert so it passes.
rebroad pushed a commit to rebroad/bitcoin that referenced this pull request Feb 15, 2022
…itcoin#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 bitcoin#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.
janus pushed a commit to BitgesellOfficial/bitgesell that referenced this pull request Jul 24, 2022
Also uses fs::path quoting in bench printed strings and fixes a
misleading error message.

Originally suggested bitcoin/bitcoin#20744 (comment)

Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Sep 14, 2022
Summary:
This adds better test coverage and will make it easier in [[bitcoin/bitcoin#20744 | core#20744]] to remove our dependency on the two-argument boost::filesystem::absolute() function which does not have a direct equivalent in C++17.

This is a backport of [[bitcoin/bitcoin#20932 | core#20932]] [2/2]
bitcoin/bitcoin@da9caa1

Depends on D11970

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, sdulfari

Reviewed By: #bitcoin_abc, sdulfari

Differential Revision: https://reviews.bitcoinabc.org/D11971
dekm pushed a commit to unigrid-project/daemon that referenced this pull request Oct 27, 2022
a43b8e9 build: set OSX_MIN_VERSION to 10.15 (fanquake)

Pull request description:

  Taken out of bitcoin#20744, as splitting up some of the build changes was mentioned [here](bitcoin#22937 (comment)).

  This is required to use `std::filesystem` on macOS, as support for it only landed in the libc++.dylib shipped with 10.15. So if we want to move to using `std::filesystem` for `23.0`, this bump is required.

  See also: https://developer.apple.com/documentation/xcode-release-notes/xcode-11-release-notes

  > Clang now supports the C++17 \<filesystem\> library for iOS 13, macOS 10.15, watchOS 6, and tvOS 13.

  macOS 10.15 was released in October 2019. macOS OS's seem to have a life of about 3 years, so it's possible that 10.14 will become officially unsupported by the end of 2021 and prior to the release of 23.0.

  Guix builds:
  ```bash
  bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
  abc8b749be65f1339dcdf44bd1ed6ade2533b8e3b5030ad1dde0ae0cede78136  guix-build-a43b8e955558/output/dist-archive/bitcoin-a43b8e955558.tar.gz
  1edcc301eb4c02f3baa379beb8d4c78e661abc24a293813bc9d900cf7255b790  guix-build-a43b8e955558/output/x86_64-apple-darwin19/SHA256SUMS.part
  e9dbb5594a664519da778dde9ed861c3f0f631525672e17a67eeda599f16ff44  guix-build-a43b8e955558/output/x86_64-apple-darwin19/bitcoin-a43b8e955558-osx-unsigned.dmg
  11b23a17c630dddc7594c25625eea3de42db50f355733b9ce9ade2d8eba3a8f3  guix-build-a43b8e955558/output/x86_64-apple-darwin19/bitcoin-a43b8e955558-osx-unsigned.tar.gz
  257ba64a327927f94d9aa0a68da3a2695cf880b3ed1a0113c5a966dcc426eb5e  guix-build-a43b8e955558/output/x86_64-apple-darwin19/bitcoin-a43b8e955558-osx64.tar.gz
  ```

ACKs for top commit:
  hebasto:
    ACK a43b8e9
  jarolrod:
    ACK a43b8e9

Tree-SHA512: 9ac77be7cb56c068578860a3b2b8b7487c9e18b71b14aedd77a9c663f5d4bb19756d551770c02ddd12f1797beea5757b261588e7b67fb53509bb998ee8022369
backpacker69 pushed a commit to peercoin/peercoin that referenced this pull request Jan 18, 2023
Also uses fs::path quoting in bench printed strings and fixes a
misleading error message.

Originally suggested bitcoin/bitcoin#20744 (comment)

Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Fabcien added a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Feb 27, 2023
Summary: A better solution would be to remove boost fs and boost system depedencies, but since it's a huge work (see bitcoin/bitcoin#20744) let's fix the annoying warnings for now.

Test Plan:
  ninja all check
Make sure there is no warning anymore

Reviewers: #bitcoin_abc, PiRK

Reviewed By: #bitcoin_abc, PiRK

Subscribers: PiRK

Differential Revision: https://reviews.bitcoinabc.org/D13175
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Apr 21, 2023
Summary:
This concludes backport of [[bitcoin/bitcoin#20744 | core#20744]]
bitcoin/bitcoin@b87f9c5
bitcoin/bitcoin@0726932

Depends on D11974

Test Plan: CI builds

Reviewers: #bitcoin_abc, sdulfari

Reviewed By: #bitcoin_abc, sdulfari

Differential Revision: https://reviews.bitcoinabc.org/D12608
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request May 10, 2023
84f9931 guix: use upstream python-requests (2.26.0) (fanquake)
187dc1e build: use python-asn1crypto from upstream (fanquake)
b1e8f0b guix: use uptream nsis-x86_64 (fanquake)
3ccfba1 guix: use GCC 10 (over GCC 8) to build releases (fanquake)

Pull request description:

  Guix's `core-updates-frozen` branch has been merged back into `master`, and a [`version-1.4.0`](https://git.savannah.gnu.org/cgit/guix.git/log/?h=version-1.4.0) branch has been created. This is great, as it means the next Guix release is on the horizon, and it contains a number of changes I'd like to take advantage of. In particular, is migrating the version of GCC we use for releases from GCC 8 to GCC 10.3.0 (which is also the new Guix default GCC). This is my preferred method of unblocking progress in bitcoin#20744, which is currently stalled due to support for `std::filesystem` for Windows not arriving in GCC until version 9, whereas it's usable on Linux starting with GCC 8. The current set of changes in that PR [attempt to backport support](bitcoin@9604eda) for `std::filesystem`, for Windows, to GCC 8, similar to what is currently done by Debian, however that is somewhat convoluted, and using GCC 10 with our current Guix version would require updating at least one Guix patch to GCC, so is not completely straightforward either.

  Other changes included here:
  * Dropping our `--no-*` patch for mingw binutils ld, as we can take advantage of the `--disable-*` flags that are now available in binutlils 2.37. The security check tests are updated accordingly.
  * Dropping our current patch for NSIS, as it's been integrated upstream, however given we are building v3.05, we need a different one (kichik/nsis@229b613) for compiling against GCC 10.
  * Removing our `python-asn1crypto` package definition, as an identical package is available in Guix. Over time we should look at trying to get the rest of the python packages we define here upstreamed.
  * Adding a patch for `python-elfsteem` to fix an issue in the example code when using Python 3.9+.
  * Our base glibc (`2.24`) now inherits from glibc-2.31. Guix has removed packages of glibc < 2.29, and the current version of glibc is `2.33`. However glibc-2.31 is the newest version that still contains a workaround for installing sunrpc data, which we need, so inheriting from that version seemed like the most straightforward solution.
  * As mentioned, Guix has removed glibcs < 2.29, so we add our own package definition for glibc 2.27, which we use for our RISC-V toolchain (also inheriting from 2.31).

  The guix commit hash currently points to the head of the `version-1.4.0` branch. This can be updated to an official release tag when one is available.

  Looking for Concept ACKs on migrating to using GCC 10.3 for building releases. Keeping in mind issues like bitcoin#20005, however that particular bug should be fixed in GCC 10.3.0+, according to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95189.

  Guix Builds:
  ```bash
  bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
  ea56ef38bd94dbcb11b9d10e2f10c205109daad03fea4313f79892fc497ba68d  guix-build-84f9931cb449/output/aarch64-linux-gnu/SHA256SUMS.part
  01123ab23e5a09dc06a897837389e859d302ba2b18fbe827936ec8983765e7df  guix-build-84f9931cb449/output/aarch64-linux-gnu/bitcoin-84f9931cb449-aarch64-linux-gnu-debug.tar.gz
  7a24e25c2237e5aeb14508b91c5c6954572814e1767e892c164494f32d73b0c0  guix-build-84f9931cb449/output/aarch64-linux-gnu/bitcoin-84f9931cb449-aarch64-linux-gnu.tar.gz
  0e1dba0233da1f487222b128964980d50393e61a6971bcf4c71951c29fdf3993  guix-build-84f9931cb449/output/arm-linux-gnueabihf/SHA256SUMS.part
  8cd4c6f42abc81427f1d2500f86daced2a4ee78882dd9d03b5a0211a1d96306e  guix-build-84f9931cb449/output/arm-linux-gnueabihf/bitcoin-84f9931cb449-arm-linux-gnueabihf-debug.tar.gz
  c180db6bffb1a54b6dc65929d86d5eba9adf876a28ad320590ed230233e57299  guix-build-84f9931cb449/output/arm-linux-gnueabihf/bitcoin-84f9931cb449-arm-linux-gnueabihf.tar.gz
  4efcda7b63646eb46dabea7122fb026f2c063d2919a9dcbbffbc0929b9c56ced  guix-build-84f9931cb449/output/dist-archive/bitcoin-84f9931cb449.tar.gz
  1e35e96034fed00674f362d6471fb402dd2758cec2860ded4fd7e37c38935a44  guix-build-84f9931cb449/output/powerpc64-linux-gnu/SHA256SUMS.part
  96a0b7f54d3b3935c134f8c2aaaf11a314b54c9d7924ba751503caa16bd1c840  guix-build-84f9931cb449/output/powerpc64-linux-gnu/bitcoin-84f9931cb449-powerpc64-linux-gnu-debug.tar.gz
  ae05137b6fb3494120f5413bf8a94ca3c1b0c047e1f512e6c2c5a0b1f122f075  guix-build-84f9931cb449/output/powerpc64-linux-gnu/bitcoin-84f9931cb449-powerpc64-linux-gnu.tar.gz
  c22e5fbcdcdbfa5d385537e2c1dab55004d9e94396ebccef0bc3d216edfacbbe  guix-build-84f9931cb449/output/powerpc64le-linux-gnu/SHA256SUMS.part
  52602b41e81a921435d93f2a3ae29549aa65a4147cdbf1ed7d9e4a44c4dc902a  guix-build-84f9931cb449/output/powerpc64le-linux-gnu/bitcoin-84f9931cb449-powerpc64le-linux-gnu-debug.tar.gz
  a2cc7e9385452163a7bda99f6f9aa630fd35d4ba13d4fd9a4dd7e8062206650d  guix-build-84f9931cb449/output/powerpc64le-linux-gnu/bitcoin-84f9931cb449-powerpc64le-linux-gnu.tar.gz
  e75fadf1b1c7e4ae3d52e7a8051a881de17bd4d9d32c1ca29ca0ddbb8028ee51  guix-build-84f9931cb449/output/riscv64-linux-gnu/SHA256SUMS.part
  3b643c33842a15befb5d36d13b598a5e628c11b95671336c8dea51b5eed9c79a  guix-build-84f9931cb449/output/riscv64-linux-gnu/bitcoin-84f9931cb449-riscv64-linux-gnu-debug.tar.gz
  e9a1ee7451502508cde73dc300aca8a421e379ac08c3f4adaf8c768fbfa942ac  guix-build-84f9931cb449/output/riscv64-linux-gnu/bitcoin-84f9931cb449-riscv64-linux-gnu.tar.gz
  c0508a0872cf1415a47983d2ebbc9e5a46282ce7b6453afac544e0d1315b7bf9  guix-build-84f9931cb449/output/x86_64-apple-darwin/SHA256SUMS.part
  7c02267cb91e2649088af5e96f81142beaad67f6a1a0588355174a4157b31458  guix-build-84f9931cb449/output/x86_64-apple-darwin/bitcoin-84f9931cb449-osx-unsigned.dmg
  46dbf5a911abfa63e3c5aa8440289da5fdea89da013253c08768ce58b798a99d  guix-build-84f9931cb449/output/x86_64-apple-darwin/bitcoin-84f9931cb449-osx-unsigned.tar.gz
  ab2e2360f18cb1b80bfd37f1a9508a938e89237767120472f932402cc809f0eb  guix-build-84f9931cb449/output/x86_64-apple-darwin/bitcoin-84f9931cb449-osx64.tar.gz
  f58aa000692f7ea09ab8e7ec159a806d3a665f0f70558e62a53d56afb361eb02  guix-build-84f9931cb449/output/x86_64-linux-gnu/SHA256SUMS.part
  78a76aef8469b07a41588e019a6dfa890c36fd5becf2c8d73a71c9e72bcabde6  guix-build-84f9931cb449/output/x86_64-linux-gnu/bitcoin-84f9931cb449-x86_64-linux-gnu-debug.tar.gz
  5e6e0040b37ff035de41c8fcfee5d498bd19fa489024704dd4caa0ab9f566450  guix-build-84f9931cb449/output/x86_64-linux-gnu/bitcoin-84f9931cb449-x86_64-linux-gnu.tar.gz
  d6e6af70f277d9c9ef9b4773ec05920355ac07ebec71ff3e179676047329964b  guix-build-84f9931cb449/output/x86_64-w64-mingw32/SHA256SUMS.part
  37f24f6899e7803ed07bd0f5eb3f0fb6237ac1254dd72f446e9e4e488a927c8e  guix-build-84f9931cb449/output/x86_64-w64-mingw32/bitcoin-84f9931cb449-win-unsigned.tar.gz
  14f7d1c14a5fc3b4c336d301f936c5578d6e31d61ec720dfc9d4129445d1e2a2  guix-build-84f9931cb449/output/x86_64-w64-mingw32/bitcoin-84f9931cb449-win64-debug.zip
  c8049dcc0308a76f21dd781e8561ebbafa84034fbf8e3afa7d4017866d7fd195  guix-build-84f9931cb449/output/x86_64-w64-mingw32/bitcoin-84f9931cb449-win64-setup-unsigned.exe
  fb1e6580c25b073118f121aabaa04aa09643bc97cfeaea7c9a24bbe65c33cbb6  guix-build-84f9931cb449/output/x86_64-w64-mingw32/bitcoin-84f9931cb449-win64.zip
  ```

ACKs for top commit:
  hebasto:
    re-ACK 84f9931

Tree-SHA512: 2f5f4f6bb1f55a048dba88523f235320e51c4af963355abf6a86b7035623b2100ae3dc44396c76fbeea89ae9cfbc5342abd3e2c41760ede8b689d7757d6e7f25
fanquake added a commit to fanquake/lncm-docker-bitcoind that referenced this pull request May 29, 2023
Neither of these are required to run Bitcoin Core 25.0 or 24.0,
see bitcoin/bitcoin#21064 and
bitcoin/bitcoin#20744.
@bitcoin bitcoin locked and limited conversation to collaborators Jun 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No match for fs::path operator+= when compiling with Boost 1.78 C++17 std::fs