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

achow101 commented at 5:48 am on December 23, 2021: member

With Boost 1.78 (installed on my system), I get a compiler error:

0./fs.h: In function ‘fs::path fs::operator+(fs::path, fs::path)’:
1./fs.h:91:8: error: no match for ‘operator+=’ (operand types are ‘fs::path’ and ‘std::remove_reference<fs::path&>::type’ {aka ‘fs::path’})
2   91 |     p1 += std::move(p2);
3      |     ~~~^~~~~~~~~~~~~~~~

It appears that in Boost 1.78, operator+= became a template function, and our path class which subclasses boost::filesystem::path::path no longer has an operator+= which will work with it.

I also opened an issue with boost: https://github.com/boostorg/filesystem/issues/223

achow101 added the label Bug on Dec 23, 2021

MarcoFalke added this to the milestone 23.0 on Dec 23, 2021

MarcoFalke commented at 7:19 am on December 23, 2021: member

22.x should compile fine, right?

fanquake commented at 8:18 am on December 23, 2021: member

This will be “solved” in master (23.0) by just getting rid of boost:filesystem (#20744).

22.x should compile fine, right?

Looks like it does. I’ve just compiled 22.x @ 56156a1f08b08127a0d7fc0abd4826f371382a98 on Arch Linux, which packages Boost 1.78.0-1, and that worked fine. Note that for 22.x depends Boost would also available.

So what we need to decide is what to do for the period between now and when #20744 is merged, for anyone building master, using a system boost >= 1.78.0. Obviously they could also build against depends which is still version 1.71.0.

achow101 commented at 6:02 pm on December 23, 2021: member

Boost has committed a fix for this issue, but it won’t be available until the next boost release, which is probably going to be in April.

ryanofsky commented at 11:32 pm on December 23, 2021: member

I think the only thing calling code uses this operator+ function for is appending literal “.json” or “.dat” suffixes, so it should be fine to replace it with something less flexible but more portable like:

 0diff --git a/src/fs.h b/src/fs.h
 1index 3cf4371fb4e..f179cc04d5c 100644
 2--- a/src/fs.h
 3+++ b/src/fs.h
 4@@ -85,11 +85,11 @@ static inline auto quoted(const std::string& s)
 5     return boost::io::quoted(s, '&');
 6 }
 7 
 8-// Allow safe path append operations.
 9-static inline path operator+(path p1, path p2)
10+// Allow adding literal path suffixes, which is safe as long as the suffix is ASCII.
11+static inline path operator+(path p, char const* suffix)
12 {
13-    p1 += std::move(p2);
14-    return p1;
15+    p += suffix;
16+    return p;
17 }
18 
19 /**

MarcoFalke commented at 10:28 am on December 31, 2021: member

OpenSuse Tumbleweed is also affected

martinus commented at 2:35 pm on January 6, 2022: contributor

Just ran into the same problem on my Manjaro (Arch) Linux. With your diff @ryanofsky it compiles but I get test errors:

0wallet/test/init_tests.cpp(72): error: in "init_tests/walletinit_verify_walletdir_no_trailing": check walletdir == expected_path has failed ["/tmp/test_common_Bitcoin Core/4a5bfaa41ad260303f7dfa2fc8bdf3b9229792fbdb8aadc1853590cf54970ea2/wallets/" != "/tmp/test_common_Bitcoin Core/4a5bfaa41ad260303f7dfa2fc8bdf3b9229792fbdb8aadc1853590cf54970ea2/wallets"]

It seems that fs::canonical doesn’t remove trailing slashes any more. As far as I know this is also not necessary for std::filesytem::path (see https://eel.is/c++draft/filesystems#fs.path.generic-6) so it’s something that might change in the future.

In any case, this is the diff that works for me (all tests seem to work):

 0diff --git a/src/fs.h b/src/fs.h
 1index 3cf4371fb4..724bb0f79a 100644
 2--- a/src/fs.h
 3+++ b/src/fs.h
 4@@ -88,8 +88,7 @@ static inline auto quoted(const std::string& s)
 5 // Allow safe path append operations.
 6 static inline path operator+(path p1, path p2)
 7 {
 8-    p1 += std::move(p2);
 9-    return p1;
10+    return p1.concat(p2);
11 }
12 
13 /**
14diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp
15index 7ef5a0cf55..68e7d001b3 100644
16--- a/src/wallet/load.cpp
17+++ b/src/wallet/load.cpp
18@@ -28,7 +28,7 @@ bool VerifyWallets(WalletContext& context)
19         fs::path wallet_dir = fs::PathFromString(args.GetArg("-walletdir", ""));
20         boost::system::error_code error;
21         // The canonical path cleans the path, preventing >1 Berkeley environment instances for the same directory
22-        fs::path canonical_wallet_dir = fs::canonical(wallet_dir, error);
23+        fs::path canonical_wallet_dir = fs::canonical(wallet_dir, error).remove_trailing_separator();
24         if (error || !fs::exists(wallet_dir)) {
25             chain.initError(strprintf(_("Specified -walletdir \"%s\" does not exist"), fs::PathToString(wallet_dir)));
26             return false;
27 /**

remove_trailing_separator is undocumented though and doesn’t exist for std::filesystem.

fanquake commented at 12:24 pm on January 7, 2022: member

For anyone who’s having issues with Boost 1.78.0, I’m going to beg for review in #20744. Ideally we can fix this issue in master by just removing the Boost filesystem code entirely.

ryanofsky commented at 2:44 pm on January 10, 2022: member

re: #23846 (comment)

Just ran into the same problem on my Manjaro (Arch) Linux. With your diff @ryanofsky it compiles but I get test errors:

Maybe this is tangential if we will use #20744 instead of fixing compability with boost, but it doesn’t seem like those test failures have to do the with operator+ function. They seem like a different compatibility change with fs::canonical, so I don’t think there is a problem with my suggested fix for operator+.

I do think there is a problem with your suggested fix for operator+ because the single argument concat method doesn’t seem to exist before boost 1.78:

https://www.boost.org/doc/libs/1_77_0/libs/filesystem/doc/reference.html#path-concatenation https://www.boost.org/doc/libs/1_78_0/libs/filesystem/doc/reference.html#path-concatenation

Fabcien referenced this in commit c4dad634ab on Jan 13, 2022

dongcarl commented at 5:37 pm on January 19, 2022: member

Temporary workaround for Arch users:

0$ sudo pacman -U https://archive.archlinux.org/packages/b/boost/boost-1.76.0-6-x86_64.pkg.tar.zst https://archive.archlinux.org/packages/b/boost-libs/boost-libs-1.76.0-6-x86_64.pkg.tar.zst

achow101 commented at 8:46 pm on January 19, 2022: member

While #20744 is the preferred permanent fix, it seems that there are some lingering issues with it, particularly for Windows builds, that may take some time to resolve. So in the interest of being able to build master without depends, I’ve opened #24104 with a quick fix for Boost 1.78 compatibility issues.

fanquake closed this on Jan 20, 2022

sidhujag referenced this in commit cd1c711b18 on Jan 20, 2022

DrahtBot locked this on Jan 20, 2023

No match for fs::path operator+= when compiling with Boost 1.78 #23846