build: macOS toolchain simplification and bump

dongcarl commented at 8:22 pm on June 10, 2020: member

This PR achieves 3 main things:

It simplifies the macOS SDK generation by putting the logic inside a (semi-)portable python3 script gen-sdk
It transitions us to using libc++ headers extracted from the Xcode.app, which is more correct as those headers better match the .tbd library stubs we use from the MacOSX.sdk (located under the same Xcode.app). Previously, we used libc++ headers copied from our downloaded, pinned clang (see native_cctools.mk).
It bumps the macOS toolchain in a way that fulfills all of the following constraints:
1. The new SDK should support compiling with C++17 (our current one doesn’t)
2. The new toolchain should not change our minimum supported macOS version (-mmacosx-version-min)
3. The new toolchain should expect to use a version of cctools that is supported by https://github.com/tpoechtrager/cctools-port

For the constraints in (3), you can reference this chart to see that the newest toolchain we can use with our cctools-port is 11.3.1, and the rest of the constraints were tested with local builds.

But the other Wikipedia chart says that the “min macOS to run” for Xcode 11.3.1 is 10.14.4, doesn’t that violate constraint (ii)?

This confused me at first too, but the “min macOS to run” is for the Xcode.app App itself. The SDK still supports 10.12, as evident in a few plist files and as proven through local builds.

Why bundle all of this together in a single PR?

We need (1) and (2) together, because if we don’t, manually adding the libc++ headers and writing that out in a README.md is going to result in a lot of user error, so it’s great to have these together to be more correct and also make it easier on the user at the same time.

We need (3) together with everything else because bumping (or in the case of (1), renaming) the SDK requires some human coordination and may break some builds. And since it’s not that complicated a change, it makes sense to do it together with the rest.

dongcarl renamed this:
~~2020 06 macos sdkgen simplify~~
Simplify macOS SDK generation and include `libc++` headers
on Jun 10, 2020

dongcarl added the label Build system on Jun 10, 2020

dongcarl force-pushed on Jun 10, 2020

laanwj commented at 5:54 pm on June 11, 2020: member

For anyone who’s looked at the last PR, this one just has a simpler sdk-gen.sh script that I wrote myself, and includes some .xip unpacking innovations thanks to theuni.

Thanks for working on this. This is great news. Maybe can manage to do it myself this time without a Mac :+1:

in contrib/macdeploy/README.md:54 in ac1ded1a3f outdated

