8000 depends: Allow building with system clang by dongcarl · Pull Request #17919 · bitcoin/bitcoin · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

depends: Allow building with system clang #17919

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

Merged

Conversation

dongcarl
Copy link
Contributor
@dongcarl dongcarl commented Jan 13, 2020

This replaces: #17099


This patchset allows us to force depends to use system clang.

Previously, #17099 removes our dependency on a specific clang we download from llvm.org, but theuni pointed out that since OSX builds are only ever built with a version of clang that is chosen and "blessed" by Apple, it is more likely that the user will encounter problems if they use their system clang. This patchset forces the user to set FORCE_USE_SYSTEM_CLANG=1 in order to use their system clang (when they know what they're doing)

@DrahtBot
Copy link
Contributor
DrahtBot commented Jan 13, 2020

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

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.

@maflcko
Copy link
Member
maflcko commented Feb 11, 2020

wen rebase, sir?

@maflcko
Copy link
Member
maflcko commented Feb 11, 2020

To give some context, I am still running into issues cross-compiling for mac on non-x86 arch. So I am hoping that this pull improves it for me.

Preprocessing native_cctools...
-- The C compiler identification is unknown
-- The CXX compiler identification is unknown
-- The ASM compiler identification is unknown
-- Found assembler: /home/travis/build/MarcoFalke/bitcoin-core/depends/work/build/x86_64-apple-darwin16/native_cctools/3764b223c011574971ee3ae09ce968ba5dc2f00f-fb072d1ddcc/toolchain/bin/clang
-- Check for working C compiler: /home/travis/build/MarcoFalke/bitcoin-core/depends/work/build/x86_64-apple-darwin16/native_cctools/3764b223c011574971ee3ae09ce968ba5dc2f00f-fb072d1ddcc/toolchain/bin/clang
-- Check for working C compiler: /home/travis/build/MarcoFalke/bitcoin-core/depends/work/build/x86_64-apple-darwin16/native_cctools/3764b223c011574971ee3ae09ce968ba5dc2f00f-fb072d1ddcc/toolchain/bin/clang -- broken
CMake Error at /usr/share/cmake-3.10/Modules/CMakeTestCCompiler.cmake:52 (message):
  The C compiler

    "/home/travis/build/MarcoFalke/bitcoin-core/depends/work/build/x86_64-apple-darwin16/native_cctools/3764b223c011574971ee3ae09ce968ba5dc2f00f-fb072d1ddcc/toolchain/bin/clang"

  is not able to compile a simple test program.

  It fails with the following output:

    Change Dir: /home/travis/build/MarcoFalke/bitcoin-core/depends/work/build/x86_64-apple-darwin16/native_cctools/3764b223c011574971ee3ae09ce968ba5dc2f00f-fb072d1ddcc/libtapi/build/CMakeFiles/CMakeTmp
    
    Run Build Command:"/usr/bin/make" "cmTC_09e5c/fast"
    make[1]: Entering directory '/home/travis/build/MarcoFalke/bitcoin-core/depends/work/build/x86_64-apple-darwin1
