build, macos: Fix `qt` package build with new Xcode 15 linker

hebasto commented at 11:51 am on September 27, 2023: member

Fixes #28541 by backporting an upstream patch.

Guix build:

0x86_64
1b37713bc8a526662eac3d9535924f4a4d2893c58f9c12d3c7599e761e6ff677c  guix-build-79ef528511f0/output/arm64-apple-darwin/SHA256SUMS.part
20befb524181aa10e1635a2616a8bed53f51beafa4f0d495d3bf52a64cbd2d977  guix-build-79ef528511f0/output/arm64-apple-darwin/bitcoin-79ef528511f0-arm64-apple-darwin-unsigned.tar.gz
39cba170f2ffe542c33fdd1ac52b7684dd6301e91d32aa45af7b4ce8769d88d4a  guix-build-79ef528511f0/output/arm64-apple-darwin/bitcoin-79ef528511f0-arm64-apple-darwin-unsigned.zip
404556309266c791ae4d7409359222c88cd7aeb569566f7ef4d29816148a5b7e4  guix-build-79ef528511f0/output/arm64-apple-darwin/bitcoin-79ef528511f0-arm64-apple-darwin.tar.gz
551229df8e104a2ffcd5c5b3f81f7585e1258ef10461d136948ea2a2d690a920d  guix-build-79ef528511f0/output/dist-archive/bitcoin-79ef528511f0.tar.gz
63fe216a05561f2fe7229ddf186ff495b29a5cc31b6f35f407187573d072c5743  guix-build-79ef528511f0/output/x86_64-apple-darwin/SHA256SUMS.part
7961d71104e61a2baf727576eb2da630697bb4f109f66e73be5c96add25378d12  guix-build-79ef528511f0/output/x86_64-apple-darwin/bitcoin-79ef528511f0-x86_64-apple-darwin-unsigned.tar.gz
85598f514d065756ac376e2f3c4f8e758bfba53a43ddef778f106456de1536073  guix-build-79ef528511f0/output/x86_64-apple-darwin/bitcoin-79ef528511f0-x86_64-apple-darwin-unsigned.zip
95360ae1f1b7d96a44a33b2c87708b466e4a7bf3f9de0fc58bccbbcdb21ee254e  guix-build-79ef528511f0/output/x86_64-apple-darwin/bitcoin-79ef528511f0-x86_64-apple-darwin.tar.gz

build, macos: Fix `qt` package build with new Xcode 15 linker 79ef528511

DrahtBot commented at 11:51 am on September 27, 2023: contributor

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

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#28532 (qt: enable -ltcg for windows under LTO by fanquake)
#25391 (guix: Use LTO to build releases by fanquake)
#21778 (build: LLD based macOS toolchain by fanquake)

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.

hebasto added the label macOS on Sep 27, 2023

hebasto added the label Build system on Sep 27, 2023

maflcko added the label DrahtBot Guix build requested on Sep 27, 2023

fanquake added the label Needs backport (25.x) on Sep 27, 2023

DrahtBot commented at 10:57 pm on September 27, 2023: contributor

Guix builds (on x86_64)

File	commit c9f288244b8d183e09a917025922b99e3368ef78(master)	commit 37a4a1033824b4c194d6195ecab6c260f1949b44(master and this pull)
SHA256SUMS.part	`42de1160de03cfb0...`	`4b73e5a3205dbdf6...`
*-aarch64-linux-gnu-debug.tar.gz	`71b1f5c8d9e9bfd4...`	`0aa92abd4334efeb...`
*-aarch64-linux-gnu.tar.gz	`dd653e6ecc0dc1ac...`	`cfa9a0f8a667e7eb...`
*-arm-linux-gnueabihf-debug.tar.gz	`35390cc2d589cdc0...`	`2c8d3ac542624d0a...`
*-arm-linux-gnueabihf.tar.gz	`02a7542f7e313f41...`	`a56d7bc90d4e32a4...`
*-arm64-apple-darwin-unsigned.tar.gz	`308c0e7f1def2cf0...`	`e7365125faa922b0...`
*-arm64-apple-darwin-unsigned.zip	`5a4b00190fa0cdde...`	`43a93557c41c9b33...`
*-arm64-apple-darwin.tar.gz	`63c9b9705f9a954c...`	`53f293aa868d8d0c...`
*-powerpc64-linux-gnu-debug.tar.gz	`8f6b18c067004964...`	`5f48fd6ff87c4d98...`
*-powerpc64-linux-gnu.tar.gz	`acdf08bef308c148...`	`0c01997bd3661a34...`
*-powerpc64le-linux-gnu-debug.tar.gz	`e98727cac73fb47e...`	`ec0701b87f73ce2d...`
*-powerpc64le-linux-gnu.tar.gz	`1071de351bc4c847...`	`9f133b52e52837cf...`
*-riscv64-linux-gnu-debug.tar.gz	`d86c33975d7ab5f0...`	`6410d740f972a9d4...`
*-riscv64-linux-gnu.tar.gz	`61b80a4e2b1e67ae...`	`875a6cd8f6b86eb8...`
*-x86_64-apple-darwin-unsigned.tar.gz	`c5c551d7ddf2c952...`	`f0af85213cb6b929...`
*-x86_64-apple-darwin-unsigned.zip	`ddb1cced452553b0...`	`13a2ac47b35618bb...`
*-x86_64-apple-darwin.tar.gz	`4fb40e3742708fb4...`	`bdc7b3d12defeb7b...`
*-x86_64-linux-gnu-debug.tar.gz	`f0f09c5b962632cd...`	`215ef5e9eea5e4ee...`
*-x86_64-linux-gnu.tar.gz	`504f1fdc02b601df...`	`2293aeacca58e7d8...`
*.tar.gz	`2099c1570f5ec761...`	`bf11fba7b4b9d9fd...`
guix_build.log	`60656ee77aef272c...`	`bb085752c705e787...`
guix_build.log.diff		`e0a2d91ba2b80577...`

DrahtBot removed the label DrahtBot Guix build requested on Sep 27, 2023

maaku commented at 11:54 pm on September 27, 2023: contributor

Confirmed this fixes the issue described in #28541 for me. Thank you!

0xETHengineer approved

fanquake commented at 9:13 am on October 2, 2023: member

Looks like this might still fail, if you aren’t running macOS 14 (maybe aarch64 only)? I see:

0compiling .moc/moc_qfbscreen_p.cpp
1compiling .moc/moc_qfbvthandler_p.cpp
2gmake[4]: Entering directory '/bitcoin/depends/work/build/aarch64-apple-darwin22.6.0/qt/5.15.5-8525c199952/qtbase/src/widgets'
3uic dialogs/qfiledialog.ui
4dyld[55501]: Symbol not found: __ZTVNSt3__13pmr25monotonic_buffer_resourceE
5  Referenced from: <577DB68A-5C1A-38A3-8285-AC3E88B934FE> /bitcoin/depends/work/build/aarch64-apple-darwin22.6.0/qt/5.15.5-8525c199952/qtbase/bin/uic
6  Expected in:     <3EE92404-8FC3-374B-A598-D5C9A8CD64B5> /usr/lib/libc++.1.dylib
7/bin/sh: line 1: 55501 Abort trap: 6           /bitcoin/depends/work/build/aarch64-apple-darwin22.6.0/qt/5.15.5-8525c199952/qtbase/bin/uic dialogs/qfiledialog.ui -o .uic/ui_qfiledialog.h
8compiling kernel/qt_widgets_pch.h
9gmake[4]: *** [Makefile:72588: .uic/ui_qfiledialog.h] Error 134

This seems to be the same as https://forum.qt.io/topic/149842/qt-6-5-2-build-crash-after-update-to-macos14-sdk.

fanquake commented at 9:50 am on October 2, 2023: member

