build: set minimum required Boost to 1.64.0 #22320
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Now that we require Boost 1.64.0+, Boost Process will be available.
b8bcaa5 to
957f358
Compare
|
Concept ACK |
|
Concept ACK. |
|
Boost Process was introduced in 1.64. Debian stretch has boost 1.62, other main distros have 1.65+, right? If so, will we have any benefit of bumping boost version to 1.65 (instead of 1.64)? |
The only benefit might be being able to take advantage of any new Boost Test related features. However it doesn't look like there is anything substantial in that regard. |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
This was referenced Jun 23, 2021
|
review ACK 957f358 didn't test |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Jun 24, 2021
957f358 build: remove check for Boost Process header (fanquake) df2c933 build: remove workaround for Boost and std::atomic (fanquake) 2bf2116 build: set minimum required Boost to 1.64.0 (fanquake) Pull request description: Setting a newer minimum required Boost means we can remove the awkward header / compile check for Boost Process. If we don't do this, we should at-least make Boost Process being missing no longer a failure, otherwise anyone building using Boost < 1.64.0 would have to pass `--disable-external-signer` as well. The only system I can see that is affected here, (doesn't have new enough system packages) is Debian Oldstable. However, anyone compiling there, can use depends. They can also no-longer use the system GCC (6.0), and I'd assume would be using Clang 7, which would be the newest compiler available to them. It's extended, LTS support also end in 1 year from now, so anyone still using it should be considering upgrading. Debian Buster (Stable) has 1.67+, Ubuntu Bionic has 1.65+, any of the BSDs, recent Fedora, macOS etc all also have well and truly new enough Boost versions available. I think this is something we should just do for 22.0. If not, definitely for 23.0. Fixes bitcoin#22319. Compiling Bitcoin Core should work, as `windows.h` will be included. Alternative to bitcoin#22294. Would also close bitcoin#22269. bitcoin#19128 could be re-opened. ACKs for top commit: laanwj: Tested ACK 957f358 that this fixes bitcoin#22269. MarcoFalke: review ACK 957f358 Tree-SHA512: a8ffa7933dce8bf994892ef16664103d7b4e1008e52628e9becb918a7727232dfb51b23100a82dc2b60cd9af5877abc32dc2d3754a7e1b3ac5410a92fdf393f3
|
Please check at least RHEL next time. For reference, looks like it's got 1.66 so should be fine here. |
This was somewhat covered by the fact that RHEL is based off a semi-recent Fedora. It would be fine in any case, because if it had a too-old version of Boost, anyone wanting to compile on that system can use depends. |
fanquake
added a commit
that referenced
this pull request
Oct 30, 2021
9ba7c44 refactor: get wallet path relative to wallet_dir (Michael Dietz) Pull request description: Now that boost has been updated > 1.60 (see #22320), we can simplify how we get wallet path relative to wallet_dir by using: `boost::filesystem::lexically_relative`, removing a TODO. Test coverage comes from `test/functional/wallet_multiwallet.py` I first tried this in #20265 which was my first attempted PR, and funny enough exactly 1 year later I'm opening this one to hopefully finally close this. ACKs for top commit: ryanofsky: Code review ACK 9ba7c44. Basically this same code change is made in #20744 commit b70c843, so this PR helps simplify that one lsilva01: Code Review ACK 9ba7c44 Tree-SHA512: 6ccb91a18bcb52c3ae0c789a94a18fb5be7db7769fd1121552d63f259fbd32b50c3dcf169cec0b02f978321db3bc60eb4b881b8327e9764f32e700236e0d8a35
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Oct 30, 2021
9ba7c44 refactor: get wallet path relative to wallet_dir (Michael Dietz) Pull request description: Now that boost has been updated > 1.60 (see bitcoin#22320), we can simplify how we get wallet path relative to wallet_dir by using: `boost::filesystem::lexically_relative`, removing a TODO. Test coverage comes from `test/functional/wallet_multiwallet.py` I first tried this in bitcoin#20265 which was my first attempted PR, and funny enough exactly 1 year later I'm opening this one to hopefully finally close this. ACKs for top commit: ryanofsky: Code review ACK 9ba7c44. Basically this same code change is made in bitcoin#20744 commit b70c843, so this PR helps simplify that one lsilva01: Code Review ACK 9ba7c44 Tree-SHA512: 6ccb91a18bcb52c3ae0c789a94a18fb5be7db7769fd1121552d63f259fbd32b50c3dcf169cec0b02f978321db3bc60eb4b881b8327e9764f32e700236e0d8a35
gwillen
pushed a commit
to ElementsProject/elements
that referenced
this pull request
Jun 1, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Setting a newer minimum required Boost means we can remove the awkward header / compile check for Boost Process.
If we don't do this, we should at-least make Boost Process being missing no longer a failure, otherwise anyone building using Boost < 1.64.0 would have to pass
--disable-external-signeras well.The only system I can see that is affected here, (doesn't have new enough system packages) is Debian Oldstable. However, anyone compiling there, can use depends. They can also no-longer use the system GCC (6.0), and I'd assume would be using Clang 7, which would be the newest compiler available to them. It's extended, LTS support also end in 1 year from now, so anyone still using it should be considering upgrading.
Debian Buster (Stable) has 1.67+, Ubuntu Bionic has 1.65+, any of the BSDs, recent Fedora, macOS etc all also have well and truly new enough Boost versions available.
I think this is something we should just do for 22.0. If not, definitely for 23.0.
Fixes #22319. Compiling Bitcoin Core should work, as
windows.hwill be included.Alternative to #22294.
Would also close #22269.
#19128 could be re-opened.