8000
6/native_cctools/3764b223c011574971ee3ae09ce968ba5dc2f00f-fb072d1ddcc/libtapi/build/CMakeFiles/CMakeTmp'
    /usr/bin/make -f CMakeFiles/cmTC_09e5c.dir/build.make CMakeFiles/cmTC_09e5c.dir/build
    make[2]: Entering directory '/home/travis/build/MarcoFalke/bitcoin-core/depends/work/build/x86_64-apple-darwin16/native_cctools/3764b223c011574971ee3ae09ce968ba5dc2f00f-fb072d1ddcc/libtapi/build/CMakeFiles/CMakeTmp'
    Building C object CMakeFiles/cmTC_09e5c.dir/testCCompiler.c.o
    /home/travis/build/MarcoFalke/bitcoin-core/depends/work/build/x86_64-apple-darwin16/native_cctools/3764b223c011574971ee3ae09ce968ba5dc2f00f-fb072d1ddcc/toolchain/bin/clang    -o CMakeFiles/cmTC_09e5c.dir/testCCompiler.c.o   -c /home/travis/build/MarcoFalke/bitcoin-core/depends/work/build/x86_64-apple-darwin16/native_cctools/3764b223c011574971ee3ae09ce968ba5dc2f00f-fb072d1ddcc/libtapi/build/CMakeFiles/CMakeTmp/testCCompiler.c
    /home/travis/build/MarcoFalke/bitcoin-core/depends/work/build/x86_64-apple-darwin16/native_cctools/3764b223c011574971ee3ae09ce968ba5dc2f00f-fb072d1ddcc/toolchain/bin/clang: 1: /home/travis/build/MarcoFalke/bitcoin-core/depends/work/build/x86_64-apple-darwin16/native_cctools/3764b223c011574971ee3ae09ce968ba5dc2f00f-fb072d1ddcc/toolchain/bin/clang: �ELF����: not found
    /home/travis/build/MarcoFalke/bitcoin-core/depends/work/build/x86_64-apple-darwin16/native_cctools/3764b223c011574971ee3ae09ce968ba5dc2f00f-fb072d1ddcc/toolchain/bin/clang: 2: /home/travis/build/MarcoFalke/bitcoin-core/depends/work/build/x86_64-apple-darwin16/native_cctools/3764b223c011574971ee3ae09ce968ba5dc2f00f-fb072d1ddcc/toolchain/bin/clang: @!: not found
    /home/travis/build/MarcoFalke/bitcoin-core/depends/work/build/x86_64-apple-darwin16/native_cctools/3764b223c011574971ee3ae09ce968ba5dc2f00f-fb072d1ddcc/toolchain/bin/clang: 7: /home/travis/build/MarcoFalke/bitcoin-core/depends/work/build/x86_64-apple-darwin16/native_cctools/3764b223c011574971ee3ae09ce968ba5dc2f00f-fb072d1ddcc/toolchain/bin/clang: Syntax error: ")" unexpected
    CMakeFiles/cmTC_09e5c.dir/build.make:65: recipe for target 'CMakeFiles/cmTC_09e5c.dir/testCCompiler.c.o' failed
    make[2]: *** [CMakeFiles/cmTC_09e5c.dir/testCCompiler.c.o] Error 2
    make[2]: Leaving directory '/home/travis/build/MarcoFalke/bitcoin-core/depends/work/build/x86_64-apple-darwin16/native_cctools/3764b223c011574971ee3ae09ce968ba5dc2f00f-fb072d1ddcc/libtapi/build/CMakeFiles/CMakeTmp'
    Makefile:126: recipe for target 'cmTC_09e5c/fast' failed
    make[1]: *** [cmTC_09e5c/fast] Error 2
    make[1]: Leaving directory '/home/travis/build/MarcoFalke/bitcoin-core/depends/work/build/x86_64-apple-darwin16/native_cctools/3764b223c011574971ee3ae09ce968ba5dc2f00f-fb072d1ddcc/libtapi/build/CMakeFiles/CMakeTmp'

@dongcarl dongcarl force-pushed the 2020-01-depends-macos-allow-system-clang branch from c8926fe to 39da707 Compare February 11, 2020 20:28
@dongcarl
Copy link
Contributor Author

Rebased over #18072 and updated PR description.

@dongcarl
Copy link
Contributor Author

