depends: Split boost into build/host packages + bump + cleanup #19764

dongcarl · 2020-08-18T21:15:42Z

This PR improves the robustness of our boost package in depends, most notably:

Bumps boost from 1.70.0 to 1.71.0, because 1.71.0:
1. Removes the need to patch out the unused variable.
  
  bitcoin/depends/packages/boost.mk
  
  Line 36 in f8462a6
  
  sed -i.old "s/int ret_sig = 0;//" boost/process/detail/posix/wait_group.hpp && \
  
  Upstream boost patched it out in boostorg/process@d20b64c, which was first included in the 1.71.0 release
2. Comes packaged with a version of b2 which allows us to override its CXX and CXXFLAGS. Previously, choosing a toolset while building b2 such as clang or gcc would force b2's build system to invoke the compiler as a bare, hardcoded clang or gcc. However, our depends build system often want to customize this behaviour, adding extra flags or invoking the compiler by an alternate name. So this is useful.
  1. Commit where CXX was introduced: boostorg/build@374f965
  2. Commit where CXXFLAGS was introduced: boostorg/build@5d49abc
The boost package is now split into native_b2 and boost, better representing what actually happens.
- In our depends build system, we have a distinction between native packages and non-native packages. The output of native packages are meant to be used on the machine that's performing the build, and the output of non-native packages are meant to be used on/for the machine that will ultimately be running bitcoin. Previously, boost existed in depends as a non-native package, but that's partly inaccurate because the ./bootstrap.sh invocation in its $(package)_config_cmds stage actually produced a binary called b2, which is run on the machine that's performing the build. This means that b2 is a native package which is being built in an environment set up for the non-native package boost. This reveals a hidden unintended behavior in our depends build system: for linux->darwin cross builds, we use gcc for native packages, and clang for non-native packages. But b2 was actually being built using clang, since it was being built in an environment set up for non-native packages.

theuni you might be interested in taking a look

Empact · 2020-08-19T15:52:44Z

Could improve the documentation of these changes by including why each change was made, in addition to what was done. https://chris.beams.io/posts/git-commit/#why-not-how

Speaking as someone who is only so familiar with the build system who would like to learn more.

fanquake

Concept ACK. Looks like a good cleanup. Bit of a bummer (for slower systems) that we'll have to extract Boost twice now. Do you think using bcp is on the roadmap after this?

depends/packages/boost.mk

fanquake · 2020-08-20T06:29:49Z

depends/packages/boost.mk

@@ -9,7 +9,7 @@ define $(package)_set_vars
 $(package)_config_opts_release=variant=release
 $(package)_config_opts_debug=variant=debug
 $(package)_config_opts=--layout=tagged --build-type=complete --user-config=user-config.jam
-$(package)_config_opts+=threading=multi link=static -sNO_BZIP2=1 -sNO_ZLIB=1
+$(package)_config_opts+=threading=multi link=static -sNO_COMPRESSION=1


Even though this option shouldn't affect us, given we don't build iostreams, this is a nice simplification. Easier to pass sNO_COMPRESSION and not worry about new types being added in future, as we were already missing NO_LZMA and NO_ZSTD.

depends/packages/native_b2.mk

DrahtBot · 2020-08-20T19:51:56Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

Replace boost::filesystem with std::filesystem #19245 ([WIP DONOTMERGE] Replace boost::filesystem with std::filesystem (in c++17) by kiminuo)

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.

dongcarl · 2020-08-25T16:43:34Z

@Empact I updated the description with more justification and context, please let me know if anything's unclear!

theuni · 2020-08-27T20:06:56Z

Concept ACK.

Bit of a bummer (for slower systems) that we'll have to extract Boost twice now. Do you think using bcp is on the roadmap after this?

Very true, I had completely forgotten about this overhead when we discussed this. +1 for looking into reducing the size of the contents of the tarballs.

Also, it's worth investigating switching from the boost .tar.bz2 to the .tar.gz. Maybe that can make up for some of the slowdown at the cost of a size increase?

