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.
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
dongcarl added the label
Build system
on Aug 18, 2020
dongcarl added the label
Needs gitian build
on Aug 18, 2020
dongcarl added the label
Needs Guix build
on Aug 18, 2020
Empact
commented at 3:52 pm on August 19, 2020:
member
You’ve dropped _archiver_, but haven’t replaced it with _ar until the following commit (94c18d5aa8ca7325365c1dd4ab310961383dde64), which means 0fc10d153ea1eaa05c39f9706369f11f5ab3944d doesn’t compile.
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.
in
depends/packages/native_b2.mk:7
in
94c18d5aa8outdated
This doesn’t seem to be working as expected. When I pass CC=clang CXX=clang++, native_b2 is still built using the GCC toolset (Boost is built with Clang). I modified this to use a similar change as you’ve made above, i.e:
Ah, the reason why setting CC=clang and CXX=clang++ doesn’t work is because CC and CXX only control the host_{CC,CXX}, and since native_b2 is a native package, its $(package)_cxx derives its value from build_CXX. So setting build_CXX=clang should work.
As for the case of just setting $(package)_toolset_$(host_os)=clang, $(package)_toolset_$(host_os) strictly depends on $(package)_cxx, so changing that by itself won’t influence $(package)_cxx. Since it’s a strict dependency, we should probably remove this variable at some point in the future and replace it with a simple $(if $(findstring clang,$($(package)_cxx)),clang,gcc) to avoid confusion.
fanquake
commented at 6:52 am on August 20, 2020:
member
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?
DrahtBot
commented at 7:51 pm on August 20, 2020:
member
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Conflicts
Reviewers, this pull request conflicts with the following ones:
#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.
DrahtBot removed the label
Needs gitian build
on Aug 21, 2020
DrahtBot removed the label
Needs Guix build
on Aug 23, 2020
dongcarl force-pushed
on Aug 25, 2020
dongcarl force-pushed
on Aug 25, 2020
dongcarl
commented at 4:43 pm on August 25, 2020:
member
@Empact I updated the description with more justification and context, please let me know if anything’s unclear!
DrahtBot added the label
Needs rebase
on Aug 27, 2020
theuni
commented at 8:06 pm on August 27, 2020:
member
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?
practicalswift
commented at 8:18 pm on September 23, 2020:
contributor
Concept ACK
DrahtBot removed the label
Needs rebase
on Sep 23, 2020
ryanofsky approved
ryanofsky
commented at 4:02 pm on September 25, 2020:
member
Basic code review ACK68d53e8892c71083f4e91931e519c2ac82dfd7a7. 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.
MarcoFalke deleted a comment
on Sep 26, 2020
MarcoFalke deleted a comment
on Sep 26, 2020
fanquake changes_requested
fanquake
commented at 6:03 am on October 1, 2020:
member
This looks good, great PR description 👍 . However in 6cba51ea9fc9c7265a7afa3e77ac6c20937e822b you’ve forgotten to remove the unused_var_in_process.patch file.
dongcarl force-pushed
on Nov 6, 2020
dongcarl
commented at 3:38 pm on November 6, 2020:
member
:facepalm: Rebased and removed the patch! Should be good to go!
MarcoFalke added the label
Needs gitian build
on Nov 7, 2020
MarcoFalke added the label
Needs Guix build
on Nov 7, 2020
DrahtBot
commented at 7:05 am on November 9, 2020:
member
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2025-04-08 18:12 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me