72-tar -s "/MacOSX.sdk/MacOSX10.14.sdk/" -C Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/ -czf MacOSX10.14.sdk.tar.gz MacOSX.sdk
73+# Generate a MacOSX10.14.sdk.tar.gz from the supplied Xcode.app
74+./contrib/macdeploy/gen_sdk_package.sh '/path/to/Xcode.app'
75 ```
76 
77+### Historial macOS SDK Extraction Notes

laanwj commented at 5:56 pm on June 11, 2020:

I’m not sure keeping instructions around for previous SDKs is that useful. Only for the release that is used for building. Older information can always be found in git.

dongcarl commented at 6:55 pm on June 11, 2020:

Fixed!

dongcarl force-pushed on Jun 11, 2020

in contrib/macdeploy/README.md:46 in 66aca4bc0c outdated

71-may wish to delete the `intermediate 5.hfs` file and `MacOSX10.11.sdk` (the
72-directory) when you've confirmed the extraction succeeded.
73+### Step 2: Generating `MacOSX10.14.sdk.tar.gz` from `Xcode.app`
74+
75+To generate `MacOSX10.14.sdk.tar.gz`, run the script
76+[`gen_sdk_package.sh`](./gen_sdk_package.sh) with the XCODEDIR environment

theuni commented at 6:39 pm on June 12, 2020:

“with the XCODEDIR environment variable pointing to…”, or “./contrib/macdeploy/gen_sdk_package.sh ‘/path/to/Xcode.app’”

Which is it? :)

theuni commented at 6:44 pm on June 12, 2020:

gen_sdk_package.sh != gen-sdk.sh (below as well)

in contrib/macdeploy/gen-sdk.sh:10 in 66aca4bc0c outdated

0@@ -0,0 +1,121 @@
1+#!/usr/bin/env bash
2+export LC_ALL=C
3+set -e -o pipefail
4+
5+die() { cat 1>&2 ; exit 1; }
6+
7+# Defaults to our current SDK's name, but overridable
8+SDKNAME="${SDKNAME:-MacOSX10.14.sdk}"

theuni commented at 6:40 pm on June 12, 2020:

Can we name this something to indicate that we’ve included the headers as well?

Edit: expanding on this: since builders presumably already have a different, incompatible MacOSX10.14.sdk in sources, I think it makes sense to rename the unpacked dir (SDK in hosts/darwin.mk) as well as the tarball to avoid clashes/confusion.

theuni commented at 6:41 pm on June 12, 2020: member

Thanks for doing this. Testing now.

theuni commented at 7:38 pm on June 12, 2020: member

I’m seeing a Qt build failure due to some symlink/hardlink issue:

0kernel/qdnslookup_unix.cpp:52:10: fatal error: 'arpa/nameser.h' file not found
1#include <arpa/nameser.h>
2         ^~~~~~~~~~~~~~~~
31 error generated.
4Makefile:12337: recipe for target '.obj/qdnslookup_unix.o' failed

The link is broken hardlink in the generated sdk:

0 $ ls -al MacOSX10.14.sdk/usr/include/arpa/nameser.h
1lrwxrwxrwx 1 cory cory 26 Jun 12 14:28 MacOSX10.14.sdk/usr/include/arpa/nameser.h -> MacOSX10.14.sdk./nameser.h

Whereas in the original SDK it’s a working symlink:

0 $ ls -al MacOSX10.14.sdk/usr/include/arpa/nameser.h
1lrwxrwxrwx 1 cory cory 12 Jun 12 14:28 MacOSX10.14.sdk/usr/include/arpa/nameser.h -> ../nameser.h

Edit: For reference, here’s my extraction output from Ubuntu 18.04.2:

 0cory@desktop:15:41:55:~/dev/bitcoin2 (19240)
 1 $ ./contrib/macdeploy/gen-sdk.sh /tmp/apple-sdk-tools/Xcode.app
 2Going to attempt to extract the MacOSX10.14.sdk SDK from the Xcode.app located at:
 3	'/tmp/apple-sdk-tools/Xcode.app'
 4
 5And place the resulting gzipped tarball at:
 6	'/home/cory/dev/bitcoin2/MacOSX10.14.sdk.tar.gz'
 7
 8Going to use tar command: 'tar'
 9Detected tar style: 'GNU'
10
11Adding MacOSX10.14.sdk SDK files...
12Adding libc++ headers...
13Compressing using gzip...
14
15Done! Find the resulting gzipped tarball at:
16	'/home/cory/dev/bitcoin2/MacOSX10.14.sdk.tar.gz'

DrahtBot commented at 3:03 am on June 13, 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:

#18288 (build: Add MemorySanitizer (MSan) in Travis to detect use of uninitialized memory by practicalswift)
#17919 (depends: Allow building with system clang by dongcarl)

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 added the label Needs gitian build on Jun 13, 2020

dongcarl added the label Needs Guix build on Jun 13, 2020

dongcarl removed the label Needs Guix build on Jun 13, 2020

fanquake commented at 8:20 am on June 14, 2020: member

Concept ACK.

Cool. Looks like Travis is happy now. Can you squash the fixup commits?

I ran some gitian builds locally to compare the binaries produced by master (195822f1e05e2f36002c906667d4c639663f23b5) and master + this diff:

 0diff --git a/contrib/gitian-descriptors/gitian-osx.yml b/contrib/gitian-descriptors/gitian-osx.yml
 1index bbae7201e..936d097d2 100644
 2--- a/contrib/gitian-descriptors/gitian-osx.yml
 3+++ b/contrib/gitian-descriptors/gitian-osx.yml
 4@@ -32,7 +32,7 @@ remotes:
 5 - "url": "https://github.com/bitcoin/bitcoin.git"
 6   "dir": "bitcoin"
 7 files:
 8-- "MacOSX10.14.sdk.tar.gz"
 9+- "MacOSX10.14-with-libcxx-headers.sdk.tar.gz"
10 script: |
11   set -e -o pipefail
12 
13@@ -90,7 +90,7 @@ script: |
14   BASEPREFIX="${PWD}/depends"
15 
16   mkdir -p ${BASEPREFIX}/SDKs
17-  tar -C ${BASEPREFIX}/SDKs -xf ${BUILD_DIR}/MacOSX10.14.sdk.tar.gz
18+  tar -C ${BASEPREFIX}/SDKs -xf ${BUILD_DIR}/MacOSX10.14-with-libcxx-headers.sdk.tar.gz
19 
20   # Build dependencies for each host
21   for i in $HOSTS; do
22diff --git a/depends/hosts/darwin.mk b/depends/hosts/darwin.mk
23index 82e086a32..70baeae90 100644
24--- a/depends/hosts/darwin.mk
25+++ b/depends/hosts/darwin.mk
26@@ -1,6 +1,6 @@
27 OSX_MIN_VERSION=10.12
28 OSX_SDK_VERSION=10.14
29-OSX_SDK=$(SDK_PATH)/MacOSX$(OSX_SDK_VERSION).sdk
30+OSX_SDK=$(SDK_PATH)/MacOSX$(OSX_SDK_VERSION)-with-libcxx-headers.sdk
31 darwin_CC=clang -target $(host) -mmacosx-version-min=$(OSX_MIN_VERSION) --sysroot $(OSX_SDK)
32 darwin_CXX=clang++ -target $(host) -mmacosx-version-min=$(OSX_MIN_VERSION) --sysroot $(OSX_SDK) -stdlib=libc++
33 
34diff --git a/depends/packages/native_cctools.mk b/depends/packages/native_cctools.mk
35index 4195230b4..19fcf54f9 100644
36--- a/depends/packages/native_cctools.mk
37+++ b/depends/packages/native_cctools.mk
38@@ -73,6 +73,5 @@ define $(package)_stage_cmds
39   cp lib/libLTO.so $($(package)_staging_prefix_dir)/lib/ && \
40   cp -rf lib/clang/$($(package)_clang_version)/include/* $($(package)_staging_prefix_dir)/lib/clang/$($(package)_clang_version)/include/ && \
41   cp bin/llvm-dsymutil $($(package)_staging_prefix_dir)/bin/$(host)-dsymutil && \
42-  if `test -d include/c++/`; then cp -rf include/c++/ $($(package)_staging_prefix_dir)/include/; fi && \
43   if `test -d lib/c++/`; then cp -rf lib/c++/ $($(package)_staging_prefix_dir)/lib/; fi

The binaries produced by the “new” SDK are all slightly larger (except for bitcoin-wallet) than the binaries currently produced by master:

Commit	`bitcoind`	`bitcoin-cli`	`bitcoin-qt`	`bitcoin-tx`	`bitcoin-wallet`	`test_bitcoin`
Master	9084960	399464	24920412	1042272	2721068	15010424
This PR	9135728	407680	24958868	1050504	2717020	15021368

I did a quick comparison of bitcoind, there is a single symbols difference:

 0--- 19.txt
 1+++ ea.txt
 2@@ -26,14 +26,15 @@
 3 U std::bad_exception::what() const
 4 U std::runtime_error::what() const
 5 U std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::find(char, unsigned long) const
 6 U std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::rfind(char, unsigned long) const
 7 U std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::compare(char const*) const
 8 U std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::compare(unsigned long, unsigned long, char const*) const
 9 U std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::compare(unsigned long, unsigned long, char const*, unsigned long) const
10+T std::__1::enable_if<__can_be_converted_to_string_view<char, std::__1::char_traits<char>, std::__1::basic_string_view<char, std::__1::char_traits<char> > >::value, int>::type std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::compare<std::__1::basic_string_view<char, std::__1::char_traits<char> > >(std::__1::basic_string_view<char, std::__1::char_traits<char> > const&) const
11 U std::__1::__shared_weak_count::__get_deleter(std::type_info const&) const
12 U std::__1::__vector_base_common<true>::__throw_length_error() const
13 U std::__1::__vector_base_common<true>::__throw_out_of_range() const

There is also one less initializer function called in the new bitcoind, however I haven’t looked into that yet.

DrahtBot removed the label Needs gitian build on Jun 14, 2020

dongcarl force-pushed on Jun 15, 2020

dongcarl commented at 1:41 pm on June 15, 2020: member

Squashed in the fixups!

in contrib/macdeploy/README.md:32 in 1944c5ff79 outdated

30+approach (tested on Debian Buster) is outlined below:
31 
32 ```bash
33+# Install/clone tools needed for extracting Xcode.app
34+apt install cpio
35+git clone https://github.com/theuni/apple-sdk-tools.git

