-
Notifications
You must be signed in to change notification settings - Fork 37.7k
build: split libtapi and clang out of native_cctools #21457
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
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
Concept ACK! So much clearer and glad to hear it fixes problems too! |
Concept ACK. |
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.
Approach ACK 477ed59
- 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
-
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 inld-450.3
. -
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 PRnative/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.
Concept ACK, this seems better organized. |
Gitian builds
|
Light Code-Review ACK 477ed59 |
477ed59
to
765e0be
Compare
Rebased and took a couple minor changes.
Why? This would delete the ./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
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.
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] |
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 |
reACK 765e0be |
Guix build:
Gitian build
Keeping testing for now... |
Right. I overlooked that. |
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.
ACK 765e0be, verified building of the native_cctools
package step-by-step.
…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
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
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
This splits our native cctools package into two additional packages:
native_clang
andnative_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:
For Guix (
FORCE_USE_SYSTEM_CLANG=1
):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:
Gitian: