Skip to content

Set minimum required Boost to 1.47.0#8920

Merged
laanwj merged 2 commits intobitcoin:masterfrom
fanquake:set-minimum-boost
Oct 17, 2016
Merged

Set minimum required Boost to 1.47.0#8920
laanwj merged 2 commits intobitcoin:masterfrom
fanquake:set-minimum-boost

Conversation

@fanquake
Copy link
Member

@fanquake fanquake commented Oct 14, 2016 •
edited
Loading

Boost is on the way out, but in the mean time, having the minimum set to 1.20.0 is misleading, and can lead to issues like #8868.

1.47.0 is the first version to include Chrono. See #8875 for some history.

paveljanik
paveljanik reviewed Oct 14, 2016
View reviewed changes
build-aux/m4/ax_boost_base.m4 Outdated
Copy link
Contributor

@paveljanik paveljanik Oct 14, 2016

Choose a reason for hiding this comment

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

Eh?

paveljanik
paveljanik reviewed Oct 14, 2016
View reviewed changes
build-aux/m4/ax_boost_base.m4 Outdated
Copy link
Contributor

@paveljanik paveljanik Oct 14, 2016

Choose a reason for hiding this comment

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

The serial should reflect the original version used as a base - coming from https://www.gnu.org/software/autoconf-archive/ax_boost_base.html

@paveljanik
Copy link
Contributor

paveljanik commented Oct 14, 2016 •
edited
Loading

So you are updating the file to match and at the same time upgrading the minimal requirement. Please always mention this in the PR text and not hide the first part.

Changes like this should be done in two commits, I think.

@luke-jr
Copy link
Member

luke-jr commented Oct 14, 2016

Concept ACK, but shouldn't some documentation need updating as well?

laanwj
laanwj reviewed Oct 15, 2016
View reviewed changes
build-aux/m4/ax_boost_base.m4 Outdated
Copy link
Member

@laanwj laanwj Oct 15, 2016

Choose a reason for hiding this comment

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

concept ACK - though personally I'd prefer passing in this version as a parameter from our configure.ac, if possible, instead of changing this upstream file

@laanwj
Copy link
Member

laanwj commented Oct 15, 2016

Concept ACK, but shouldn't some documentation need updating as well?

If only we had documentation for minimum version requirements, it should be updated. But doesn't look like we do, build-unix only lists the libraries necessary not their minimum supported versions. We've never really kept track of those.

@laanwj laanwj mentioned this pull request Oct 15, 2016
Closed
@fanquake fanquake mentioned this pull request Oct 17, 2016
Closed
paveljanik
paveljanik approved these changes Oct 17, 2016
View reviewed changes
Copy link
Contributor

@paveljanik paveljanik left a comment

Choose a reason for hiding this comment

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

This looks much better, thank you!

ACK 6dd3723

@paveljanik
Copy link
Contributor

paveljanik commented Oct 17, 2016

Build log diff:

-checking for boostlib >= 1.20.0... yes
+checking for boostlib >= 1.47.0... yes

Great!

@jonasschnelli
Copy link
Contributor

jonasschnelli commented Oct 17, 2016

utACK 6dd3723

@maflcko
Copy link
Member

maflcko commented Oct 17, 2016

utACK 6dd3723

@laanwj laanwj merged commit 6dd3723 into bitcoin:master Oct 17, 2016
laanwj added a commit that referenced this pull request Oct 17, 2016
@laanwj
Merge #8920: Set minimum required Boost to 1.47.0
c901113 
6dd3723 Set minimum required Boost to 1.47.0 (fanquake)
282abd8 [build-aux] Boost_Base serial 27 (fanquake)
@fanquake fanquake deleted the set-minimum-boost branch October 17, 2016 11:45
@theuni
Copy link
Member

theuni commented Oct 17, 2016

post-merge ACK.

@Zenitur
Copy link

Zenitur commented Nov 4, 2016

An error when build with Boost 1.49: http://pastebin.com/b2JHAE01

@paveljanik
Copy link
Contributor

paveljanik commented Nov 5, 2016

@Zenitur Upstream bug https://svn.boost.org/trac/boost/ticket/6790

@Zenitur
Copy link

Zenitur commented Nov 6, 2016

@paveljanik now it works. Thank you!

luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Dec 2, 2016
@fanquake @luke-jr
[build-aux] Boost_Base serial 27
c134d92 
Github-Pull: bitcoin#8920
Rebased-From: 282abd8
luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Dec 2, 2016
@fanquake @luke-jr
Set minimum required Boost to 1.47.0
40169dc 
Github-Pull: bitcoin#8920
Rebased-From: 6dd3723
@laanwj laanwj mentioned this pull request Feb 8, 2017
Closed
@laanwj laanwj mentioned this pull request Feb 9, 2017
Merged
laanwj added a commit to laanwj/bitcoin that referenced this pull request Feb 10, 2017
@laanwj
Remove fallbacks for boost_filesystem < v3
056aba2 
Minimum boost version was bumped to 1.47.0 in bitcoin#8920, which
means the configure step won't even pass with older boost.

This version has boost filesystem v3, which means the
(crappy) fallbacks for older versions can go.
schinzelh pushed a commit to dashpay/dash that referenced this pull request Oct 23, 2017
@fanquake @schinzelh
Boost_Base serial 27
74394f1 
Github-Pull: bitcoin#8920
Rebased-From: 282abd8
schinzelh pushed a commit to dashpay/dash that referenced this pull request Oct 23, 2017
@fanquake @schinzelh
Set minimum required Boost to 1.47.0
938a6fb 
Github-Pull: bitcoin#8920
Rebased-From: 6dd3723
codablock pushed a commit to codablock/dash that referenced this pull request Jan 12, 2018
@laanwj @codablock
Merge bitcoin#8920: Set minimum required Boost to 1.47.0
19316b7 
6dd3723 Set minimum required Boost to 1.47.0 (fanquake)
282abd8 [build-aux] Boost_Base serial 27 (fanquake)
HashUnlimited pushed a commit to chaincoin/chaincoin that referenced this pull request Feb 27, 2018
@laanwj
Remove fallbacks for boost_filesystem < v3
88b536c 
Minimum boost version was bumped to 1.47.0 in bitcoin#8920, which
means the configure step won't even pass with older boost.

This version has boost filesystem v3, which means the
(crappy) fallbacks for older versions can go.

Conflicts:
	src/test/testutil.cpp
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request Oct 16, 2018
@fanquake
Set minimum required Boost to 1.47.0
cae922d 
Github-Pull: bitcoin#8920
Rebased-From: 6dd3723
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request Oct 16, 2018
@fanquake
[build-aux] Boost_Base serial 27
31be10f 
Github-Pull: bitcoin#8920
Rebased-From: 282abd8
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request Jun 23, 2019
@laanwj @lateminer
Remove fallbacks for boost_filesystem < v3
da76c0a 
Minimum boost version was bumped to 1.47.0 in bitcoin#8920, which
means the configure step won't even pass with older boost.

This version has boost filesystem v3, which means the
(crappy) fallbacks for older versions can go.
@hebasto hebasto mentioned this pull request Jul 13, 2019
Closed
@jnewbery jnewbery mentioned this pull request Dec 11, 2020
Merged
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@laanwj laanwj laanwj left review comments

+1 more reviewer

@paveljanik paveljanik paveljanik approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@theuni theuni

Labels

Utils/log/libs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@fanquake @paveljanik @luke-jr @laanwj @jonasschnelli @maflcko @theuni @Zenitur