Sjors commented at 6:02 pm on June 15, 2020:

If we merge this, I’d like to move that repo over to the bitcoin-core org.

in contrib/macdeploy/README.md:35 in 1944c5ff79 outdated

53-./pbzx/pbzx -n Content | cpio -i
54-
55-find Xcode.app -type d -name MacOSX.sdk -exec sh -c 'tar --transform="s/MacOSX.sdk/MacOSX10.14.sdk/" -c -C$(dirname {}) MacOSX.sdk/ | gzip -9n > MacOSX10.14.sdk.tar.gz' \;
56+# Unpack Xcode_10.2.1.xip and place the resulting Xcode.app in your current
57+# working directory
58+python3 apple-sdk-tools/extract_xcode.py -f Xcode_10.2.1.xip | cpio -d -i

Sjors commented at 6:04 pm on June 15, 2020:

When running the extract tool 41bfaca0db3a900c52fb66b6eae794901b9510cb on macOS 10.15.5 I’m flooded with:

0../apple-sdk-tools/extract_xcode.py -f Xcode_10.2.1.xip| cpio -d -i
1./Xcode.app/Contents/Developer/Platforms/AppleTVSimulator.platform/Developer/SDKs/AppleTVSimulator.sdk/usr/include/_ctype.h: Can't create 'Xcode.app/Contents/Developer/Platforms/AppleTVSimulator.platform/Developer/SDKs/AppleTVSimulator.sdk/usr/include/_ctype.h'
2...
328524433 blocks

The shasum’s of the resulting directories, after sorting, obtained with incantation:

0find Xcode.app -type f -print0  | xargs -0 shasum -a 256 > shasum_list_unsorted.txt
1sort shasum_list_unsorted.txt > shasum_list_sorted.txt
2shasum -a 256 shasum_list_sorted.txt 
3ae267efecbfe750e4193903fdf1044af8c7ba263072a11ad99143a34a7e2b1a0  shasum_list_sorted.txt

When using xip -x I get the same checksum, nice!

I get the same shasum on Ubuntu 20.04 (LC_ALL=C sort)

dongcarl commented at 6:22 pm on June 16, 2020:

Nice!

in contrib/macdeploy/README.md:52 in 1944c5ff79 outdated

