Replace boost::filesystem with std::filesystem #19245

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, https://github.com/google/leveldb/pull/760, #6093

TODO

• Remove Boost's filesystem from https://github.com/bitcoin/bitcoin/blob/master/configure.ac (help is welcome)
  • Relevant PR: #7302 & https://github.com/bitcoin/bitcoin/commit/76ac35f36d87078da62f95b4a1167ec296e37363
  • Done in #19667.
• Am I missing -lstdc++fs?
• What about #19933?
• Address this change #19960
• unicodepath test in src/test/dbwrapper_tests.cpp seems to require some UTF-8 locale.
  • See also https://github.com/bitcoin/bitcoin/blob/ae9ee5bdb18c5babbfa2875ab694df118d6a1c1b/src/util/system.cpp#L1249
  • 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

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

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

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

I think so. Yes.

Nice work.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #20586 (Fix Windows build with --enable-werror by hebasto)
• #20487 (draft: Add syscall sandboxing using seccomp-bpf (Linux secure computing mode) by practicalswift)
• #20017 (rpc: Add RPCContext by promag)
• #19460 (multiprocess: Add bitcoin-wallet -ipcconnect option by ryanofsky)
• #18077 (net: Add NAT-PMP port forwarding support by hebasto)
• #17127 (util: Correct permissions for datadir and wallets subdir by hebasto)
• #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.

Nice! Please ping me when it's ready for review :)

@kiminuo Now that #20413 is merged, are you planning on rebasing this and getting it ready for review?

@kiminuo Now that #20413 is merged, are you planning on rebasing this and getting it ready for review?

I will get to it (hopefully soon ™). However, @MarcoFalke mentioned on IRC briefly that std::fs is not available on some supported OSs even with C++17 enabled. I'm not sure I understood correctly. If true, it would delay this feature months or years.

If you want to work on this. No problem. I'm quite busy.

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.

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.