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.

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

depends: fix boost mac cross build with clang 9+ #17231

Merged
merged 1 commit into from
Oct 25, 2019

Conversation

theuni
Copy link
Member

@theuni theuni commented Oct 23, 2019

The ancient "darwin-4.9.1" profile has long been used to match against clang, which prior to version 9, reported 4.9.1 as its version when invoking "clang++ -dumpversion". Presumably this was a historical compatibility quirk related to Apple's switch from gcc to clang.

This was "fixed" in clang 9.0, so that -dumpversion reports the real version. Unfortunately that had the side-effect of breaking the (brittle) boost compiler detection.

Move to the seemingly more-correct "clang-darwin" profile, which passes the checks and builds correctly.

Also switch to using ar rather than libtool for archiving, as it's what the clang-darwin profile expects to be using.

Note that because this is using a different profile, some of the final command-line arguments end up changing. Those changes look sane at a glance.

The ancient "darwin-4.9.1" profile has long been used to match against
clang, which prior to version 9, reported 4.9.1 as its version when
invoking "clang++ -dumpversion". Presumably this was a historical
compatibility quirk related to Apple's switch from gcc to clang.

This was "fixed" in clang 9.0, so that -dumpversion reports the real
version. Unfortunately that had the side-effect of breaking the
(brittle) boost compiler detection.

Move to the seemingly more-correct "clang-darwin" profile, which passes
the checks and builds correctly.

Also switch to using ar rather than libtool for archiving, as it's what
the clang-darwin profile expects to be using.

Note that because this is using a different profile, some of the final
command-line arguments end up changing. The changes look sane at a
glance.
@fanquake
Copy link
Member

ACK 50037e9 - tested on on macOS, will wait for the gitian build.

This came up as part of the discussion in #16392, which was going to bump our Clang version from 3.7.1 to 6.0.1. Cory suggested moving straight to latest (9.0.0), and we discovered that Boost wouldn't build.

For a long time, Clangs -dumpversion has been hardcoded to return 4.2.1 (for compatibility with GCC), this was changed with Clang 9.0.0 (details here).

This breaks our Boost compilation when using toolset=darwin-4.2.1. You can see how Boost is currently being configured on macOS in darwin.jam. The new profile is clang-darwin.jam.

Also switch to using ar rather than libtool for archiving, as it's what the clang-darwin profile expects to be using.

The clang-darwin config does expect ar.
The darwin config used to use libtool.

Note that because this is using a different profile, some of the final command-line arguments end up changing. Those changes look sane at a glance.

From what I can see, the changed options between the two configurations are:

We've lost -gdwarf-2 -fexceptions -Wno-long-long -fpermissive -pedantic:

-gdwarf-2 Produce debugging information in DWARF version 2 format
-fexceptions Enable exception handling.
-fpermissive Downgrade some diagnostics about nonconformant code from errors to warnings.
-Wpedantic Issue warnings needed for strict compliance to the standard.

and picked up -x c++ -Wno-variadic-macros:

-x <language> Treat subsequent input files as having type <language>
-Wvariadic-macros Warn about using variadic macros.

Note that -Wno-long-long is still being passed, it just used to be passed twice. Example full commands are below.

toolset=darwin-4.2.1:

darwin.compile.c++ bin.v2/libs/thread/build/darwin-4.2.1/release/link-static/threadapi-pthread/threading-multi/visibility-hidden/future.o

    "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++" "-mmacosx-version-min=10.10" "-stdlib=libc++" "--sysroot" "/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk"   -fvisibility-inlines-hidden -std=c++11 -fvisibility=hidden     -I/Users/michael/github/bitcoin/depends/x86_64-apple-darwin18.7.0/include     -m64 -O3 -Wall -fvisibility=hidden -gdwarf-2 -fexceptions -Wno-long-long -Wno-inline -Wextra -Wno-long-long -Wno-unused-parameter -Wunused-function -fpermissive -pedantic -DBOOST_ALL_NO_LIB=1 -DBOOST_THREAD_BUILD_LIB=1 -DBOOST_THREAD_DONT_USE_CHRONO -DBOOST_THREAD_POSIX -DNDEBUG  -I"." -c -o "bin.v2/libs/thread/build/darwin-4.2.1/release/link-static/threadapi-pthread/threading-multi/visibility-hidden/future.o" "libs/thread/src/future.cpp"

clang-darwin:

clang-darwin.compile.c++ bin.v2/libs/thread/build/clang-darwin-11.0/release/link-static/threadapi-pthread/threading-multi/visibility-hidden/future.o

    "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++" "-mmacosx-version-min=10.10" "-stdlib=libc++" "--sysroot" "/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk" -x c++ -fvisibility-inlines-hidden -std=c++11 -fvisibility=hidden     -I/Users/michael/github/bitcoin/depends/x86_64-apple-darwin18.7.0/include     -m64 -O3 -Wall -fvisibility=hidden -Wno-inline -Wextra -Wno-long-long -Wno-unused-parameter -Wno-variadic-macros -Wunused-function -DBOOST_ALL_NO_LIB=1 -DBOOST_THREAD_BUILD_LIB=1 -DBOOST_THREAD_DONT_USE_CHRONO -DBOOST_THREAD_POSIX -DNDEBUG -I"." -c -o "bin.v2/libs/thread/build/clang-darwin-11.0/release/link-static/threadapi-pthread/threading-multi/visibility-hidden/future.o" "libs/thread/src/future.cpp"