81 ```bash
82-apt-get install p7zip-full sleuthkit
83-contrib/macdeploy/extract-osx-sdk.sh
84-rm -rf 5.hfs MacOSX10.11.sdk
85+# Generate a MacOSX10.14-with-libcxx-headers.sdk.tar.gz from the supplied Xcode.app
86+./contrib/macdeploy/gen-sdk.sh '/path/to/Xcode.app'

Sjors commented at 6:31 pm on June 15, 2020:

0e7fe352b806ed3cd5a735e1aa476edd0b0c7a24918e49c4c4992203dab1ce28 MacOSX10.14-with-libcxx-headers.sdk.tar.gz when run on macOS, but 6c6319005fe579d0dd174e15ef8d3beab63f1aed4d15e73305eba8c63ffebebb on Ubuntu.

dongcarl commented at 6:23 pm on June 16, 2020:

That’s somewhat expected, as bsdtar and gnutar (and even different versions of gnutar) might lay out the archive in different ways that all conform to standard. :relaxed:

Sjors commented at 7:05 pm on June 16, 2020:

If you can live with a reproducibility issue, it must be fine :-)

Sjors commented at 7:18 pm on June 15, 2020: member

Concept ACK. I did some testing with 1944c5ff798d149422bf6ac7deb999789eef64d2 on macOS and Ubuntu. Code looks reasonable, but mostly above my head.

I found extract_xcode.py to be deterministic, but gen-sdk.sh produces a different checksum depending on the OS.

theuni commented at 7:07 pm on June 16, 2020: member

I discussed this today with @fanquake, and we realized that after this change, we’ll also need to bump our SDK in the near-future so that we can begin testing with c++17. And when we do that, it’s also a good time to bump our clang/cctools to match.

I hacked that together and tested quickly, and all seems to be working as expected: https://github.com/theuni/bitcoin/tree/19240

At the risk of slowing down this PR, we could potentially integrate the rest of the toolchain bump in here as well. The upside of that would be that builders only have to download and generate a new sdk tarball once, whereas if we do this in stages, they’ll have do it twice.

But then again, Carl has been maintaining these changes for quite a while now and is probably tired of waiting. @dongcarl Your call :)

in depends/packages/native_cctools.mk:76 in 1944c5ff79 outdated

72@@ -73,6 +73,5 @@ define $(package)_stage_cmds
73   cp lib/libLTO.so $($(package)_staging_prefix_dir)/lib/ && \
74   cp -rf lib/clang/$($(package)_clang_version)/include/* $($(package)_staging_prefix_dir)/lib/clang/$($(package)_clang_version)/include/ && \
75   cp bin/llvm-dsymutil $($(package)_staging_prefix_dir)/bin/$(host)-dsymutil && \
76-  if `test -d include/c++/`; then cp -rf include/c++/ $($(package)_staging_prefix_dir)/include/; fi && \
77   if `test -d lib/c++/`; then cp -rf lib/c++/ $($(package)_staging_prefix_dir)/lib/; fi

theuni commented at 7:30 pm on June 16, 2020:

This can be deleted now too, we know where to expect the c++ libs now.

Sjors commented at 3:29 pm on June 17, 2020: member

Let’s merge this first. As long as the other stuff gets in before 0.21 most people won’t be impacted anyway.

dongcarl commented at 5:33 pm on June 17, 2020: member

Thought a bit about what’s needed for the steps that are required before 0.21, and it seems like I’ll have to redo the entire gen-sdk.sh in python for the tarfile and plistlib modules, so it probably doesn’t make sense to merge this in the meantime. Will push up a new version in this PR when I have it.

Many thanks to laanwj, theuni, fanquake, and especially Sjors for reviewing.

dongcarl marked this as a draft on Jun 17, 2020

theuni commented at 5:38 pm on June 17, 2020: member

Let’s merge this first.

Sounds good.

My only remaining ask for this PR is something @dongcarl and I discussed yesterday: Adding the xcode version string to the (already lengthy!) sdk tarball/dirname. This is to compensate for the fact that Apple updates the SDK in new XCode versions, so unfortunately “MacOSX10.xy.sdk” alone doesn’t mean much. Adding this string will prevent future clashes on our end.

Edit: Whoops, look like Carl and I commented here at the same time. Sorry for throwing another wrench in this :(

dongcarl force-pushed on Jun 18, 2020

theuni commented at 1:12 am on June 19, 2020: member

@dongcarl Whoa, that was quick!

I haven’t really reviewed the new script, but it worked great. Built locally, looks good. I went ahead and uploaded the resulting tarball for Travis so we can see where we stand.

Now that this includes a toolchain bump, mind updating the PR title/description? I think this is ready for in-depth review now.

in contrib/macdeploy/README.md:22 in 538bb0bbad outdated

19+### Step 1: Obtaining `Xcode.app`
20+
21+Our current macOS SDK
22+(`Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers.tar.gz`) can be
23+extracted from
24+[Xcode_11.3.1.xip](https://download.developer.apple.com/Developer_Tools/Xcode_11.3.1/Xcode_11.3.1.xip).

Sjors commented at 8:03 am on June 19, 2020:

The URL is correct, but in my experience it never works directly, not even if you’re logged in. I always need to manually navigate to https://developer.apple.com/download/more/ search for the correct Xcode version and then download. Also, maybe use 11.5?

fanquake commented at 9:30 am on June 19, 2020:

Also, maybe use 11.5?

NACK. 11.3.1 was picked specifically. It’s what is referenced throughout the docs, build system and in the tarball name. There’s also no guarantee that you’ll generate the same tarball between various versions of Xcode.

Sjors commented at 1:36 pm on June 19, 2020:

What do you mean by “picked specifically”? It’s introduced in this PR. The only other place I see it is on the Travis macOS native machine, but the choice there doesn’t matter.

theuni commented at 1:41 pm on June 19, 2020:

Sorry for not explaining in more detail in the commit message.

11.3.1 was chosen as the last Xcode version which uses (Apple’s version of) clang 8.x.x, for which we have a matching cctools and ld64. Starting with 11.4, they bumped to clang 9.x.x. Once cctools-port has a working matching version for clang 9, we can bump to a newer Xcode version.

Edit: This matters because the sdk/libc++ headers are updated in lockstep with clang in order to take advantage of new features (intrinsics, function attributes, etc), and because a newer version of cctools/ld64 is often useful/required for newer clang (think lto).

dongcarl commented at 2:00 pm on June 19, 2020:

A thought: to prevent future confusion, we should follow up this PR with documentation on the reasoning behind how our macOS toolchain is constructed.

Sjors commented at 2:50 pm on June 19, 2020:

More info would be a good followup. I’m surprised Apple would bump clang 8 to 9 in a minor Xcode version update. But actually that could make sense, because they want to give developers access to SDK’s for their next generation(s) OS, without forcing devs to upgrade their production machine to a beta/recent macOS (which Xcode versions tend to track). According to wikipedia Xcode 11.3.1 requires macOs 10.14.4, whereas Xcode 11.4 bumps that requirement to macOS 10.15.2. The specific choice of clang 8.x.x makes sense.

in contrib/macdeploy/README.md:32 in 538bb0bbad outdated

32+approach (tested on Debian Buster) is outlined below:
33 
34 ```bash
35+# Install/clone tools needed for extracting Xcode.app
36+apt install cpio
37+git clone https://github.com/bitcoin-core/apple-sdk-tools.git