@MarcoFalke This will definitely improve the situation for you (as I think the error you encounter is because we download a clang from llvm.org that's meant to be run on an x86_64 machine), but not sure if it will solve everything.

@dongcarl dongcarl force-pushed the 2020-01-depends-macos-allow-system-clang branch 2 times, most recently from c111d32 to bb5f90c Compare June 10, 2020 20:37
@dongcarl
Copy link
Contributor Author

Rebased over #19240 and made some simplifications. I think it's ready for review!

@dongcarl dongcarl marked this pull request as ready for review June 10, 2020 20:42
@maflcko
Copy link
Member
maflcko commented Jun 11, 2020

Looks like travis may be unable to compile mac depends, but the depends stderr is not shown. See #16368 (comment)

@dongcarl
Copy link
Contributor Author

Looks like travis may be unable to compile mac depends, but the depends stderr is not shown. See #16368 (comment)

I think it's cuz we need the regenerated macOS SDK with the libc++ headers

@dongcarl dongcarl force-pushed the 2020-01-depends-macos-allow-system-clang branch from bb5f90c to 13450e3 Compare June 12, 2020 20:52
@theuni
Copy link
Member
theuni commented Jun 12, 2020

Concept ACK and this looks to be in good shape once rebased on top of feedback/fixups in #19240.

The changes in native_cctools.mk are pretty ugly but straightforward, and current behavior is unchanged as long as FORCE_USE_SYSTEM_CLANG remains unused. Though I think @dongcarl has finally convinced me that we'll probably move on to system-clang by default at some point and drop our weird pinned one. So hopefully the ugliness is only temporary.

Well done!

@theuni
Copy link
Member
theuni commented Jun 12, 2020

Travis should be able to find MacOSX10.14-with-libcxx-headers.sdk.tar.gz now :)

@fanquake
Copy link
Member

Given that we are going to be using a system Clang, I had a bit of a look into what patching Ubuntu and Guix are doing with their Clang packages to see if there was anything we needed to consider; i.e similar to #18921.

Guix

The Clang and LLVM patches are here. They seem fairly mundane. Mostly adjusting include/search paths, and fixing sanitizer build & ABI related issues.

Ubuntu/Debian

For Clang 6.0, the most macOS related patch seems to be silent-test-macho.diff. Looks like it deletes one of the tests for llvm-objdump and macho files. Added in this commit. The Clang package doesn't seem to be modified like GCC, where certain hardening options are defaulted to ON etc.

For Clang-10, one additional macOS related patch has landed, which removes macOS specific info, i.e -mmacosx-version-min, from the man page. The options still exist in Clang, but seem to have been removed from the manpage just to make it less "Mac-centric". Added in this commit. Related bug report: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=743133.

@dongcarl dongcarl force-pushed the 2020-01-depends-macos-allow-system-clang branch from 6110cb3 to 9fe0da8 Compare June 15, 2020 14:10
@fanquake
Copy link
Member

I'm not 100% sure I know what you're talking about though, is there an open issue?

There's no issue, but it was discussed offline IIRC. I'll mention it in #19683, as I think documenting it as part of those changes is probably the easiest thing to do right now.

jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 6, 2020
Summary:
```
This patchset allows us to force depends to use system clang.

Previously, #17099 removes our dependency on a specific clang we
download from llvm.org, but theuni pointed out that since OSX builds are
only ever built with a version of clang that is chosen and "blessed" by
Apple, it is more likely that the user will encounter problems if they
use their system clang. This patchset forces the user to set
FORCE_USE_SYSTEM_CLANG=1 in order to use their system clang (when they
know what they're doing)
```