@theuni
Copy link
Member Author

theuni commented Oct 24, 2019

This came up as part of the discussion in #16392, which was going to bump our Clang version from 3.7.1 to 6.0.1. Cory suggested moving straight to latest (9.0.0), and we discovered that Boost wouldn't build.

Just a slight clarification: I suggested that if we're bumping clang, maybe we should bump higher than 6.0. I jumped straight to 9.0 first, just to make sure we still work with the most recent release.

This change will be necessary eventually, and should work fine with older clang builds as well.

Now that we can (presumably) pick anything between 3.7 and 9.0, we can evaluate and choose whatever makes the most sense.

@laanwj
Copy link
Member

laanwj commented Oct 24, 2019

-gdwarf-2 Produce debugging information in DWARF version 2 format

I think that's good. DWARF 2 is really ancient (90's), and not useful at all for modern C++ compiler output.

@maflcko
Copy link
Member

maflcko commented Oct 24, 2019

Concept ACK, haven't looked at the diff

@DrahtBot
Copy link
Contributor

Gitian builds for commit b688b85 (master):

Gitian builds for commit c266777 (master and this pull):

laanwj added a commit that referenced this pull request Oct 25, 2019
50037e9 depends: fix boost mac cross build with clang 9+ (Cory Fields)

Pull request description:

  The ancient "darwin-4.9.1" profile has long been used to match against clang, which prior to version 9, reported 4.9.1 as its version when invoking "clang++ -dumpversion". Presumably this was a historical compatibility quirk related to Apple's switch from gcc to clang.

  This was "fixed" in clang 9.0, so that -dumpversion reports the real version. Unfortunately that had the side-effect of breaking the (brittle) boost compiler detection.

  Move to the seemingly more-correct "clang-darwin" profile, which passes the checks and builds correctly.

  Also switch to using ar rather than libtool for archiving, as it's what the clang-darwin profile expects to be using.

  Note that because this is using a different profile, some of the final command-line arguments end up changing. Those changes look sane at a glance.

ACKs for top commit:
  fanquake:
    ACK 50037e9 - tested on on macOS, will wait for the gitian build.

Tree-SHA512: eac1f353513a445add6fbece7fc78dd3dbdde5e2219bfb7739b82f40bb14de449667a94d2e303d43c67d9b38e7ceb0ba5f0d8fe20b40be2017b1ca0875467c2c
@laanwj laanwj merged commit 50037e9 into bitcoin:master Oct 25, 2019
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Apr 6, 2020
Summary:
```
The ancient "darwin-4.9.1" profile has long been used to match against
clang, which prior to version 9, reported 4.9.1 as its version when
invoking "clang++ -dumpversion". Presumably this was a historical
compatibility quirk related to Apple's switch from gcc to clang.

This was "fixed" in clang 9.0, so that -dumpversion reports the real
version. Unfortunately that had the side-effect of breaking the
(brittle) boost compiler detection.

Move to the seemingly more-correct "clang-darwin" profile, which passes
the checks and builds correctly.

Also switch to using ar rather than libtool for archiving, as it's what
the clang-darwin profile expects to be using.

Note that because this is using a different profile, some of the final
command-line arguments end up changing. The changes look sane at a
glance.
```