Sjors commented at 8:16 am on June 19, 2020:

tested on macOS with 70de9bedd37b388cc5b486d4a5785c33896c262d

in contrib/macdeploy/README.md:36 in 538bb0bbad outdated

55-./pbzx/pbzx -n Content | cpio -i
56-
57-find Xcode.app -type d -name MacOSX.sdk -exec sh -c 'tar --transform="s/MacOSX.sdk/MacOSX10.14.sdk/" -c -C$(dirname {}) MacOSX.sdk/ | gzip -9n > MacOSX10.14.sdk.tar.gz' \;
58+# Unpack Xcode_11.3.1.xip and place the resulting Xcode.app in your current
59+# working directory
60+python3 apple-sdk-tools/extract_xcode.py -f Xcode_11.3.1.xip | cpio -d -i

Sjors commented at 8:25 am on June 19, 2020:

shasum, generated as before: 707b24ff63bf31e403ff45e97cbbf7d934c738f3f2aae85a37ecdcd896ca5dbb shasum_list_sorted.txt

in contrib/macdeploy/gen-sdk:31 in 538bb0bbad outdated

26+    parser.add_argument("-o", metavar='OUTSDKTGZ', nargs=1, dest='out_sdktgz', required=False)
27+
28+    args = parser.parse_args()
29+
30+    xcode_app = pathlib.Path(args.xcode_app[0]).resolve()
31+    assert xcode_app.is_dir(), f"The supplied Xcode.app path '{xcode_app}' either does not exist or is not a directory"

Sjors commented at 9:04 am on June 19, 2020:

This is not Python 3.5.6 syntax (try with pyenv)

dongcarl commented at 4:05 pm on June 19, 2020:

Shame :frowning_face:, I like f-strings! Will redo with .format.

dongcarl commented at 4:19 pm on June 19, 2020:

Done and tested with Python 3.5.9!

Sjors approved

Sjors commented at 9:26 am on June 19, 2020: member

tACK 538bb0b (some minor issues for followup or if you need to touch things)

Gitian build:

0019e21e451bdba8b803431ddf3be2bc8b2213da02d0163a23b2eda77a6ecb25e  bitcoin-538bb0bbad8d-osx-unsigned.dmg
1fc5ea07e47b37adc24513225edc93ddc6a8f6867ae0f24049a560e5a2dd0883d  bitcoin-538bb0bbad8d-osx-unsigned.tar.gz
2714bc66d4d7f0131dedd50b5cdfb6da4f1161da91bc2bbeda19f81787ff5756d  bitcoin-538bb0bbad8d-osx64.tar.gz
34958ef9e024dcf41044c0297b278e04b54e422710c6ad813c9bb5b162bd25148  src/bitcoin-538bb0bbad8d.tar.gz
44f5d9ba4ad9e0828cdb7e0c85587d51faf7850be5b8fc68d1b00fe3ccdf7efb3  bitcoin-core-osx-0.21-res.yml

