deploy: remove some tools when cross-compiling for macOS

fanquake commented at 10:59 am on April 16, 2024: member

Neither of these tools are actually used when we are cross-compiling for macOS. They are used when we have to adjust non-static libs during a deploy after building on a macOS machine. Simplies #29739 (will be rebased on top).

Guix (aarch64):

08f29bce75d7f574306a0e38d793e0e4e145b547a4b9e5a755a54976121d8ac41  guix-build-5afd3c302051/output/arm64-apple-darwin/SHA256SUMS.part
19ba01fe46be715adcbe80f39dc7dbe449f32ca9d9b660da698f933aef3e6d80b  guix-build-5afd3c302051/output/arm64-apple-darwin/bitcoin-5afd3c302051-arm64-apple-darwin-unsigned.tar.gz
237719437e951449341d0e10dcc4afe93e955d59de5312ce6351e1fa01b4927ac  guix-build-5afd3c302051/output/arm64-apple-darwin/bitcoin-5afd3c302051-arm64-apple-darwin-unsigned.zip
306a79fc871dcd4290f5f7e7e9de19a5a535203d20279f4555d1c319d07abe2d0  guix-build-5afd3c302051/output/arm64-apple-darwin/bitcoin-5afd3c302051-arm64-apple-darwin.tar.gz
498d2b8b37197dcad36a04eb2f3ff2130b859220a17b83a4186a78dcf0af4eafd  guix-build-5afd3c302051/output/dist-archive/bitcoin-5afd3c302051.tar.gz
5df63ff44ef41565ff13ce6dde5485173a18d5866ebc316df86f9ebd91fda18f5  guix-build-5afd3c302051/output/x86_64-apple-darwin/SHA256SUMS.part
628362ce9e80d5e78db198efa5f89434fbe76ca91df5fde7455da4d50ceb8523a  guix-build-5afd3c302051/output/x86_64-apple-darwin/bitcoin-5afd3c302051-x86_64-apple-darwin-unsigned.tar.gz
7534745b679eb9e8e408dd251a6bf0829e62e12f7a41772b8a57a044ded14208c  guix-build-5afd3c302051/output/x86_64-apple-darwin/bitcoin-5afd3c302051-x86_64-apple-darwin-unsigned.zip
8f53d0c9a1bb83d548c7d274c7d39653a3989fb1b4efec49e73dd1cac7c92074c  guix-build-5afd3c302051/output/x86_64-apple-darwin/bitcoin-5afd3c302051-x86_64-apple-darwin.tar.gz

DrahtBot commented at 10:59 am on April 16, 2024: contributor

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	TheCharlatan

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#29739 (depends: swap some cctools tools for LLVM tools by fanquake)
#29450 (build: replace custom MAC_OSX macro with standard __APPLE__ for consistent macOS detection by paplorinc)

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.

fanquake added the label DrahtBot Guix build requested on Apr 16, 2024

in Makefile.am:129 in 5afd3c3020 outdated

125@@ -126,7 +126,7 @@ $(OSX_ZIP): deploydir
126 	cd $(APP_DIST_DIR) && find . | sort | $(ZIP) -X@ $@
127 
128 $(APP_DIST_DIR)/$(OSX_APP)/Contents/MacOS/Bitcoin-Qt: $(OSX_APP_BUILT) $(OSX_PACKAGING)
129-	INSTALL_NAME_TOOL=$(INSTALL_NAME_TOOL) OTOOL=$(OTOOL) STRIP=$(STRIP) $(PYTHON) $(OSX_DEPLOY_SCRIPT) $(OSX_APP) $(OSX_VOLNAME) -translations-dir=$(QT_TRANSLATION_DIR)
130+	OTOOL=$(OTOOL) $(PYTHON) $(OSX_DEPLOY_SCRIPT) $(OSX_APP) $(OSX_VOLNAME) -translations-dir=$(QT_TRANSLATION_DIR)

hebasto commented at 11:09 am on April 16, 2024:

https://github.com/bitcoin/bitcoin/blob/c7567d9223a927a88173ff04eeb4f54a5c02b43d/contrib/macdeploy/macdeployqtplus#L212

hebasto commented at 11:10 am on April 16, 2024:

https://github.com/bitcoin/bitcoin/blob/c7567d9223a927a88173ff04eeb4f54a5c02b43d/contrib/macdeploy/macdeployqtplus#L231

fanquake commented at 11:11 am on April 16, 2024:

Did you read the description? Neither of these code paths are hit when cross compiling.

hebasto commented at 11:16 am on April 16, 2024:

Did you read the description?

I did.

Neither of these code paths are hit when cross compiling.

It is not obvious from reading the macdeployqtplus code. Maybe add a few comments to make it clear?

DrahtBot commented at 9:51 pm on April 16, 2024: contributor

Guix builds (on x86_64)

File	commit c7567d9223a927a88173ff04eeb4f54a5c02b43d(master)	commit efdc4420660bb0b3f2b0fe6518c9154c2aead023(master and this pull)
SHA256SUMS.part	`0d0fc9910c71801e...`	`d77ae186fd3020ba...`
*-aarch64-linux-gnu-debug.tar.gz	`f8af7013dcae9a1c...`	`dfa447d84f245661...`
*-aarch64-linux-gnu.tar.gz	`ddeb7ce74fdd2329...`	`f6aa8560aaa25ec6...`
*-arm-linux-gnueabihf-debug.tar.gz	`99f5761d802f7da6...`	`63ccceae45b60e74...`
*-arm-linux-gnueabihf.tar.gz	`0f4b3e515f238cd7...`	`cf995da64a1bf17e...`
*-arm64-apple-darwin-unsigned.tar.gz	`c5ef39c6ad8a8281...`	`9b1462f84dc9b665...`
*-arm64-apple-darwin-unsigned.zip	`5356b31cbbd15b64...`	`a9492d66a37d8952...`
*-arm64-apple-darwin.tar.gz	`5b16c5013a4843ca...`	`df7acdd9e4f7401e...`
*-powerpc64-linux-gnu-debug.tar.gz	`c81b1f327aff5535...`	`3b6bcba20c39ed3e...`
*-powerpc64-linux-gnu.tar.gz	`c9d03e5d063377d3...`	`3f15a2e8a0701953...`
*-riscv64-linux-gnu-debug.tar.gz	`9aa7cf7fe6a34a2e...`	`f5b61822ade1fbb1...`
*-riscv64-linux-gnu.tar.gz	`1483a98592c23b57...`	`48f80d76ad1bf8d2...`
*-x86_64-apple-darwin-unsigned.tar.gz	`7bdfa268f4002a33...`	`739956f0ca7262a6...`
*-x86_64-apple-darwin-unsigned.zip	`aa9e70d6d9a79650...`	`cdea52c43b62e6c3...`
*-x86_64-apple-darwin.tar.gz	`af5be2b0f4df0085...`	`304203da0939431a...`
*-x86_64-linux-gnu-debug.tar.gz	`44e3cb211881f7e4...`	`b15a7b3e279a8d93...`
*-x86_64-linux-gnu.tar.gz	`01dbf3d65f727130...`	`d246d8f51593c097...`
*.tar.gz	`f05a0240184f515d...`	`1bc3bff268e37c47...`
guix_build.log	`462fee383aac06ad...`	`590c7ef60ae96d96...`
guix_build.log.diff		`9811e906d3810302...`

DrahtBot removed the label DrahtBot Guix build requested on Apr 16, 2024

build: don't pass strip to macOS deploy if cross-compiling

This could only be called in code paths that cannot be hit.

78b6b5c485

build: don't use install_name_tool for macOS deploy when cross-compiling

This is only needed when compiling on macOS. This means we can also
better scope the usage of `-headerpad_max_install_names`.

3bee51427a

build: better scope usage of -Wl,-headerpad_max_install_names

If we aren't using install_name_tool when cross-compiling, we don't need
to test for / add it to LDFLAGS when that is the case.

1a9aa8d4ee

fanquake force-pushed on Apr 17, 2024

fanquake commented at 10:41 am on April 17, 2024: member

Added a commit to decrease the scope of using -Wl,-headerpad_max_install_names.

in Makefile.am:129 in 78b6b5c485 outdated

125@@ -126,7 +126,7 @@ $(OSX_ZIP): deploydir
126 	cd $(APP_DIST_DIR) && find . | sort | $(ZIP) -X@ $@
127 
128 $(APP_DIST_DIR)/$(OSX_APP)/Contents/MacOS/Bitcoin-Qt: $(OSX_APP_BUILT) $(OSX_PACKAGING)
129-	INSTALL_NAME_TOOL=$(INSTALL_NAME_TOOL) OTOOL=$(OTOOL) STRIP=$(STRIP) $(PYTHON) $(OSX_DEPLOY_SCRIPT) $(OSX_APP) $(OSX_VOLNAME) -translations-dir=$(QT_TRANSLATION_DIR)
130+	INSTALL_NAME_TOOL=$(INSTALL_NAME_TOOL) OTOOL=$(OTOOL) $(PYTHON) $(OSX_DEPLOY_SCRIPT) $(OSX_APP) $(OSX_VOLNAME) -translations-dir=$(QT_TRANSLATION_DIR)

Sjors commented at 1:15 pm on April 17, 2024:

78b6b5c485191b85ae201df9d5ef0bcdaaa9c190: note for other reviewers

BUILD_OS is set to darwin when either (see configure.ac):

we’re not cross-compiling and TARGET_OS=darwin; or
we are cross-compiling and $build_os=darwin

BUILD_DARWIN is set when [test "$BUILD_OS" = "darwin"]. So the !BUILD_DARWIN code path here is taken when we are cross compiling (from a non-darwin system).

After this commit $STRIP is only used for Windows stuff.

It’s not clear to me what $STRIP is/was doing. So I’ll just check if this PR doesn’t break anything for me.

Sjors commented at 1:28 pm on April 17, 2024: member

Just tested that both the guix build and local make deploy still work on Intel macOS 14.4.1.

0751ede1b4f680d44c97a9aab396e0a485e3a47c88ecc30ec8b83e53784fc3f50  guix-build-1a9aa8d4eedf/output/arm64-apple-darwin/SHA256SUMS.part
1871cf387d5d60efc0ec9e50f975a9f44b2e2f9b7d92d1f2744affc3c8a5e1655  guix-build-1a9aa8d4eedf/output/arm64-apple-darwin/bitcoin-1a9aa8d4eedf-arm64-apple-darwin-unsigned.tar.gz
26a8de4ac9647549d146a53a9167b00f72ac09168939284b76fa9b6cf81595fea  guix-build-1a9aa8d4eedf/output/arm64-apple-darwin/bitcoin-1a9aa8d4eedf-arm64-apple-darwin-unsigned.zip
36a00443340b2b8d13ed86a5d3947845ec2da20cfb0f0f80cce1434a0a4581557  guix-build-1a9aa8d4eedf/output/arm64-apple-darwin/bitcoin-1a9aa8d4eedf-arm64-apple-darwin.tar.gz
4ec0320586c07bcc6b80505ed6b3de6307fd99d43b505919bb4f95ff27f1d8991  guix-build-1a9aa8d4eedf/output/dist-archive/bitcoin-1a9aa8d4eedf.tar.gz
514a0907bf9c91a2ca19b5a9b53071159be36d82d69c287ffd6a621695f4a637d  guix-build-1a9aa8d4eedf/output/x86_64-apple-darwin/SHA256SUMS.part
6862b59e90473c37d4f95d083fccf12416ddc92f9177a2ffb71180a11b3432d9b  guix-build-1a9aa8d4eedf/output/x86_64-apple-darwin/bitcoin-1a9aa8d4eedf-x86_64-apple-darwin-unsigned.tar.gz
7ddcb16882dde50880f9dffc6ac7ab5ac01b220bbedad4c16e9ff15dcf3b30cef  guix-build-1a9aa8d4eedf/output/x86_64-apple-darwin/bitcoin-1a9aa8d4eedf-x86_64-apple-darwin-unsigned.zip
8720a0689c916e878a5ff050b6183e993af751950b906593d4cbaf4b22a22466c  guix-build-1a9aa8d4eedf/output/x86_64-apple-darwin/bitcoin-1a9aa8d4eedf-x86_64-apple-darwin.tar.gz

