Replace boost::filesystem with std::filesystem

[kiminuo]

Introduction

C++17 has introduced the filesystem library. cppreference.com describes very well the origin of the library:

The filesystem library was originally developed as boost.filesystem, was published as the technical specification ISO/IEC TS 18822:2015, and finally merged to ISO C++ as of C++17. The boost implementation is currently available on more compilers and platforms than the C++17 library.

This draft PR was created to gain feedback and examine what would be necessary to switch from boost::filesystem to std::filesystem in bitcoin codebase.

Bitcoin codebase contains src/fs.h which is a wrapper for the currently used filesystem library. The first impression is that one may just replace namespace fs = boost::filesystem; with namespace fs = std::filesystem; and all will be good. Unfortunately, there are some differences between the Boost's filesystem library and the c++17 filesystem library. A nice summary by Davis Herring can be read here.

Differences between filesystem implementations that affects Bitcoin code

1. boost::filesystem::system_complete() was renamed to std::filesystem::absolute() [source]

2. boost::filesystem::absolute() has second const path& base parameter which was dropped in std::filesystem::absolute (see reasoning).

   Each such instance like this one can be fixed by changing:

   fs::path path = fs::absolute(request.params[0].get_str(), GetDataDir());

   to

   fs::path path = fs::absolute(GetDataDir() / request.params[0].get_str()); where / is function call of operator/ function.

   Note: Notably path("foo") / "c:/bar" yields c:/bar. However, that's how boost::filesystem::absolute(p, base_dir) behaves too.

3. boost::filesystem::path::imbue() is not present in std::filesystem::path.

4. boost::filesystem::unique_path() is not present in std::filesystem.

5. boost::filesystem::equivalent(path1, path2) differs from std::filesystem::equivalent(path1, path2) as the later reports not supported error if one of the files does not exist whereas Boost returns false.

   • See also cppreference.

Support for C++17 <filesystem>

• Clang 7+ (see [Phoronix][git commit])
• GCC 8.1+ ([GCC status page] [git commit])
  • GCC 8.x requires linking with -lstdc++fs.

Issues & PRs that may be linked with this PR

#13103, #13787, google/leveldb#760, #6093

TODO

• Remove Boost's filesystem from https://github.com/bitcoin/bitcoin/blob/master/configure.ac (help is welcome)
  • Relevant PR: C++11 build/runtime fixes #7302 & 76ac35f
  • Done in build: set minimum required Boost to 1.58.0 #19667.
• Am I missing -lstdc++fs?
• What about wallet: bugfix; if datadir has a trailing '/' listwalletdir would strip lead char of walletname #19933?
• Address this change build: The vcpkg tool has introduced a proper way to use manifests #19960
• unicodepath test in src/test/dbwrapper_tests.cpp seems to require some UTF-8 locale.
  • See also

    bitcoin/src/util/system.cpp

    Line 1249 in ae9ee5b

    // Set the default input/output charset is utf-8

  • https://github.com/microsoft/STL/blob/2aa944beba9af1d72537fd5e35d402c8b830a5dc/stl/src/filesystem.cpp#L266 - C++17 in Microsoft C++ standard library
  • https://docs.microsoft.com/en-us/cpp/standard-library/filesystem?view=msvc-160
  • https://docs.microsoft.com/en-us/cpp/standard-library/file-system-navigation?view=msvc-160

Inspiration

• https://github.com/ceptontech/filesystem
• https://github.com/ceztko/u8filesystem

[practicalswift]

Concept ACK: this will be nice to have in the future when we switch to C++17. Thanks for experimenting!

[maflcko]

Would it make sense to split out the two absolute() commits, because they make sense even with C++11?

[kiminuo]

Would it make sense to split out the two absolute() commits, because they make sense even with C++11?

I think so. Yes.

[laanwj]

Nice work.

[DrahtBot]

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

• Fix Windows build with --enable-werror #20586 (Fix Windows build with --enable-werror by hebasto)
• Add syscall sandboxing using seccomp-bpf (Linux secure computing mode) #20487 (draft: Add syscall sandboxing using seccomp-bpf (Linux secure computing mode) by practicalswift)
• rpc: Add RPCContext #20017 (rpc: Add RPCContext by promag)
• multiprocess: Add bitcoin-wallet -ipcconnect option #19460 (multiprocess: Add bitcoin-wallet -ipcconnect option by ryanofsky)
• net: Add NAT-PMP port forwarding support #18077 (net: Add NAT-PMP port forwarding support by hebasto)
• util: Set safe permissions for data directory and wallets/ subdir #17127 (util: Correct permissions for datadir and wallets subdir by hebasto)
• Multiprocess bitcoin #10102 ([experimental] Multiprocess bitcoin by ryanofsky)

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.

[maflcko]

Hi, catching up on the IRC log.

I just wanted to say that this is blocked on #20460 and there is nothing you can do about this in this pull. Fixing #20460 will need build system and gitian/guix, as well as CI changes. This would be too much to do here.

[fanquake]

Thanks for your work here so far @kiminuo. I've addressed the build system requirements, and combined this changes into #20744 (still attributed to you). However due to runtime and compiler requirements it's unlikely we'll be able to start using them just yet. I'm going to close this PR in favour of #20744 for now.