Lightly tested DMG on macOS 10.15.5

dongcarl force-pushed on Jun 19, 2020

dongcarl renamed this:
~~Simplify macOS SDK generation and include `libc++` headers~~
build: macOS toolchain simplification and bump
on Jun 19, 2020

dongcarl marked this as ready for review on Jun 19, 2020

in contrib/macdeploy/gen-sdk:48 in 4a7932285c outdated

43+        pl = plistlib.load(fp)
44+        sdk_version = pl['ProductVersion']
45+        sdk_build_id = pl['ProductBuildVersion']
46+    print("Found MacOSX SDK (version: {version}, build id: {build_id})".format(version=sdk_version, build_id=sdk_build_id))
47+
48+    out_name = "Xcode-{version}-{build_id}-extracted-SDK-with-libcxx-headers".format(version=sdk_version, build_id=sdk_build_id)

Sjors commented at 6:00 pm on June 19, 2020:

This should use {xcode_version}, not {version}; it broke when you changed the formatting.

dongcarl commented at 6:36 pm on June 19, 2020:

Good catch! Fixed.

Sjors changes_requested

Sjors commented at 6:01 pm on June 19, 2020: member

4a7932285ca7fc9a8e91af58567106b2ff04519d is almost there…

contrib: macdeploy: Correctly generate macOS SDK

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.

b3394ab235

dongcarl force-pushed on Jun 19, 2020

Sjors commented at 7:42 pm on June 19, 2020: member

re-tACK cbdaa3a179c5011b651cb4c9cbcef482816d822d

Gitian build changed, presumable because you rebased (from a79bca2f1fb25f433d6e100a31a3acfde2656ce1 to febe5823b4ae0de5cd0e6da3f69555acd0724267)?

02f13a3069c1608f0e3c5ab0fa561e22273165acf710c8cc2fe399790b4fd56de  bitcoin-cbdaa3a179c5-osx-unsigned.dmg
1fb00a5c1306c33fc54e3aafde51cdc12ab890785eda253717b3924529a6d2366  bitcoin-cbdaa3a179c5-osx-unsigned.tar.gz
24ea413de539ee7e972a5f4d60108016729d7cb92e6d15d5a2747b083fae87140  bitcoin-cbdaa3a179c5-osx64.tar.gz
31173147fd75c59bd44724b489280ea7895b61946a3c91f410d98b5770aa6250a  src/bitcoin-cbdaa3a179c5.tar.gz
4dae1d7de69df0f8deb72a0138c74fcb32a4d70a4358e99b78ae2fa362834e4bb  bitcoin-core-osx-0.21-res.yml

dongcarl commented at 2:50 am on June 20, 2020: member

I can reproduce Sjor’s results!

0- out_manifest: |
1    2f13a3069c1608f0e3c5ab0fa561e22273165acf710c8cc2fe399790b4fd56de  bitcoin-cbdaa3a179c5-osx-unsigned.dmg
2    fb00a5c1306c33fc54e3aafde51cdc12ab890785eda253717b3924529a6d2366  bitcoin-cbdaa3a179c5-osx-unsigned.tar.gz
3    4ea413de539ee7e972a5f4d60108016729d7cb92e6d15d5a2747b083fae87140  bitcoin-cbdaa3a179c5-osx64.tar.gz
4    1173147fd75c59bd44724b489280ea7895b61946a3c91f410d98b5770aa6250a  src/bitcoin-cbdaa3a179c5.tar.gz

MarcoFalke commented at 10:21 am on June 20, 2020: member

Let me know when this is ready so I can put the backdoored binary on DrahtBot :)

MarcoFalke commented at 10:52 am on June 20, 2020: member

Before merge, the OP should be edited to remove the historic description, which is accessible through the GitHub history drop-down

MarcoFalke deleted a comment on Jun 20, 2020

MarcoFalke commented at 11:07 am on June 20, 2020: member

the ci failure can be ignored: #19329 (comment)

MarcoFalke commented at 11:08 am on June 20, 2020: member

Concept ACK cbdaa3a179c5011b651cb4c9cbcef482816d822d (I read the OP and don’t disagree)

dongcarl commented at 2:33 pm on June 20, 2020: member

@MarcoFalke theuni said he’ll run thru it once more on Monday, should be pretty safe after that!

in depends/packages/native_cctools.mk:2 in cbdaa3a179 outdated

0@@ -1,14 +1,14 @@
1 package=native_cctools
2-$(package)_version=3764b223c011574971ee3ae09ce968ba5dc2f00f
3+$(package)_version=45fe81aceccb8f8b23f1d0f0ecfd13e29eecae98

fanquake commented at 6:40 am on June 21, 2020:

0$(package)_version=4da2f3b485bcf4cef526f30c0b8c0bcda99cdbb4

dongcarl commented at 2:17 pm on June 22, 2020:

Done!

theuni commented at 7:24 pm on June 22, 2020:

Nice spot, thanks.

fanquake changes_requested

fanquake commented at 6:51 am on June 21, 2020: member

