-
Notifications
You must be signed in to change notification settings - Fork 37.7k
fs: Make compatible with boost 1.78 #24104
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review ACK 575ecd0. 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.
575ecd0
to
dc5d6b0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review ACK dc5d6b0 🪄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK dc5d6b0
Summary: ``` 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. ``` Backport of [[bitcoin/bitcoin#24104 | core#24104]]. This replaces the build failure work around introduced in D10822. Test Plan: With boost 1.78: ninja all check-all Reviewers: #bitcoin_abc, PiRK Reviewed By: #bitcoin_abc, PiRK Differential Revision: https://reviews.bitcoinabc.org/D10850
dc5d6b0 fs: Make compatible with boost 1.78 (Andrew Chow) Pull request description: Boost 1.78 removed `operator+` in a way that breaks our usage of it in a subclass. A [proposed workaround](boostorg/filesystem#223 (comment)) 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 bitcoin#23846 ACKs for top commit: ryanofsky: Code review ACK dc5d6b0 vincenzopalazzo: ACK bitcoin@dc5d6b0 Tree-SHA512: d4d8e7b49b8dfbf0ced9bfe9a2b3827841227fc755fc799f19159076b0ccf882432cc8b6ad93cdeda98fb58b942b9ba50a9e0a6b4f6b1e0097e80f1074ae5682
Github-Pull: bitcoin#24104 Rebased-From: dc5d6b0
021c3d8 fs: Make compatible with boost 1.78 (Andrew Chow) Pull request description: Backports #24104 to the `22.x` branch to fix the build with Boost 1.78.0. ACKs for top commit: achow101: ACK 021c3d8 gruve-p: ACK 021c3d8 hebasto: ACK 021c3d8 Tree-SHA512: 439c463c36b15a8507d58c3d9c6a02f6dfb209bcc85a8ed39a9cc3fe023530f9f74cc3fd545710b0bb15b4ff6afae802471c6819db7cd851dddd537938e0eef5
…or rc5 37076d7 Bump version to -rc5 (Pablo Greco) 5629cae fs: Make compatible with boost 1.78 (Andrew Chow) 60b913e Elements-qt: Correctly display the amount in the sender's wallet after using Send button (Andrea Bonel) 872478b docs: describe elements transaction serialization format (#1148) (James Dorfman) dd2d758 fixed minor issues found in review (Allen Piscitello) 2da7d75 removing test that fails due to blinded issuances, which results in incorrect reissuance token ids (Allen Piscitello) 420de43 removing code to blind issuances. PSET should be modified to include an option to blind or unblind issuances, defaulting to unblind. (Allen Piscitello) Pull request description: Backport #1150 #1154 and #1148 from master. Backport bitcoin/bitcoin#24104 from Bitcoin Core Bump to -rc5 ACKs for top commit: jamesdorfman: utACK 37076d7. delta1: utACK 37076d7 Tree-SHA512: 616322f94c17008cc7fd582203c22b33c73b922269ab33fd13b270f491eda9b30d4f18efa3f7887e247bef46b63c4810f4084e409bea98120702c426a83343a8
c5960bc [GA] Specify explicit Python 3.8 for macOS native builds (Fuzzbawls) 945724b [Test] Fix multiwallet test with Boost 1.78+ (Fuzzbawls) 5e0166e [GA] Bump native macOS builds to macos-11 (Fuzzbawls) d207d00 [GA] Run unit and functional tests on native macOS build (Fuzzbawls) Pull request description: Bump our native macOS GA runner to macOS 11 (as 10.15 is now deprecated and will cease to function in December), as well as enable unit and functional tests for our native macOS runner, and fix an issue with Boost 1.78+ compatibility for the multiwallet functional test. Last commit inspired by bitcoin#24104 > 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. ACKs for top commit: yenachar: utACK c5960bc Tree-SHA512: 149adee2c3cb56106f72dd26eb93b3f0b2fd6ccf89315734554c2b1e60c21fcbb6126fbdcc492f49e11aaa5bbbecd3e62d8cb6c351a52a2c35cb3b670696a4c4
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 toboost::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 thatfs::canonical
is used.Lastly,
fs::create_directories
now has an error message sayingcreate_directories
instead ofcreate_directory
. This caused wallet_multiwallet.py to fail. The error message check has been updated to be able accept either string.Fixes #23846