Skip to content

Conversation

fanquake
Copy link
Member

This splits our native cctools package into two additional packages: native_clang and native_libtapi. This is in an effort to not only make our mac toolchain more understandable, but also to reduce duplication, and as a nice side-effect, fix the issue mentioned here.

Everything about the current build process should remain the same. For gitian, that is:

  • Download LLVM Clang 8.0.0 binary.
  • Build libtapi using downloaded Clang.
  • Build cctools using downloaded Clang and libtapi.
  • Build the rest of depends..

For Guix (FORCE_USE_SYSTEM_CLANG=1):

  • Build libtapi using using system Clang.
  • Build cctools using system Clang and libtapi.
  • Build the rest of depends..

After this has been merged, my plan is to combine a modified version of #20454 and #21414 with #19817, and from what I understand that will be enough to support Apple Arm cross compilation.

Builds at 477ed59:
Guix:

find output -type f -name *$(git rev-parse --short HEAD)*.* -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
7e57b9e5a2109d1a35f0091d86f975c1b1d73ac70ac2609cefbe1134efbf2c87  output/bitcoin-477ed59f49f3-osx-unsigned.dmg
dd11e71c2634ac2fa883d1e45cbd6de194fad37624bb56b8b8a6213fd40d6050  output/bitcoin-477ed59f49f3-osx-unsigned.tar.gz
64384eaa2fd9768992d86a06a1414c9e92e84ba21a875696483df2bb5828e2a2  output/bitcoin-477ed59f49f3-osx64.tar.gz
8a889e88db952d2c82ef44713c04aba95b777441f578738ff6d8a0d251e51da3  output/src/bitcoin-477ed59f49f3.tar.gz

Gitian:

d0eee8542d5f3d662555ad7218d2dc9f3f862656e65bcb2f01f256bfa0deead2  bitcoin-477ed59f49f3-osx-unsigned.dmg
ba7bc94897e42e7a037e352c4e4e1730f181c6d76b6d6a2785bbd7bf85614c83  bitcoin-477ed59f49f3-osx-unsigned.tar.gz
c4426d1d310a2fbffcaf2b7df0da4ec97bd11aab07085006dae68777b03f6372  bitcoin-477ed59f49f3-osx64.tar.gz
8a889e88db952d2c82ef44713c04aba95b777441f578738ff6d8a0d251e51da3  src/bitcoin-477ed59f49f3.tar.gz
a746831467dc8ff17ec5df06fc9288a859c1961d8c0b632d97b42f080dbd825d  bitcoin-core-osx-22-res.yml

@DrahtBot
Copy link
Contributor

DrahtBot commented Mar 17, 2021

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

Conflicts

No conflicts as of last run.

@dongcarl
Copy link
Contributor

Concept ACK! So much clearer and glad to hear it fixes problems too!

@hebasto
Copy link
Member

hebasto commented Mar 18, 2021

Concept ACK.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approach ACK 477ed59

  1. Some cleanup suggestions:
--- a/depends/packages/native_clang.mk
+++ b/depends/packages/native_clang.mk
@@ -20,3 +20,8 @@ define $(package)_stage_cmds
   cp lib/libLTO.so $($(package)_staging_prefix_dir)/lib/ && \
   cp -rf lib/clang/$($(package)_version)/include/* $($(package)_staging_prefix_dir)/lib/clang/$($(package)_version)/include/
 endef
+
+define $(package)_postprocess_cmds
+  rm bin/llvm-config && \
+  rmdir include
+endef
--- a/depends/packages/native_cctools.mk
+++ b/depends/packages/native_cctools.mk
@@ -32,3 +32,7 @@ endef
 define $(package)_stage_cmds
   $(MAKE) DESTDIR=$($(package)_staging_dir) install
 endef
+
+define $(package)_postprocess_cmds
+  rm -rf share
+endef
  1. Probably, it is out of this PR scope, but I think we could drop the ld64_disable_threading.patch as it seems that non-determinism was fixed in ld-450.3.

  2. Currently, my only concerns are about the differences in sizes for some files, e.g.:

  • native/bin/x86_64-apple-darwin18-ObjectDump -- 833kB on master, 829kB with PR
  • native/bin/x86_64-apple-darwin18-vtool -- 86kB on master, 82kB with PR

UPDATE: Also a style nit -- I suggest in new code with the make syntax in variable assignments around = and += with spaces.

@DrahtBot
Copy link
Contributor

Guix builds

File commit a9d1b40
(master)
commit 7f2d32e
(master and this pull)
*-aarch64-linux-gnu-debug.tar.gz 0f4694caf39317aa... c92b5176f3321f38...
*-aarch64-linux-gnu.tar.gz 4872f8b786581c1f... 7e475c616776a416...
*-arm-linux-gnueabihf-debug.tar.gz fa08eaff0bb78361... 02122916990778eb...
*-arm-linux-gnueabihf.tar.gz a88bba05c3cab057... 3869a295c4c15e9d...
*-osx-unsigned.dmg d5799e5f579a345d... c960a1a86b929bf2...
*-osx-unsigned.tar.gz c7f14bae7834fffe... 6a3675a7bfd1168b...
*-osx64.tar.gz 40d29fe56dae7891... b18912e1ead26335...
*-powerpc64-linux-gnu-debug.tar.gz ace332f222e916b6... da1dc4c0d0d2a7cf...
*-powerpc64-linux-gnu.tar.gz 6d677e30960990c2... 175e2c908ef48dd4...
*-powerpc64le-linux-gnu-debug.tar.gz f1e306c48cbe2c82... db6d5f0d62068806...
*-powerpc64le-linux-gnu.tar.gz 5da50e25d575b1e3... 739f19efc4d151e2...
*-riscv64-linux-gnu-debug.tar.gz 6fe18ca4217127c8... 5e985765c1002b61...
*-riscv64-linux-gnu.tar.gz ff42caf352e66688... a5ebb9a3d670147e...
*-win-unsigned.tar.gz c0d4021a0d90e6a3... 67372e3cd49933cf...
*-win64-debug.zip 7d09398b71cf5b5b... 29efc4a8920f21e0...
*-win64-setup-unsigned.exe f6c43613283493d8... 51814b9454aaa817...
*-win64.zip 630450b2ccc1cf85... 421534c6db472597...
*-x86_64-linux-gnu-debug.tar.gz 8584c3ae507df23c... bc5a197708eb612e...
*-x86_64-linux-gnu.tar.gz c26e44b319ef3a46... 9a314c710c8a7992...
*.tar.gz 56e411d3291bc111... 619e1328dbfa7701...
guix_build.log 4fd1141f30716e53... 348afa0ebd89e6fb...
guix_build.log.diff e7383c741bc5d9f4...

@laanwj
Copy link
Member

laanwj commented Mar 18, 2021

Concept ACK, this seems better organized.

@DrahtBot
Copy link
Contributor

Gitian builds

File commit 63952f7
(master)
commit cf35e06
(master and this pull)
*-aarch64-linux-gnu-debug.tar.gz 5484237ee5c3fa99... 4b996910a8b0e9e3...
*-aarch64-linux-gnu.tar.gz 2a05550165b38652... a31eca6967cb7b65...
*-arm-linux-gnueabihf-debug.tar.gz bfcf00e42246eae3... 53a96e677e6ab17d...
*-arm-linux-gnueabihf.tar.gz 49401c03ceb458b0... 01c835aa1f641fa0...
*-osx-unsigned.dmg bb46816571411f20... 3384588ef2363c8c...
*-osx64.tar.gz 87297fdb1bd1e468... 03f6bc0cff1b8d49...
*-powerpc64-linux-gnu-debug.tar.gz ef0bc03e9dc3bc9b... 6d19770251be5f0d...
*-powerpc64-linux-gnu.tar.gz 8b994ad2faf8d00f... 3f97305f18e7c352...
*-powerpc64le-linux-gnu-debug.tar.gz a21bb50f2a1c6444... 8f781e172a4657c6...
*-powerpc64le-linux-gnu.tar.gz ac3bbefb3bb763cf... ad739b016529b015...
*-riscv64-linux-gnu-debug.tar.gz d1127ea1c91d13d2... cc0f43a696733b71...
*-riscv64-linux-gnu.tar.gz 2a1d70a226aea599... 8c746d36742daff3...
*-win64-debug.zip 49bb0d62d4d992b1... e6b2bfd80a1216c9...
*-win64-setup-unsigned.exe 2e31a6942609ae70... 0b69d6eff841809d...
*-win64.zip eea2a3d2ae87bd94... 11e79e7c5e2d8812...
*-x86_64-linux-gnu-debug.tar.gz c6ec5e1f93179f02... bb29f9a9dcef789a...
*-x86_64-linux-gnu.tar.gz 9ec8794caea11319... 92509122ffa514ef...
*.tar.gz debbf7a0bedc3e02... 4f965a85e8737e3b...
bitcoin-core-linux-22-res.yml e8cbff85e5387c08... 2e1c7c881f13fa62...
bitcoin-core-osx-22-res.yml d17361a4da1f9904... 42bb10dbe0eacb68...
bitcoin-core-win-22-res.yml fbc466bbb012a14d... b39b2e63906782a2...
linux-build.log bda29ffb8f627075... 39bdcaa5d3641d52...
osx-build.log 4464f7ec164817d1... bf2e34ce151b6add...
win-build.log d947d26c42344b85... 33e4f3232ccc50fc...
bitcoin-core-linux-22-res.yml.diff 79d4deb35923274f...
bitcoin-core-osx-22-res.yml.diff 7cbf24e83a860d8e...
bitcoin-core-win-22-res.yml.diff 811e3a3cb711bd1d...
linux-build.log.diff 7caf82291ba99247...
osx-build.log.diff 259cbbc5c1e71b9d...
win-build.log.diff 2a65db8d1dbc6727...

@dongcarl
Copy link
Contributor

Light Code-Review ACK 477ed59
Tested on Arch Linux both with and without FORCE_USE_SYSTEM_CLANG=1

@fanquake fanquake force-pushed the split_native_cctools branch from 477ed59 to 765e0be Compare March 30, 2021 06:54
@fanquake
Copy link
Member Author

Rebased and took a couple minor changes.

Some cleanup suggestions:
rm bin/llvm-config && \

Why? This would delete the llvm-config we just copied into /bin, and you'd end up with:

./configure: line 13194: /home/ubuntu/bitcoin/depends/x86_64-apple-darwin18/native/bin/llvm-config: No such file or directory
./configure: line 13195: /home/ubuntu/bitcoin/depends/x86_64-apple-darwin18/native/bin/llvm-config: No such file or directory
checking for lto_get_version in -lLTO... no

when configuring native_cctools.

Probably, it is out of this PR scope, but I think we could drop the ld64_disable_threading.patch as it seems that non-determinism was fixed in ld-450.3.

Yes that's out of scope here, and from those links it's not exactly clear to me that all the determinism issues have been fixed. Could be looked at as part of #19817.

Currently, my only concerns are about the differences in sizes for some files, e.g.:

I had a look, and I'm pretty sure that's just due to runpath differences, i.e master:

0x000000000000001d (RUNPATH)            Library runpath: [$ORIGIN/../lib:/home/ubuntu/bitcoin/depends/work/build/x86_64-apple-darwin18/native_cctools/55562e4073dea0fbfd0b20e0bf69ffe6390c7f97-93268e656c0/toolchain/lib:/home/ubuntu/bitcoin/depends/work/build/x86_64-apple-darwin18/native_cctools/55562e4073dea0fbfd0b20e0bf69ffe6390c7f97-93268e656c0/lib:/home/ubuntu/bitcoin/depends/work/build/x86_64-apple-darwin18/native_cctools/55562e4073dea0fbfd0b20e0bf69ffe6390c7f97-93268e656c0/lib64:/home/ubuntu/bitcoin/depends/work/build/x86_64-apple-darwin18/native_cctools/55562e4073dea0fbfd0b20e0bf69ffe6390c7f97-93268e656c0/lib32]

vs this PR:

0x000000000000001d (RUNPATH)            Library runpath: [$ORIGIN/../lib:/home/ubuntu/bitcoin/depends/x86_64-apple-darwin18/native/lib]

@fanquake
Copy link
Member Author

Guix builds:

6de9f9d2b829c10f2f9e8ceb96a14d77af3a5fc0038454809807b5de7477fdfb  output/bitcoin-765e0be5347e-osx-unsigned.dmg
71cdfe7d7dd59c7061832e981380778808c2ba5a1a0f677795cd62262f797b4a  output/bitcoin-765e0be5347e-osx-unsigned.tar.gz
cb596fc85531a9d85ee08c9a6442d953071e50d2e70b4ab96f4be6bea342b766  output/bitcoin-765e0be5347e-osx64.tar.gz
f3d6a45c120a613ccc4256637d65b500fd38ee1588020d406c573677d40d6509  output/src/bitcoin-765e0be5347e.tar.gz

Gitian builds:

aa9d65412d8c0ffccfc31997011e6991b58da537b9dcb6ecf8c6c1da30a47356  bitcoin-765e0be5347e-osx-unsigned.dmg
bca8f720ad30a900ef96235e4c411674e6717e48290923f3a5afbc6542933d12  bitcoin-765e0be5347e-osx-unsigned.tar.gz
2330223c676f18270765b62dcba2564701f12ea109add64a577f3998c9760434  bitcoin-765e0be5347e-osx64.tar.gz
f3d6a45c120a613ccc4256637d65b500fd38ee1588020d406c573677d40d6509  src/bitcoin-765e0be5347e.tar.gz
4ff786f4fad80af69734dd34c7e0ae86c28e3bc7c497f340627a716c0aa209b0  bitcoin-core-osx-22-res.yml

@dongcarl
Copy link
Contributor

reACK 765e0be

@hebasto
Copy link
Member

hebasto commented Mar 30, 2021

Guix build:

$ find output -type f -name *$(git rev-parse --short HEAD)*.* -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
6de9f9d2b829c10f2f9e8ceb96a14d77af3a5fc0038454809807b5de7477fdfb  output/bitcoin-765e0be5347e-osx-unsigned.dmg
71cdfe7d7dd59c7061832e981380778808c2ba5a1a0f677795cd62262f797b4a  output/bitcoin-765e0be5347e-osx-unsigned.tar.gz
cb596fc85531a9d85ee08c9a6442d953071e50d2e70b4ab96f4be6bea342b766  output/bitcoin-765e0be5347e-osx64.tar.gz
f3d6a45c120a613ccc4256637d65b500fd38ee1588020d406c573677d40d6509  output/src/bitcoin-765e0be5347e.tar.gz

Gitian build

Generating report
aa9d65412d8c0ffccfc31997011e6991b58da537b9dcb6ecf8c6c1da30a47356  bitcoin-765e0be5347e-osx-unsigned.dmg
bca8f720ad30a900ef96235e4c411674e6717e48290923f3a5afbc6542933d12  bitcoin-765e0be5347e-osx-unsigned.tar.gz
2330223c676f18270765b62dcba2564701f12ea109add64a577f3998c9760434  bitcoin-765e0be5347e-osx64.tar.gz
f3d6a45c120a613ccc4256637d65b500fd38ee1588020d406c573677d40d6509  src/bitcoin-765e0be5347e.tar.gz
e23a162479e60ccde6a01c1a9c47c05e84c6b62ce381102fba6e61b6aed17777  bitcoin-core-osx-22-res.yml
Done.

Keeping testing for now...

@hebasto
Copy link
Member

hebasto commented Mar 30, 2021

Why? This would delete the llvm-config we just copied into /bin, and you'd end up with:

Right. I overlooked that.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 765e0be, verified building of the native_cctools package step-by-step.

@fanquake fanquake merged commit 1a0d145 into bitcoin:master Mar 31, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Mar 31, 2021
…ools

765e0be build: split native_cctools (fanquake)

Pull request description:

  This splits our native cctools package into two additional packages: `native_clang` and `native_libtapi`. This is in an effort to not only make our mac toolchain more understandable, but also to reduce duplication, and as a nice side-effect, fix the issue mentioned [here](bitcoin#19817 (comment)).

  Everything about the current build process should remain the same. For gitian, that is:
  * Download LLVM Clang 8.0.0 binary.
  * Build libtapi using downloaded Clang.
  * Build cctools using downloaded Clang and libtapi.
  * Build the rest of depends..

  For Guix (`FORCE_USE_SYSTEM_CLANG=1`):
  * Build libtapi using using system Clang.
  * Build cctools using system Clang and libtapi.
  * Build the rest of depends..

  After this has been merged, my plan is to combine a modified version of bitcoin#20454 and bitcoin#21414 with bitcoin#19817, and  from what I  understand that will be enough to support Apple Arm cross compilation.

  Builds at 477ed59:
  Guix:
  ```bash
  find output -type f -name *$(git rev-parse --short HEAD)*.* -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
  7e57b9e5a2109d1a35f0091d86f975c1b1d73ac70ac2609cefbe1134efbf2c87  output/bitcoin-477ed59f49f3-osx-unsigned.dmg
  dd11e71c2634ac2fa883d1e45cbd6de194fad37624bb56b8b8a6213fd40d6050  output/bitcoin-477ed59f49f3-osx-unsigned.tar.gz
  64384eaa2fd9768992d86a06a1414c9e92e84ba21a875696483df2bb5828e2a2  output/bitcoin-477ed59f49f3-osx64.tar.gz
  8a889e88db952d2c82ef44713c04aba95b777441f578738ff6d8a0d251e51da3  output/src/bitcoin-477ed59f49f3.tar.gz
  ```

  Gitian:
  ```bash
  d0eee8542d5f3d662555ad7218d2dc9f3f862656e65bcb2f01f256bfa0deead2  bitcoin-477ed59f49f3-osx-unsigned.dmg
  ba7bc94897e42e7a037e352c4e4e1730f181c6d76b6d6a2785bbd7bf85614c83  bitcoin-477ed59f49f3-osx-unsigned.tar.gz
  c4426d1d310a2fbffcaf2b7df0da4ec97bd11aab07085006dae68777b03f6372  bitcoin-477ed59f49f3-osx64.tar.gz
  8a889e88db952d2c82ef44713c04aba95b777441f578738ff6d8a0d251e51da3  src/bitcoin-477ed59f49f3.tar.gz
  a746831467dc8ff17ec5df06fc9288a859c1961d8c0b632d97b42f080dbd825d  bitcoin-core-osx-22-res.yml
  ```

ACKs for top commit:
  dongcarl:
    reACK 765e0be
  hebasto:
    ACK 765e0be, verified building of the `native_cctools` package step-by-step.

Tree-SHA512: 61cf2b092fb8b9724adda1084e0cac9db889cd5e391914b43592aebc470fae3c1cbabc8b59a0abce6e7bad8c646694fe927f26f4deb18b60d7fd92f374f62feb
kwvg pushed a commit to kwvg/dash that referenced this pull request Aug 24, 2021
kwvg pushed a commit to kwvg/dash that referenced this pull request Aug 24, 2021
kwvg pushed 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 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 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 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
kwvg added a commit to kwvg/dash that referenced this pull request Sep 4, 2021
DeckerSU added a commit to DeckerSU/KomodoOcean that referenced this pull request Nov 16, 2021
update build system for darwin cross-compile to match bitcoin upstream:

- bitcoin/bitcoin#21457 - split libtapi and clang out of native_cctools
- bitcoin/bitcoin#19817 - macOS toolchain bump
TheComputerGenie pushed a commit to TheComputerGenie/KomodoOcean that referenced this pull request Jan 5, 2022
update build system for darwin cross-compile to match bitcoin upstream:

- bitcoin/bitcoin#21457 - split libtapi and clang out of native_cctools
- bitcoin/bitcoin#19817 - macOS toolchain bump
gades pushed a commit to cosanta/cosanta-core that referenced this pull request May 4, 2022
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
@fanquake fanquake deleted the split_native_cctools branch November 9, 2022 16:40
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