I wonder if a better (short-term) solution here, would be to invoke the deprecated linker using -ld_classic. Although I’m not sure if this will behave with the older ld64.

hebasto commented at 10:44 am on October 2, 2023: member

Looks like this might still fail, if you aren’t running macOS 14 (maybe aarch64 only)? I see:

0compiling .moc/moc_qfbscreen_p.cpp
1compiling .moc/moc_qfbvthandler_p.cpp
2gmake[4]: Entering directory '/bitcoin/depends/work/build/aarch64-apple-darwin22.6.0/qt/5.15.5-8525c199952/qtbase/src/widgets'
3uic dialogs/qfiledialog.ui
4dyld[55501]: Symbol not found: __ZTVNSt3__13pmr25monotonic_buffer_resourceE
5  Referenced from: <577DB68A-5C1A-38A3-8285-AC3E88B934FE> /bitcoin/depends/work/build/aarch64-apple-darwin22.6.0/qt/5.15.5-8525c199952/qtbase/bin/uic
6  Expected in:     <3EE92404-8FC3-374B-A598-D5C9A8CD64B5> /usr/lib/libc++.1.dylib
7/bin/sh: line 1: 55501 Abort trap: 6           /bitcoin/depends/work/build/aarch64-apple-darwin22.6.0/qt/5.15.5-8525c199952/qtbase/bin/uic dialogs/qfiledialog.ui -o .uic/ui_qfiledialog.h
8compiling kernel/qt_widgets_pch.h
9gmake[4]: *** [Makefile:72588: .uic/ui_qfiledialog.h] Error 134

This seems to be the same as forum.qt.io/topic/149842/qt-6-5-2-build-crash-after-update-to-macos14-sdk.

That is another issue different from #28541 which is supposed to be fixed by this PR.

fanquake commented at 12:54 pm on October 2, 2023: member

Two other thoughts:

Is it worth bumping to qt 5.15.10, before applying (more) patches. I haven’t looked if there have been other relevant bug fixes since 5.15.5.
We could also just merge this as-is, as it’s fixing the build in at least some cases, and the case above is already broken on master (with the new tools), and just less broken after the patch. I’m just unsure if this could break other still-working configurations.

hebasto commented at 3:36 pm on October 2, 2023: member

Is it worth bumping to qt 5.15.10, before applying (more) patches. I haven’t looked if there have been other relevant bug fixes since 5.15.5.

When I started to work on this PR, I verified whether an update to 5.15.10 allows us to drop any of our Qt patches. All of them are still required. And we have no reported Qt-related bugs that might be fixed in 5.15.10.

Suggesting to wait for Qt 5.15.16 (for Bitcoin Core v27.0 or v28.0), which will allow us to drop the patch introduced in this PR.

fanquake commented at 3:55 pm on October 2, 2023: member

Suggesting to wait for Qt 5.15.16 (for Bitcoin Core v27.0 or v28.0),

We need to bump ealier than that, otherwise Qt (and it’s lack of proper support for c++20) is going to be a blocker for c++20, which we’ll want to do very soon after branch off.

hebasto commented at 7:08 pm on October 2, 2023: member

When I started to work on this PR, I verified whether an update to 5.15.10 allows us to drop any of our Qt patches. All of them are still required.

I was wrong. Regardless that on https://codereview.qt-project.org/q/I70db7e4c27f0d3da5d0af33cb491d72c312d3fa8 the backport to Qt 5.15 has status “Deferred”, the change from dont_hardcode_x86_64.patch has been backported and available since Qt 5.15.8.

maaku commented at 8:09 pm on October 2, 2023: contributor

Bitcoin Core currently does not build on a platform used by many developers, and you suggest waiting until v27 or v28?

hebasto commented at 9:34 pm on October 2, 2023: member

Bitcoin Core currently does not build on a platform used by many developers, and you suggest waiting until v27 or v28?

If you are referring to a fix of #28541, then this PR is ready for reviewing and merging.