depends/funcs.mk

hebasto · 2020-09-16T08:17:27Z

Concept ACK. It would be nice to document native_* packages usage in packages.md (maybe in a follow up pull).

7a89f2e build: Fix target name (Hennadii Stepanov) Pull request description: It seems like a typo :) This PR: - fixes errors when building a package in depends for `HOST=x86_64-apple-darwin16` (fix #19799) - is a correct alternative to d25e0e3 from #19764 ACKs for top commit: icota: tACK 7a89f2e dongcarl: Code Review ACK 7a89f2e theuni: ACK 7a89f2e. Tree-SHA512: a0bcbc6805d3450e201476ef1e22e0eb53903db1586c5515314c19afd337bded887e56de0fbe62feaf359b2de15dbccd49a44f1a8b566b4c64f5ae3d94a2ab6d

7a89f2e build: Fix target name (Hennadii Stepanov) Pull request description: It seems like a typo :) This PR: - fixes errors when building a package in depends for `HOST=x86_64-apple-darwin16` (fix bitcoin#19799) - is a correct alternative to bitcoin@d25e0e3 from bitcoin#19764 ACKs for top commit: icota: tACK bitcoin@7a89f2e dongcarl: Code Review ACK 7a89f2e theuni: ACK 7a89f2e. Tree-SHA512: a0bcbc6805d3450e201476ef1e22e0eb53903db1586c5515314c19afd337bded887e56de0fbe62feaf359b2de15dbccd49a44f1a8b566b4c64f5ae3d94a2ab6d

dongcarl · 2020-09-23T19:11:51Z

Rebased over #19868 and dropped my solution.

practicalswift · 2020-09-23T20:18:35Z

Concept ACK

ryanofsky

Basic code review ACK 68d53e8. I was looking at this and didn't dig into all the details of all the changes, but did think they all look safe and sane and were well motivated.

I guess most ideally the boost build system would accept different build and host toolchains in its configuration, and use the right tools for the right things, so everything could be built in one package. But I guess its bootstrap script isn't really written to work this way, so the separate native b2 package makes sense and is a reasonable way to get the right compiler used.

fanquake

This looks good, great PR description 👍 . However in 6cba51e you've forgotten to remove the unused_var_in_process.patch file.

We already have $(package)_ar, so just use that instead

dongcarl · 2020-11-23T20:52:56Z

Pushed a pure-rebase. Still ready for merge!

laanwj · 2020-11-24T09:31:43Z

Concept and code review ACK f190343

This was forgotten in bitcoin#19764.

0918eb4 doc: Document current boost dependency as 1.71.0 (Wladimir J. van der Laan) Pull request description: This was forgotten in bitcoin#19764. ACKs for top commit: practicalswift: ACK 0918eb4 fanquake: ACK 0918eb4 Tree-SHA512: bd4a39b96b95adeb725767b283f4cf04d9f0d6ac352e7dc67f88cf575b00a24c6d3f4bf51fe362e0c89aeebb6c7e8e9add9f9f17e843121efd30f8edef6128bc

…bump + cleanup f190343 depends: boost: Specify cflags+compileflags (Carl Dong) b2328b7 depends: boost: Remove unnecessary _archiver_ (Carl Dong) ab9e047 depends: boost: Cleanup toolset selection (Carl Dong) 86002e7 depends: boost: Cleanup architecture/address-model (Carl Dong) d7048fa depends: boost: Disable all compression (Carl Dong) 9cf2ee5 depends: boost: Split into non-/native packages (Carl Dong) a57b498 depends: boost: Bump to 1.71.0 (Carl Dong) 800655f depends: boost: Refer to version in URL (Carl Dong) Pull request description: This PR improves the robustness of our boost package in depends, most notably: 1. Bumps boost from `1.70.0` to `1.71.0`, because `1.71.0`: 1. Removes the need to patch out the unused variable. https://github.com/bitcoin/bitcoin/blob/f8462a6d2794be728cf8550f45d19a354aae59cf/depends/packages/boost.mk#L36 Upstream boost patched it out in boostorg/process@d20b64c, which was first included in the `1.71.0` release 2. Comes packaged with a version of `b2` which allows us to override its `CXX` and `CXXFLAGS`. Previously, choosing a toolset while building `b2` such as `clang` or `gcc` would force `b2`'s build system to invoke the compiler as a bare, hardcoded `clang` or `gcc`. However, our `depends` build system often want to customize this behaviour, adding extra flags or invoking the compiler by an alternate name. So this is useful. 1. Commit where `CXX` was introduced: boostorg/build@374f965 2. Commit where `CXXFLAGS` was introduced: boostorg/build@5d49abc 2. The boost package is now split into `native_b2` and `boost`, better representing what actually happens. - In our `depends` build system, we have a distinction between `native` packages and non-`native` packages. The output of `native` packages are meant to be used on the machine that's performing the build, and the output of non-`native` packages are meant to be used on/for the machine that will ultimately be running bitcoin. Previously, `boost` existed in `depends` as a non-`native` package, but that's partly inaccurate because the `./bootstrap.sh` invocation in its `$(package)_config_cmds` stage actually produced a binary called `b2`, which is run on the machine that's performing the build. This means that `b2` is a `native` package which is being built in an environment set up for the non-`native` package `boost`. This reveals a hidden unintended behavior in our `depends` build system: for linux->darwin cross builds, we use `gcc` for `native` packages, and `clang` for non-`native` packages. But `b2` was actually being built using `clang`, since it was being built in an environment set up for non-`native` packages. theuni you might be interested in taking a look ACKs for top commit: laanwj: Concept and code review ACK f190343 Tree-SHA512: f8b728a34da4f0a9a985a819a5762f2fc2689ea24c7eba1d24d26dfbd4c59f202227c699b0a4069dab10b6329cf9f4c6dd95082685776ee43dd5f7b659acdef1

0918eb4 doc: Document current boost dependency as 1.71.0 (Wladimir J. van der Laan) Pull request description: This was forgotten in bitcoin#19764. ACKs for top commit: practicalswift: ACK 0918eb4 fanquake: ACK 0918eb4 Tree-SHA512: bd4a39b96b95adeb725767b283f4cf04d9f0d6ac352e7dc67f88cf575b00a24c6d3f4bf51fe362e0c89aeebb6c7e8e9add9f9f17e843121efd30f8edef6128bc

@dongcarl

Boost build backports Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#17087 - bitcoin/bitcoin#17231 - bitcoin/bitcoin#17928 - bitcoin/bitcoin#18820 - bitcoin/bitcoin#19764 Kudos to @dongcarl for all the excellent upstream depends system hackery!

de7766c Build: Update Boost download URL (Fuzzbawls) 7be66c9 Doc: document updated boost version in dependencies.md (Fuzzbawls) bcb77b6 depends: boost: Specify cflags+compileflags (Carl Dong) b8f8574 depends: boost: Remove unnecessary _archiver_ (Carl Dong) 29fdbd9 depends: boost: Cleanup toolset selection (Carl Dong) 28393b6 depends: boost: Cleanup architecture/address-model (Carl Dong) 6af3ffa depends: boost: Disable all compression (Carl Dong) 0f09788 depends: boost: Split into non-/native packages (Carl Dong) de97b06 depends: boost: Bump to 1.71.0 (Carl Dong) 19f474b depends: boost: Refer to version in URL (Carl Dong) 7d4257c depends: Propagate only specific CLI variables to sub-makes (Carl Dong) fcbf870 depends: boost: Use clang toolset if clang in CXX (Carl Dong) aad5009 depends: boost: Split target-os from toolset (Carl Dong) fae749b depends: boost: Specify toolset to bootstrap.sh (Carl Dong) c2bfedb depends: Propagate well-known vars into depends (Carl Dong) 091ae4a depends: Consistent use of package variable (Peter Bushnell) 635bdc1 depends: fix boost mac cross build with clang 9+ (Cory Fields) d796365 build: Add variable printing target to Makefiles (Carl Dong) Pull request description: Backports the following upstream PRs to clean up and update the Boost dependency - bitcoin#17087 - bitcoin#17231 - bitcoin#17928 - bitcoin#18820 - bitcoin#19764 ACKs for top commit: random-zebra: ACK de7766c furszy: no code changes after rebase, utACK de7766c and merging.. Tree-SHA512: 4abe88718892bce40a2df023e99a26a16ce3c5d470f55e70d6c6cca117ee8b8bb29968be6d40873bc9ece3f9df769bea248cbb38c1c5c2f318016702533f2736

…eanup

The bare minimum necessary change for bitcoin#19764. Marked as "partial merge" for attribution only, otherwise considered unmerged

merge bitcoin#18820, bitcoin#19764, bitcoin#13686, bitcoin#17538, bitcoin#18405: zmq backports, boost depends split

7a89f2e build: Fix target name (Hennadii Stepanov) Pull request description: It seems like a typo :) This PR: - fixes errors when building a package in depends for `HOST=x86_64-apple-darwin16` (fix bitcoin#19799) - is a correct alternative to bitcoin@d25e0e3 from bitcoin#19764 ACKs for top commit: icota: tACK bitcoin@7a89f2e dongcarl: Code Review ACK 7a89f2e theuni: ACK 7a89f2e. Tree-SHA512: a0bcbc6805d3450e201476ef1e22e0eb53903db1586c5515314c19afd337bded887e56de0fbe62feaf359b2de15dbccd49a44f1a8b566b4c64f5ae3d94a2ab6d

dongcarl added Build system Needs gitian build DrahtBot Guix build requested labels Aug 18, 2020

fanquake reviewed Aug 20, 2020

View reviewed changes

This was referenced Aug 20, 2020

build: improve sed robustness by not using sed #19761

Merged

Replace boost::filesystem with std::filesystem #19245

Closed

DrahtBot removed Needs gitian build DrahtBot Guix build requested labels Aug 21, 2020

dongcarl force-pushed the 2020-08-boost-build-pickup branch 2 times, most recently from ce84a8f to 8c38443 Compare August 25, 2020 16:42

DrahtBot added the Needs rebase label Aug 27, 2020

theuni reviewed Aug 27, 2020

View reviewed changes

depends/funcs.mk Outdated Show resolved Hide resolved

This was referenced Aug 31, 2020

build: Error when bulding a package in depends for HOST=x86_64-apple-darwin16 #19799

Closed

build: Fix target name #19868

Merged

dongcarl force-pushed the 2020-08-boost-build-pickup branch from 8c38443 to 68d53e8 Compare September 23, 2020 19:11

DrahtBot removed the Needs rebase label Sep 23, 2020

ryanofsky approved these changes Sep 25, 2020

View reviewed changes

bitcoin deleted a comment from DrahtBot Sep 26, 2020

fanquake requested changes Oct 1, 2020

View reviewed changes

dongcarl force-pushed the 2020-08-boost-build-pickup branch from 68d53e8 to 207f820 Compare November 6, 2020 15:35

dongcarl added 3 commits November 23, 2020 15:50

depends: boost: Cleanup toolset selection

ab9e047

depends: boost: Remove unnecessary _archiver_

b2328b7

We already have $(package)_ar, so just use that instead

depends: boost: Specify cflags+compileflags

f190343

dongcarl force-pushed the 2020-08-boost-build-pickup branch from 207f820 to f190343 Compare November 23, 2020 20:51

maflcko added Needs gitian build DrahtBot Guix build requested labels Nov 24, 2020

laanwj merged commit 7ebbee5 into bitcoin:master Nov 24, 2020

laanwj added a commit to laanwj/bitcoin that referenced this pull request Nov 24, 2020

doc: Document current boost dependency as 1.71.0

0918eb4

This was forgotten in bitcoin#19764.

laanwj mentioned this pull request Nov 24, 2020

doc: Document current boost dependency as 1.71.0 #20473

Merged

maflcko removed DrahtBot Guix build requested Needs gitian build labels Nov 25, 2020

str4d mentioned this pull request Jan 15, 2021

Boost build backports zcash/zcash#4950

Merged

Fuzzbawls mentioned this pull request Apr 22, 2021

[Depends] Bump boost to 1.71.0 and optimize flow PIVX-Project/PIVX#2331

Merged

kwvg added a commit to kwvg/dash that referenced this pull request Aug 22, 2021

bitcoin#19764: Split boost into build/host packages + bump + cleanup

fee7bfa

kwvg added a commit to kwvg/dash that referenced this pull request Aug 22, 2021

bitcoin#19764: Split boost into build/host packages + bump + cleanup

ff42f36

kwvg added a commit to kwvg/dash that referenced this pull request Aug 27, 2021

merge bitcoin#19764: Split boost into build/host packages + bump + cl…

6a367fc

…eanup

kwvg added a commit to kwvg/dash that referenced this pull request Aug 27, 2021

partial merge bitcoin#18677: Multiprocess build support

a003178

The bare minimum necessary change for bitcoin#19764. Marked as "partial merge" for attribution only, otherwise considered unmerged

PastaPastaPasta added a commit to dashpay/dash that referenced this pull request Aug 31, 2021

Merge pull request #4356 from kittywhiskers/depends

bbc8623

merge bitcoin#18820, bitcoin#19764, bitcoin#13686, bitcoin#17538, bitcoin#18405: zmq backports, boost depends split

lyricidal mentioned this pull request Nov 1, 2021

[Upstream] [Depends] Bump boost to 1.71.0 and optimize flow PRCYCoin/PRCYCoin#139

Merged

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

depends: Split boost into build/host packages + bump + cleanup #19764

depends: Split boost into build/host packages + bump + cleanup #19764

Uh oh!

dongcarl commented Aug 18, 2020 •

edited

Loading

Uh oh!

Empact commented Aug 19, 2020

Uh oh!

fanquake left a comment

Uh oh!

Uh oh!

Uh oh!

fanquake Aug 20, 2020

Uh oh!

Uh oh!

DrahtBot commented Aug 20, 2020 •

edited

Loading

Uh oh!

dongcarl commented Aug 25, 2020

Uh oh!

theuni commented Aug 27, 2020

Uh oh!

Uh oh!

hebasto commented Sep 16, 2020

Uh oh!

dongcarl commented Sep 23, 2020

Uh oh!

practicalswift commented Sep 23, 2020

Uh oh!

ryanofsky left a comment •

edited

Loading

Uh oh!

fanquake left a comment

Uh oh!

dongcarl commented Nov 23, 2020

Uh oh!

laanwj commented Nov 24, 2020

Uh oh!

Uh oh!

depends: Split boost into build/host packages + bump + cleanup #19764

depends: Split boost into build/host packages + bump + cleanup #19764

Uh oh!

Conversation

dongcarl commented Aug 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Empact commented Aug 19, 2020

Uh oh!

fanquake left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fanquake Aug 20, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DrahtBot commented Aug 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

dongcarl commented Aug 25, 2020

Uh oh!

theuni commented Aug 27, 2020

Uh oh!

Uh oh!

hebasto commented Sep 16, 2020

Uh oh!

dongcarl commented Sep 23, 2020

Uh oh!

practicalswift commented Sep 23, 2020

Uh oh!

ryanofsky left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fanquake left a comment

Choose a reason for hiding this comment

Uh oh!

dongcarl commented Nov 23, 2020

Uh oh!

laanwj commented Nov 24, 2020

Uh oh!

Uh oh!

dongcarl commented Aug 18, 2020 •

edited

Loading

DrahtBot commented Aug 20, 2020 •

edited

Loading

ryanofsky left a comment •

edited

Loading