Backport of core [[bitcoin/bitcoin#17919 | PR17919]].

Test Plan:
  make FORCE_USE_SYSTEM_CLANG=1 build-osx
Note that this diff isn't enough to cross build OSX with the system
clang as the toolchain file will redirect the binutils to the ones from
the depends.

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Subscribers: deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D7755
furszy added a commit to PIVX-Project/PIVX that referenced this pull request May 25, 2021
e1b89ac Fix QPainter non-determinism on macOS (Andrew Chow)
831c317 macOS deploy: use the new plistlib API (Jonas Schnelli)
5857aaf doc: Document ALLOW_HOST_PACKAGES dependency option (skmcontrib)
2329e08 build: Fix behavior when ALLOW_HOST_PACKAGES unset (Hennadii Stepanov)
1768870 depends: native_ds_store 1.3.0 (fanquake)
3f9f3e5 depends: pull upstream libdmg-hfsplus changes (fanquake)
f7606dc depends: latest config.guess & config.sub (fanquake)
cc3ae74 depends: bump native_cctools for fixed lto with external clang (Cory Fields)
b26c648 depends: enable lto support for Apple's ld64 (Cory Fields)
50933d7 depends: Add documentation for FORCE_USE_SYSTEM_CLANG make flag (Carl Dong)
ba3ddf2 depends: Reformat make options as definition list (Carl Dong)
3b855a7 depends: Add justifications for macOS clang flags (Carl Dong)
4104de0 depends: specify libc++ header location for darwin (Cory Fields)
cd4335f depends: force a new host id string if FORCE_USE_SYSTEM_CLANG is in use (Cory Fields)
d30e1af depends: Allow building with system clang (Carl Dong)
234828b depends: Decouple toolchain + binutils (Carl Dong)
1dd3a5a doc: explain why passing -mlinker-version is required (fanquake)
5cc0d0f darwin: pass mlinker-version so that clang enables new features (Cory Fields)
813a552 macos: Bump to xcode 11.3.1 and 10.15 SDK (Cory Fields)
ee7085f depends: bump MacOS toolchain (Cory Fields)
e5b092b contrib: macdeploy: Remove historical extraction notes (Carl Dong)
5893caf contrib: macdeploy: Use apple-sdk-tools instead of xar+pbzx (Carl Dong)
9f2d4ba native_cctools: Don't use libc++ from pinned clang (Carl Dong)
0c8d217 Adapt rest of tooling to new SDK naming scheme (Carl Dong)
bdacfa8 contrib: macdeploy: Correctly generate macOS SDK (Carl Dong)
f7eee2c Fix naming of macOS SDK and clarify version (Andrew Chow)
62f9e23 build: use macOS 10.14 SDK (fanquake)
bc2e1af depends: native_cctools 921, ld64 409.12, libtapi 1000.10.8 (fanquake)
a296d87 depends: clang 6.0.1 (fanquake)
8f6c475 build: Set minimum supported macOS to 10.12 (Fuzzbawls)

Pull request description:

  This backports the following upstream PRs to update the macOS cross-compiling tools:

  bitcoin#17550
  bitcoin#16392
  bitcoin#18589
  bitcoin#19240
  bitcoin#19407
  bitcoin#17919
  bitcoin#19530
  bitcoin#17057
  bitcoin#20333
  bitcoin#18051
  bitcoin#19124
  bitcoin#20298
  bitcoin#20447

  The tools being updated are

  ### Clang
  Upgraded from `3.7.1` to `8.0.0`

  ### cctools

  * cctools `877.8` -> `949.0.1`
  * LD64 `253.9` -> `530`
  * TAPI `1000.10.8`

  ### DSStore
  Upgraded from `1.1.2` to `1.3.0` (this removes the biplist dependency)

  This also effectively bumps our minimum supported macOS version to 10.12 (Sierra).

ACKs for top commit:
  furszy:
    tested ACK e1b89ac
  random-zebra:
    utACK e1b89ac

Tree-SHA512: f5cec8db57e07d8855070646b9e1400d48aac1d01e3c2c3b3e134665c6372d6535f3328888bb9a75087f7b3d5231ecb4b509723bfa51bd40770ffe2810c67f65
kwvg added a commit to kwvg/dash that referenced this pull request Jul 15, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Jul 15, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Jul 20, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Jul 20, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Jul 20, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Jul 20, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Aug 1, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Aug 24, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Aug 24, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Aug 25, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Aug 25, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Aug 26, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Aug 26, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Aug 26, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Aug 27, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Aug 30, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Sep 1, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Sep 1, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Sep 1, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Sep 1, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Sep 2, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Sep 3, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Sep 3, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Sep 3, 2021
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
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.

6 participants
0