Looks good. Seems to align with what me and @theuni discussed. Tested extract_xcode.py. However, I noticed while building depends that the version of LD64 being passed around didn’t look correct -DLD64_VERSION_NUM=512.4. It should be 530. Looks like upstream didn’t update it when doing the last LD64 update, however they have noticed and fixed it, so we should move to cctools @ https://github.com/tpoechtrager/cctools-port/commit/4da2f3b485bcf4cef526f30c0b8c0bcda99cdbb4, which is in the 949.0.1-ld64-530 branch.

Adapt rest of tooling to new SDK naming scheme 3381e4a189

native_cctools: Don't use libc++ from pinned clang

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.

fbcfcf6954

contrib: macdeploy: Use apple-sdk-tools instead of xar+pbzx 351beb5c9a

contrib: macdeploy: Remove historical extraction notes 85b5e42088

depends: bump MacOS toolchain

clang   6.0.1  -> 8.0.0
cctools 921    -> 949.0.1
ld64    409.12 -> 530

5c2c835433

macos: Bump to xcode 11.3.1 and 10.15 SDK

This gets us a newer SDK with c++17 support and retains 10.12
back-compat.

Co-authored-by: Carl Dong <contact@carldong.me>

2418f739f7

dongcarl force-pushed on Jun 22, 2020

dongcarl commented at 2:19 pm on June 22, 2020: member

2418f739f75824d6689369f326b960cec254cf56 includes fanquake’s recommendation here, and a modification that makes the history more readable (some leftover mistake from my squashing).

Should be ready to merge once theuni takes a last look today.

theuni commented at 8:53 pm on June 22, 2020: member

Actually, after digging through @fanquake’s recommendation some more, it looks like the LD64_VERSION_NUM does not end up being passed through correctly. We can poke at clang’s source to see what it does with that value. One easy-to-demonstrate case can be seen here

