Skip to content
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.

Sign up for GitHub

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

Jump to bottom

bump boost test to 1.59? #19128

Closed
maflcko opened this issue May 31, 2020 · 3 comments
Closed

bump boost test to 1.59? #19128

maflcko opened this issue May 31, 2020 · 3 comments
Labels
Brainstorming Build system Feature

Comments

@maflcko
Copy link
Member

maflcko commented May 31, 2020 •
edited

To skip tests. See #16564 (comment)

See https://www.boost.org/doc/libs/1_59_0/libs/test/doc/html/boost_test/tests_organization/enabling.html

https://packages.debian.org/jessie/libboost-all-dev is going to die in about a month https://wiki.debian.org/DebianReleases#Production_Releases

https://packages.ubuntu.com/xenial/libboost-all-dev is going to die in less than a year https://ubuntu.com/about/release-cycle
@maflcko maflcko mentioned this issue May 31, 2020
Merged
1 task
@fanquake
Copy link
Member

We've just bumped Boost to 1.58.0, so this has missed the cut. If/when we bump it the next time, it'll likely be > 1.59.0, so we'll get the test features, however I'm not sure we need to keep a tracking issue open.

@fanquake fanquake mentioned this issue Jun 23, 2021
Merged
fanquake added a commit that referenced this issue Jun 24, 2021
@fanquake
Merge #22320: build: set minimum required Boost to 1.64.0
c31161f 
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 #22319. Compiling Bitcoin Core should work, as `windows.h` will be included.
  Alternative to #22294.
  Would also close #22269.
  #19128 could be re-opened.

ACKs for top commit:
  laanwj:
    Tested ACK 957f358 that this fixes #22269.
  MarcoFalke:
    review ACK 957f358

Tree-SHA512: a8ffa7933dce8bf994892ef16664103d7b4e1008e52628e9becb918a7727232dfb51b23100a82dc2b60cd9af5877abc32dc2d3754a7e1b3ac5410a92fdf393f3
@fanquake fanquake reopened this Jun 24, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this issue Jun 24, 2021
@fanquake
Merge bitcoin#22320: build: set minimum required Boost to 1.64.0
82f2101 
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
@hebasto
Copy link
Member

hebasto commented Aug 19, 2021

Ok, after #22320 this issue was re-opened.

But its description seems outdated now. Maybe update it with more clear goals?

@maflcko maflcko mentioned this issue Aug 20, 2021
Closed
@maflcko maflcko closed this as completed Aug 20, 2021
@hebasto
Copy link
Member

hebasto commented Dec 25, 2021 •
edited

To make make check compatible with https://www.boost.org/doc/libs/1_64_0/libs/test/doc/html/boost_test/tests_organization/enabling.html, changes from #22757 are required.

@bitcoin bitcoin locked and limited conversation to collaborators Dec 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Brainstorming Build system Feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test: Use boost::unit_test::enable_if<RUN_RAII_EVENT_TESTS> maflcko/bitcoin-core
3 participants
@fanquake @maflcko @hebasto