If you are quoting my comment, please consider the full context. It was an answer to “bumping to qt 5.15.10”. However, I’ve changed my opinion and submitted #28561.

maaku commented at 11:05 pm on October 2, 2023: contributor

The context was:

Is it worth bumping to qt 5.15.10, before applying (more) patches.

This PR is “applying (more) patches” so that would have meant waiting for 5.15.10.

This patch is trivial and the impact large enough to warrant merging ASAP.

maflcko commented at 8:11 am on October 3, 2023: member

Suggesting to wait for Qt 5.15.16 (for Bitcoin Core v27.0 or v28.0), which will allow us to drop the patch introduced in this PR.

How would that work? Isn’t 5.15.11 (and later) commercial-only?

fanquake commented at 8:29 am on October 3, 2023: member

then this PR is ready for reviewing and merging

Is it? Isn’t this meant to also fix this issue: #28543 (comment) ? Or is that not being fixed here anymore?

hebasto commented at 8:46 am on October 3, 2023: member

then this PR is ready for reviewing and merging

Is it? Isn’t this meant to also fix this issue: #28543 (comment) ? Or is that not being fixed here anymore?

I apologize for my poor English. By this

That is another issue different from #28541 which is supposed to be fixed by this PR.

I meant that this PR aims to fix the bug described in #28541, not in #28543 (comment).

And I have no macOS 13 at my disposal right now to work on and test the other bugfix.

fanquake commented at 9:19 am on October 3, 2023: member

Ok. Going to merge this now then, as it’s still required, regardless of #28561, and the additional build issues will have to be fixed with different patches.

We should probably also update one of the macOS jobs in the CI to be using the Xcode 15 tools. Maybe we can do that with an M1 machine now: https://github.com/github/roadmap/issues/528.

maflcko commented at 9:21 am on October 3, 2023: member

We should probably also update one of the macOS jobs in the CI to be using the Xcode 15 tools. Maybe we can do that with an M1 machine now: github/roadmap#528.

How would that work, given that the release is commercial-only? “With today’s launch, our macOS larger runners will be priced at $0.16/minute for XL and $0.12/minute for large.”

fanquake commented at 9:23 am on October 3, 2023: member

How would that work, given that the release is commercial-only?

I didn’t read the blog, and assumed they would be free to use, similar to x86_64. In that case, I guess we wont use them. Just another downside to GitHub CI.

fanquake merged this on Oct 3, 2023

fanquake closed this on Oct 3, 2023

fanquake commented at 9:32 am on October 3, 2023: member

Opened an issue to keep track of the other failure: #28566.

fanquake referenced this in commit a6683945ca on Oct 3, 2023

fanquake removed the label Needs backport (25.x) on Oct 3, 2023

fanquake commented at 9:34 am on October 3, 2023: member

Added to #28487 for backporting to 25.x.

fanquake referenced this in commit dccacf0bf7 on Oct 3, 2023

fanquake commented at 9:36 am on October 3, 2023: member

Added to #28535 for backporting to 24.x as well.

hebasto deleted the branch on Oct 3, 2023

fanquake referenced this in commit 9f8d501cb8 on Oct 4, 2023

fanquake referenced this in commit 1416d09cba on Oct 6, 2023

PastaPastaPasta referenced this in commit d5e0f4cffb on Oct 6, 2023

UdjinM6 referenced this in commit c956c5ed8d on Oct 12, 2023

Frank-GER referenced this in commit c14b223de3 on Oct 13, 2023

gades referenced this in commit aea2dbd055 on Dec 2, 2023

gades referenced this in commit 867543ba15 on Dec 9, 2023

Fabcien referenced this in commit 0ee22a4fce on Nov 27, 2024

roqqit referenced this in commit 09cab7fd91 on Dec 2, 2024

bitcoin locked this on Dec 3, 2024

build, macos: Fix qt package build with new Xcode 15 linker #28543

Reviews

Conflicts

Guix builds (on x86_64)

build, macos: Fix `qt` package build with new Xcode 15 linker #28543