bitcoin deleted a comment on Apr 17, 2024

TheCharlatan approved

TheCharlatan commented at 2:13 pm on April 20, 2024: contributor

ACK 1a9aa8d4eedff3788c792799328ad599132e0da1

Guix builds (x86_64):

0871cf387d5d60efc0ec9e50f975a9f44b2e2f9b7d92d1f2744affc3c8a5e1655  guix-build-1a9aa8d4eedf/output/arm64-apple-darwin/bitcoin-1a9aa8d4eedf-arm64-apple-darwin-unsigned.tar.gz
16a8de4ac9647549d146a53a9167b00f72ac09168939284b76fa9b6cf81595fea  guix-build-1a9aa8d4eedf/output/arm64-apple-darwin/bitcoin-1a9aa8d4eedf-arm64-apple-darwin-unsigned.zip
26a00443340b2b8d13ed86a5d3947845ec2da20cfb0f0f80cce1434a0a4581557  guix-build-1a9aa8d4eedf/output/arm64-apple-darwin/bitcoin-1a9aa8d4eedf-arm64-apple-darwin.tar.gz
3ec0320586c07bcc6b80505ed6b3de6307fd99d43b505919bb4f95ff27f1d8991  guix-build-1a9aa8d4eedf/output/dist-archive/bitcoin-1a9aa8d4eedf.tar.gz
414a0907bf9c91a2ca19b5a9b53071159be36d82d69c287ffd6a621695f4a637d  guix-build-1a9aa8d4eedf/output/x86_64-apple-darwin/SHA256SUMS.part
5862b59e90473c37d4f95d083fccf12416ddc92f9177a2ffb71180a11b3432d9b  guix-build-1a9aa8d4eedf/output/x86_64-apple-darwin/bitcoin-1a9aa8d4eedf-x86_64-apple-darwin-unsigned.tar.gz
6ddcb16882dde50880f9dffc6ac7ab5ac01b220bbedad4c16e9ff15dcf3b30cef  guix-build-1a9aa8d4eedf/output/x86_64-apple-darwin/bitcoin-1a9aa8d4eedf-x86_64-apple-darwin-unsigned.zip
7720a0689c916e878a5ff050b6183e993af751950b906593d4cbaf4b22a22466c  guix-build-1a9aa8d4eedf/output/x86_64-apple-darwin/bitcoin-1a9aa8d4eedf-x86_64-apple-darwin.tar.gz

laanwj added the label macOS on Apr 21, 2024

laanwj added the label Build system on Apr 21, 2024

fanquake added the label Needs CMake port on Apr 24, 2024

fanquake merged this on Apr 25, 2024

fanquake closed this on Apr 25, 2024

fanquake deleted the branch on Apr 25, 2024

hebasto commented at 9:20 pm on April 29, 2024: member

Apparently, CMake expects to find the install_name_tool when cross-compiling for macOS.

From https://github.com/hebasto/bitcoin/actions/runs/8885213906/job/24396062348:

0 -- The C compiler identification is Clang 17.0.6
1CMake Error at /usr/share/cmake-3.25/Modules/CMakeFindBinUtils.cmake:233 (message):
2  Could not find install_name_tool, please check your installation.

hebasto commented at 1:32 pm on April 30, 2024: member

Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/180.

hebasto removed the label Needs CMake port on Apr 30, 2024

deploy: remove some tools when cross-compiling for macOS #29890

Code Coverage

Reviews

Conflicts

Guix builds (on x86_64)