0  // Newer linkers support -demangle. Pass it if supported and not disabled by
1  // the user.
2  if (Version[0] >= 100 && !Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
3    CmdArgs.push_back("-demangle");

Linking a test with our depends-generated toolchain:

/home/cory/dev/bitcoin2/depends/x86_64-apple-darwin16/native/bin/clang++ -target x86_64-apple-darwin16 -mmacosx-version-min=10.12 –sysroot /home/cory/dev/bitcoin2/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers -stdlib=libc++ -std=c++11 test.cpp -o testing -v

Shows the link-line:

“/home/cory/dev/bitcoin2/depends/x86_64-apple-darwin16/native/bin/x86_64-apple-darwin16-ld” -dynamic -arch x86_64 -macosx_version_min 10.12.0 -syslibroot /home/cory/dev/bitcoin2/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers -o testing /tmp/test-385d92.o -lc++ -lSystem

However, a test with -mlinker-version=100:

/home/cory/dev/bitcoin2/depends/x86_64-apple-darwin16/native/bin/clang++ -target x86_64-apple-darwin16 -mmacosx-version-min=10.12 –sysroot /home/cory/dev/bitcoin2/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers -stdlib=libc++ -std=c++11 test.cpp -o testing -v -mlinker-version=100

Yields:

“/home/cory/dev/bitcoin2/depends/x86_64-apple-darwin16/native/bin/x86_64-apple-darwin16-ld” -demangle -dynamic -arch x86_64 -macosx_version_min 10.12.0 -syslibroot /home/cory/dev/bitcoin2/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers -o testing /tmp/test-a0732e.o -lc++ -lSystem

Note that “-demangle” was added as-expected only when -mlinker-version was passed. Without that option, it seems to assume the lowest possible value. So I guess this is a regression from all the way back in ca5055a5aa07aba81a87cf12f6f0526a63c423b5 :(

I suggest we take https://github.com/theuni/bitcoin/commit/718180005fb031713445498d89cc0565e5a6f6ec as part of this PR as well. Then maybe we can send @fanquake down the rabbit hole to see why clang can’t find this on its own (or if it’s even supposed to).

darwin: pass mlinker-version so that clang enables new features

Without this clang fails to add any newly-added linker features.

Removing this in ca5055a5aa07aba81a87cf12f6f0526a63c423b5 was likely a
regression.

See https://github.com/bitcoin/bitcoin/pull/19240#issuecomment-647764049
for more discussion.

adf543d714

dongcarl commented at 9:04 pm on June 22, 2020: member

Good find! cherry-picked 718180005fb031713445498d89cc0565e5a6f6ec into this PR so that we only nuke depends cache once. @theuni anything else you want to address?

theuni approved

theuni commented at 9:06 pm on June 22, 2020: member

ACK adf543d7144a44f8cd28a090a21c4e2a606862da.

Thanks, everyone for sticking with this. The scope ended up expanding a few times, but I’m really happy to get this all in at once.

Big thanks to @Sjors as well for testing along the way.

dongcarl commented at 10:27 pm on June 22, 2020: member

Gitian build succeeded locally for me:

0- out_manifest: |
1    5eb9795549bc1f3a91cfa4b8901db357343d6386a68c15298009546ed28f3f74  bitcoin-adf543d7144a-osx-unsigned.dmg
2    8f03081ef08035ee91c5fa10e1ddb86842cb318dd62ea923e7346c6f4306cd96  bitcoin-adf543d7144a-osx-unsigned.tar.gz
3    f065ab8d37a473c180bb92f7613de03ba3a1b5f604f0aaa2cd4dd8d9953a9e7f  bitcoin-adf543d7144a-osx64.tar.gz
4    ffc2e29782439497a2a27de16996d6b3de5225feb06e6f63ece3680557470e37  src/bitcoin-adf543d7144a.tar.gz

fanquake approved

fanquake commented at 7:50 am on June 23, 2020: member

ACK adf543d7144a44f8cd28a090a21c4e2a606862da - I’ll take a look at the linker issue.

fanquake merged this on Jun 23, 2020

fanquake closed this on Jun 23, 2020

Sjors commented at 9:18 am on June 23, 2020: member

Post merge: I get the same gitian result as @dongcarl found.

MarcoFalke commented at 7:39 pm on June 24, 2020: member

Is the Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers.tar.gz deterministic and where can I get it without an apple account?

MarcoFalke commented at 7:40 pm on June 24, 2020: member

@jonasschnelli https://bitcoin.jonasschnelli.ch/gitian/#nighly also needs the new tar.gz

theuni commented at 7:42 pm on June 24, 2020: member

Is the Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers.tar.gz deterministic and where can I get it without an apple account?

The .tar.gz is not deterministic for the usual tarball reasons, but its contents are.

I’m afraid you’ll need an apple account to grab Xcode.

Sjors commented at 4:29 pm on June 25, 2020: member

@MarcoFalke you can compare your results with mine: #19240 (review)

stackman27 referenced this in commit cdedd070c7 on Jun 26, 2020

MarcoFalke commented at 12:58 pm on June 27, 2020: member

Could someone please upload a mac build.log for me to compare? I get a mismatch

sidhujag referenced this in commit d0a2ef8275 on Jul 7, 2020

str4d referenced this in commit 6b785ae72d on Jul 30, 2020

zkbot referenced this in commit 2a39656e6d on Jul 30, 2020

daira referenced this in commit 30b2f46b36 on Aug 1, 2020

zkbot referenced this in commit e0692ed4df on Aug 7, 2020

jasonbcox referenced this in commit a8598eced5 on Oct 2, 2020

deadalnix referenced this in commit e329e87ce5 on Oct 3, 2020

janus referenced this in commit 5dc1ac626b on Nov 22, 2020

Fuzzbawls referenced this in commit 284483113d on Mar 26, 2021

Fuzzbawls referenced this in commit 5c6343ba43 on Mar 26, 2021

Fuzzbawls referenced this in commit 5cc0d0fb38 on Apr 5, 2021

furszy referenced this in commit 816f42d7ac on May 25, 2021

kittywhiskers referenced this in commit 0e3444a22f on Jul 15, 2021

kittywhiskers referenced this in commit 1b285aef19 on Jul 15, 2021

kittywhiskers referenced this in commit 392f3df638 on Jul 20, 2021

kittywhiskers referenced this in commit fe6dfa8ab6 on Jul 20, 2021

kittywhiskers referenced this in commit 2dfea813f5 on Jul 20, 2021

kittywhiskers referenced this in commit 992c0cd3fe on Jul 20, 2021

kittywhiskers referenced this in commit f90c39ccd1 on Aug 1, 2021

kittywhiskers referenced this in commit 67fae689da on Aug 24, 2021

kittywhiskers referenced this in commit 53c6f5ca5d on Aug 24, 2021

kittywhiskers referenced this in commit c4c99b022b on Aug 25, 2021

kittywhiskers referenced this in commit f67892c18b on Aug 25, 2021

kittywhiskers referenced this in commit 45629c0460 on Aug 26, 2021

kittywhiskers referenced this in commit 0b3d8a0684 on Aug 26, 2021

kittywhiskers referenced this in commit b4db7bdc81 on Aug 26, 2021

kittywhiskers referenced this in commit f9dd30b478 on Aug 27, 2021

kittywhiskers referenced this in commit 288c292627 on Aug 30, 2021

kittywhiskers referenced this in commit 9342439d44 on Sep 1, 2021

kittywhiskers referenced this in commit c24ce259da on Sep 1, 2021

kittywhiskers referenced this in commit 06b2d4b0f7 on Sep 1, 2021

kittywhiskers referenced this in commit 48ea33a9f7 on Sep 2, 2021

kittywhiskers referenced this in commit decd23c0d6 on Sep 3, 2021

kittywhiskers referenced this in commit 6349b7e401 on Sep 3, 2021

lyricidal referenced this in commit 66c1bcce0c on Oct 29, 2021

lyricidal referenced this in commit 4bccdff11f on Oct 31, 2021

DrahtBot locked this on Feb 15, 2022

build: macOS toolchain simplification and bump #19240

But the other Wikipedia chart says that the “min macOS to run” for Xcode 11.3.1 is 10.14.4, doesn’t that violate constraint (ii)?

Why bundle all of this together in a single PR?

Conflicts