Backport of core [[bitcoin/bitcoin#17231 | PR17231]].

Test Plan: Run the OSX Gitian build.

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D5672
MarkLTZ added a commit to litecoinz-core/litecoinz that referenced this pull request Apr 7, 2020
- doc: fix git add argument bitcoin#18513
- build: Fix libevent linking for bench_bitcoin binary bitcoin#18397
- script: fix SCRIPT_ERR_SIG_PUSHONLY error string bitcoin#18412
- doc: Comment fix merkle.cpp bitcoin#18379
- log: Fix UB with bench on genesis block bitcoin#15283
- test: Fix mining to an invalid target + ensure that a new block has the correct hash internally bitcoin#18350
- Fix missing header in sync.h bitcoin#18357
- refactor: Fix implicit value conversion in formatPingTime bitcoin#18260
- Fix .gitignore policy in build_msvc directory bitcoin#18108
- build: Fix behavior when ALLOW_HOST_PACKAGES unset bitcoin#18051
- test: Fix p2p_invalid_messages failing in Python 3.8 because of warning bitcoin#17931
- qa: Fix double-negative arg test bitcoin#17893
- build: Fix configure report about qr bitcoin#17547
- log: Fix log message for -par=1 bitcoin#17325
- bench: Fix negative values and zero for -evals flag bitcoin#17267
- depends: fix boost mac cross build with clang 9+ bitcoin#17231
- doc: Fix broken bitcoin-cli examples bitcoin#17119
- doc: fix Makefile target in benchmarking.md bitcoin#17081
- contrib: fix minor typos in makeseeds.py bitcoin#17042
- test: Fix Python Docstring to include all Args. bitcoin#17030
- doc: Fix some misspellings bitcoin#17351
- doc: Doxygen-friendly script/descriptor.h comments bitcoin#16947
- doc: Fix doxygen errors bitcoin#17945
- doc: Doxygen-friendly CuckooCache comments bitcoin#16986
- doc: Add to Doxygen documentation guidelines bitcoin#17873
MarkLTZ added a commit to litecoinz-core/litecoinz that referenced this pull request Apr 7, 2020
- doc: fix git add argument bitcoin#18513
- build: Fix libevent linking for bench_bitcoin binary bitcoin#18397
- script: fix SCRIPT_ERR_SIG_PUSHONLY error string bitcoin#18412
- doc: Comment fix merkle.cpp bitcoin#18379
- log: Fix UB with bench on genesis block bitcoin#15283
- test: Fix mining to an invalid target + ensure that a new block has the correct hash internally bitcoin#18350
- Fix missing header in sync.h bitcoin#18357
- refactor: Fix implicit value conversion in formatPingTime bitcoin#18260
- Fix .gitignore policy in build_msvc directory bitcoin#18108
- build: Fix behavior when ALLOW_HOST_PACKAGES unset bitcoin#18051
- test: Fix p2p_invalid_messages failing in Python 3.8 because of warning bitcoin#17931
- qa: Fix double-negative arg test bitcoin#17893
- build: Fix configure report about qr bitcoin#17547
- log: Fix log message for -par=1 bitcoin#17325
- bench: Fix negative values and zero for -evals flag bitcoin#17267
- depends: fix boost mac cross build with clang 9+ bitcoin#17231
- doc: Fix broken bitcoin-cli examples bitcoin#17119
- doc: fix Makefile target in benchmarking.md bitcoin#17081
- contrib: fix minor typos in makeseeds.py bitcoin#17042
- test: Fix Python Docstring to include all Args. bitcoin#17030
- doc: Fix some misspellings bitcoin#17351
- doc: Doxygen-friendly script/descriptor.h comments bitcoin#16947
- doc: Fix doxygen errors bitcoin#17945
- doc: Doxygen-friendly CuckooCache comments bitcoin#16986
- doc: Add to Doxygen documentation guidelines bitcoin#17873
- depends: Consistent use of package variable bitcoin#17928
- init: Replace URL_WEBSITE with PACKAGE_URL bitcoin#18503
- doc: Add missed copyright headers bitcoin#17691
ftrader pushed a commit to bitcoin-cash-node/bitcoin-cash-node that referenced this pull request Aug 17, 2020
Summary:
```
The ancient "darwin-4.9.1" profile has long been used to match against
clang, which prior to version 9, reported 4.9.1 as its version when
invoking "clang++ -dumpversion". Presumably this was a historical
compatibility quirk related to Apple's switch from gcc to clang.

This was "fixed" in clang 9.0, so that -dumpversion reports the real
version. Unfortunately that had the side-effect of breaking the
(brittle) boost compiler detection.

Move to the seemingly more-correct "clang-darwin" profile, which passes
the checks and builds correctly.

Also switch to using ar rather than libtool for archiving, as it's what
the clang-darwin profile expects to be using.

Note that because this is using a different profile, some of the final
command-line arguments end up changing. The changes look sane at a
glance.
```

Backport of core [[bitcoin/bitcoin#17231 | PR17231]].

Test Plan: Run the OSX Gitian build.

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D5672
zkbot added a commit to zcash/zcash that referenced this pull request Jan 15, 2021
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!
xdustinface pushed a commit to xdustinface/dash that referenced this pull request Feb 17, 2021
50037e9 depends: fix boost mac cross build with clang 9+ (Cory Fields)

Pull request description:

  The ancient "darwin-4.9.1" profile has long been used to match against clang, which prior to version 9, reported 4.9.1 as its version when invoking "clang++ -dumpversion". Presumably this was a historical compatibility quirk related to Apple's switch from gcc to clang.

  This was "fixed" in clang 9.0, so that -dumpversion reports the real version. Unfortunately that had the side-effect of breaking the (brittle) boost compiler detection.

  Move to the seemingly more-correct "clang-darwin" profile, which passes the checks and builds correctly.

  Also switch to using ar rather than libtool for archiving, as it's what the clang-darwin profile expects to be using.

  Note that because this is using a different profile, some of the final command-line arguments end up changing. Those changes look sane at a glance.

ACKs for top commit:
  fanquake:
    ACK 50037e9 - tested on on macOS, will wait for the gitian build.

Tree-SHA512: eac1f353513a445add6fbece7fc78dd3dbdde5e2219bfb7739b82f40bb14de449667a94d2e303d43c67d9b38e7ceb0ba5f0d8fe20b40be2017b1ca0875467c2c
furszy added a commit to PIVX-Project/PIVX that referenced this pull request May 9, 2021
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
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants