fs: Make compatible with boost 1.78 #24104

1. 
   achow101 commented at 8:45 pm on January 19, 2022: member

   Boost 1.78 removed operator+ in a way that breaks our usage of it in a subclass. A proposed workaround for this is to cast the argument to boost::filesystem::path, and this is backwards compatible with older versions of boost.

   Additionally, it appears that fs::canonical no longer removes trailing slashes. This was causing a test to fail. The solution is to explicitly remove the trailing separator in the one place that fs::canonical is used.

   Lastly, fs::create_directories now has an error message saying create_directories instead of create_directory. This caused wallet_multiwallet.py to fail. The error message check has been updated to be able accept either string.

   Fixes #23846

2. in src/fs.h:91 in 575ecd0ff5 outdated

   87@@ -88,7 +88,7 @@ static inline auto quoted(const std::string& s)
   88 // Allow safe path append operations.
   89 static inline path operator+(path p1, path p2)
   90 {
   91-    p1 += std::move(p2);
   92+    p1 += std::move(static_cast<boost::filesystem::path>(p2));
   

   ---

   ---

   

   ryanofsky commented at 9:15 pm on January 19, 2022:

   In commit “fs: Make compatible with boost 1.78” (575ecd0ff5025bdd2f5cb5e52049aaa202245949)

   The static_cast creates an unnecessary temporary here and makes the std::move not have any effect because its argument is already temporary. The right way to write this would be p1 += static_cast<boost::filesystem::path&&>(p2);

   You could also consider using the alternate fix from #23846 (comment). I think that fix is a little better since it gets rid of the nonstandard + operator in cases where we don’t need it and also avoids more temporaries.

   ---

   

   achow101 commented at 9:27 pm on January 19, 2022:

   Changed to the correct cast.
3. ryanofsky approved
4. 
   ryanofsky commented at 9:20 pm on January 19, 2022: member

   Code review ACK 575ecd0ff5025bdd2f5cb5e52049aaa202245949. This looks safe but the code is a little nonsensical, because the static_cast creates an unnecessary temporary, and std::move has no effect moving from a temporary that would already be moved from. I suggested two alternatives if you want to update.
5. fs: Make compatible with boost 1.78 dc5d6b0d47
6. achow101 force-pushed on Jan 19, 2022
7. ryanofsky approved
8. 
   ryanofsky commented at 9:29 pm on January 19, 2022: member

   Code review ACK dc5d6b0d4793ca978f71f69ef7d6b818794676c2 🪄
9. vincenzopalazzo approved
10. 
    vincenzopalazzo commented at 9:59 pm on January 19, 2022: none

    ACK https://github.com/bitcoin/bitcoin/pull/24104/commits/dc5d6b0d4793ca978f71f69ef7d6b818794676c2
11. DrahtBot added the label Wallet on Jan 19, 2022
12. achow101 added the label Utils/log/libs on Jan 20, 2022
13. 
    fanquake commented at 5:05 am on January 20, 2022: member

    Given that #23846 is becoming more of an issue for devs, and #20744 is still stuck while we make some final changes and fix Guix, I’m going to go-ahead and merge this.
14. fanquake merged this on Jan 20, 2022
15. fanquake closed this on Jan 20, 2022

    ---

16. Fabcien referenced this in commit b9928812c9 on Jan 20, 2022
17. sidhujag referenced this in commit cd1c711b18 on Jan 20, 2022
18. fanquake referenced this in commit 021c3d892f on Mar 5, 2022
19. fanquake referenced this in commit cb13ba6d11 on Mar 7, 2022
20. apoelstra referenced this in commit c08430ab7c on Aug 14, 2022
21. psgreco referenced this in commit 5629cae32b on Sep 1, 2022
22. apoelstra referenced this in commit 433a57a5bd on Sep 6, 2022
23. apoelstra referenced this in commit f4254f34fb on Sep 20, 2022
24. Fuzzbawls referenced this in commit 291ef05f53 on Sep 21, 2022
25. achow101 referenced this in commit a53d6c7a47 on Oct 18, 2022
26. DrahtBot locked this on Jan 20, 2023

Contributors
achow101 ryanofsky vincenzopalazzo fanquake

Labels
Wallet Utils/log/libs