-
Notifications
You must be signed in to change notification settings - Fork 725
[Depends] Update macOS cross-compiling toolchain #2272
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] Update macOS cross-compiling toolchain #2272
Conversation
This also removes some now-unnecessary cctools hacks. Co-Authored-By: Cory Fields <[email protected]>
This also removes the obsolete mlinker-version option Co-Authored-By: Cory Fields <[email protected]>
Co-Authored-By: Carl Dong <[email protected]>
Previously, we did not include the macOS SDK libc++ headers in our SDK creation process and instead used whichever libc++ headers shipped with the clang package we downloaded in depends. This change adds a script (which works on both GNU/Linux and macOS) to correctly generate the macOS SDK including the libc++ headers. This can be thought of as a simplified rewrite of tpoechtrager's script: https://github.com/tpoechtrager/osxcross/blob/d3392f4eae78f3fa3f1fd065fa423f2712825102/tools/gen_sdk_package.sh The location within the SDK where we place the libc++ headers is chosen such that clang's search path detection logic for sysroots would pick up the headers properly. We also document this change.
Now that we include the macOS SDK libc++ headers in our macOS SDK tarball, we no longer need this hack to use the libc++ from our pinned clang.
clang 6.0.1 -> 8.0.0 cctools 921 -> 949.0.1 ld64 409.12 -> 530
This gets us a newer SDK with c++17 support and retains 10.12 back-compat. Co-authored-by: Carl Dong <[email protected]>
Without this clang fails to add any newly-added linker features. Removing this in ca5055a was likely a regression. See bitcoin#19240 (comment) for more discussion.
For now they remain the same, but in the next commit, we will assign them differently according to wether or not we're using system clang.
This should be caught by the differing clang --version outputs, but because we haven't yet extracted our pinned clang, the system one is actually used for the version check. That's not a problem because bumping our pinned clang will cause a rebuild of everything anyway.
For depends builds this was fixed by fbcfcf6, which deleted the conflicting headers. When we no longer control the clang installation, we need to ensure that the SDK's libc++ headers are used rather than the ones shipped with clang. We can do that by turning off the default include path and hard-coding our own. This hard-coded path is ok because we control (via SDK packaging) where these headers end-up. Side-note: Now that this path is hard-coded in depends, we can potentially package the SDK differently, as the c++ folder can live wherever is most convenient for us.
Note that this does not _enable_ lto by default in any way, only hooks up the machinery for -flto to work correctly. enable-lto-support is explicitly used for pinned-clang because we know it works. It is neither enabled nor disabled in the external clang case so that it can be auto-detected.
tpoechtrager/cctools-port#85 was merged upstream, which fixes lto detection for external clang with some Linux Distro's including Ubuntu.
native_ds_store now takes advantage of Pythons ability to decode binary plists (since 3.4), so we can drop its biplist dependency. The call to biplist.Data() in custom_dsstore doesn't seem to do anything, and from what I can tell can just be removed.
See https://docs.python.org/3/library/plistlib.html. The new API was added in 3.4 and old removed in 3.9.
Aplies a patch to Qt that fixes the non-determinism by modifying Qt. The source of the non-determinism is how LLVM 8 optimizes qt_intersect_spans when compiling. The particular optimization that seems to be causing the problems is that a temp variable is being added for spans->y. For some reason, when it does this, it chooses different instructions to use when making that variable. We bypass this problem by patching qt_intersect_spans to always make and use this local variable.
6a1af2b to
e1b89ac
Compare
furszy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking quite nice ☕ ☕, this is something that we should had done long time ago.
Some extra points:
You can remove the random.cpp getentropy() fallback for macOS < 10.12 (bitcoin#18364) .
Plus there are other macOS < 10.12 fallbacks that can be cleaned in other places as well.
I modified some of them here https://github.com/furszy/PIVX/tree/2021_depends-macos-toolchain. (If you want to cherry-pick them here, you will need to rebase this PR on master, the last commit will conflict with the util.cpp/system.cpp rename that was recently merged).
Aside from that, will do a gitian run.
|
Yeah, I intentionally left any Qt version fallback logic in the |
|
yep, sounds good. |
furszy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested ACK e1b89ac
Hashes report:
51db9a64bc2d4cf95b34aba30cdf17601d29cbc0bceb45acc60b4b6eea58ec3b pivx-5.1.99-osx-unsigned.dmg
c75e135e86847d4676b8c560d06e22f5f211b312e3c5753eaab52c3300071971 pivx-5.1.99-osx-unsigned.tar.gz
3bdb78d5f041f7ebeea1faa3bf1fd39bc2ab4a02652014159eb2db005e4da14e pivx-5.1.99-osx64.tar.gz
a942ac90d00dd368613ea3ee8fd8b365d6d25aa019f6fe6ea33ab8b437fb6dc9 src/pivx-5.1.99.tar.gz
f47df6e0435dee6cd3369a0ee65ecf94a35792f4bc7e96753d02cf383bc22376 pivx-osx-6.0-res.yml
Took me some time to make it work locally. The gitian-build script limitations to build from a PR made me consume some time.
Had to manually checkout and rebase on master the local repo. Plus, fresh gitian-builds for OSX alone doesn't work, the target requires to have downloaded the rust target for linux: rust-1.42.0-x86_64-unknown-linux-gnu.tar.gz (this is not related to this PR, happen in master as well).
random-zebra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK e1b89ac
a68c7d5 depends: mac_alias 2.2.0 (fanquake) d8e2baf doc: Add explicit macdeployqtplus dependencies install step (Hennadii Stepanov) 013305d macdeploy: use Python 3.6 (fanquake) faf77c3 macdeploy: remove runHDIUtil in favor of directly calling subprocess.run (fanquake) 8bcfd58 macdeploy: remove existing PIVX-Core.dmg if present (fanquake) 023d3ca macdeploy: move qt_conf to where it's used (fanquake) 7cdb5bb macdeploy: consolidate .DS_Store generation (fanquake) 4da04d7 macdeploy: assume plistlib is available (fanquake) 56ab77a macdeploy: have a single level of logging output (fanquake) d111cdf macdeploy: remove add-resources argument (fanquake) 4312410 macdeploy: remove codesigning argument (fanquake) c2ee635 build: automatically determine macOS translations (fanquake) 1c44ecf scripts: filter more qt plugins we don't use in macdeployqtplus (fanquake) c854f78 scripts: misc cleanups in macdeployqtplus (fanquake) a3873ea scripts: use format() in macdeployqtplus (fanquake) a65bea5 scripts: add type annotations to macdeployqtplus (fanquake) ba179e5 build: Drop macports support (Ben Woosley) Pull request description: This is a companion to #2272 that focuses on on the `.dmg` creation aspect of macOS builds (ie, `make deploy`). The following upstream PRs are backported here: - bitcoin#15175 - bitcoin#16477 - bitcoin#20422 - bitcoin#20890 - bitcoin#21658 Also worth mentioning: This drops support for MacPorts entirely, which has been antiquated and un-maintained for quite some time, and never actually used by any PIVX macOS build doc. ACKs for top commit: furszy: Tested using depends, ACK a68c7d5. random-zebra: utACK a68c7d5 and merging... Tree-SHA512: 3e9fa81a905ca3e90f07ff1213ec69dd1220a19a6a215f256ab67f2594476dc95e8fe88f15a1c9f3314b1757a7a2e5d8e6d7a790d85c117bf4236a3833757430
The macOS startup item code was disabled for builds targeting macOS > 10.11 in bitcoin#2272. Now that we require macOS 10.12 as a minimum, we can remove the startup item code entirely, as the API we were using was removed in macOS 10.12.
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.1to8.0.0cctools
877.8->949.0.1253.9->5301000.10.8DSStore
Upgraded from
1.1.2to1.3.0(this removes the biplist dependency)This also effectively bumps our minimum supported macOS version to 10.12 (Sierra).