Skip to content

Conversation

achow101
Copy link
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

Copy link
Contributor

@ryanofsky ryanofsky left a 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.

Copy link
Contributor

@ryanofsky ryanofsky left a 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 🪄

Copy link

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK dc5d6b0

@fanquake
Copy link
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.

@fanquake fanquake merged commit a541e5d into bitcoin:master Jan 20, 2022
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jan 20, 2022
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
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 20, 2022
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
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Mar 5, 2022
fanquake added a commit that referenced this pull request Mar 7, 2022
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
apoelstra pushed a commit to apoelstra/elements that referenced this pull request Aug 14, 2022
psgreco pushed a commit to psgreco/elements that referenced this pull request Sep 1, 2022
apoelstra added a commit to ElementsProject/elements that referenced this pull request Sep 6, 2022
…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
apoelstra pushed a commit to apoelstra/elements that referenced this pull request Sep 20, 2022
Fuzzbawls added a commit to PIVX-Project/PIVX that referenced this pull request Sep 21, 2022
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
achow101 added a commit to achow101/elements that referenced this pull request Oct 18, 2022
@bitcoin bitcoin locked and limited conversation to collaborators Jan 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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