build: Fix static Darwin builds #21782
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
jarolrod
left a comment
jarolrod
left a comment
There was a problem hiding this comment.
macOS 10.14 SDK doesn't seem to have -platform_version available. When trying to perform a native depends build on macOS 10.14 I get the following error when building libevent:
ld: unknown option: -platform_version
clang: error: linker command failed with exit code 1 (use -v to see invocation)
configure:4267: $? = 1
configure:4307: result: no
The above error would mean this PR wouldn't work in its current form.
Besides broken compatibility with 10.14, this PR seems to be a step in the right direction. The following will be based on my testing of the state of dark mode: bitcoin-core/gui#275 (review)
macOS 10.15
| Master | PR |
|---|---|
 |
 |
otool -l src/qt/bitcoin-qt | grep -A 7 -B 1 BUILD_VERSION
Load command 9
cmd LC_BUILD_VERSION
cmdsize 32
platform 1
minos 10.14
sdk 10.15.6
ntools 1
tool 3
version 609.8
macOS 11.2.3
| Master | PR |
|---|---|
 |
 |
Load command 9
cmd LC_BUILD_VERSION
cmdsize 32
platform 1
minos 10.14
sdk 11.1
ntools 1
tool 3
version 609.8
Did you test a static build? |
When doing static builds, for some reasons, SDK version is not handled correctly when Darwin Driver passes it to a linker via the -platform_version option. This change makes passing the -platform_version option to a linker explicit.
|
Updated 15d41b3 -> d7c83cb (pr21782.01 -> pr21782.02, diff):
|
|
For clarity:
|
Sorry, I don't understand this.
This pr deals with macOS target only. UPDATE: |
|
Almost got thru it.
commit Used commands:
|
|
@RandyMcMillan |
|
Error:
Disabled Tests:
Besides the test error d7c83cb seems good - This may have fixed a dbus issue I was seeing as well. |
|
@RandyMcMillan I'm not sure what's wrong there, but, d7c83cb builds just fine for me on macOS 10.14 |
|
To figure out problems with this PR, could you confirm a successful build the master branch with depends on macOS 10.14? |
|
How is this comment:
solved with the current change? If the linker doesn't support @jarolrod It's not the 10.14 SDK that doesn't support |
|
@fanquake on my macOS 10.14.6 partition: I failed to build natively on macOS with depends using 15d41b3, but it builds using d7c83cb. Also, I don't see |
|
Right, because Regardless, as-is, this change is a bit of an undocumented hack. It need explanation either in depends, or in configure. Just to clarify, as I understand it, static builds aren't "broken", it's just that our current macOS toolchain, whether cross-compiling via depends, or building on macOS, doesn't fill out the |
|
@fanquake hopefully this can clarify a bit, going by my "state of dark mode" testing bitcoin-core/gui#275 (review) From the linked review, we can see that this Linux Cross Compile Native macOS Depends Build Bumped Toolchain Cross Compile Where this PR fits in Native depends build have the I combined the bumped toolchain and this PR on this branch: https://github.com/jarolrod/bitcoin/tree/ctool-and-static-darwin. Bumped Toolchain Cross Compile otool output with this pr: Another (welcome) side-effect of this PR is that the Qt |
The SDK field has also been present in the |
I cannot see |
It's going to depend on which toolchain you've used. The most recently released binaries have |
cf971c9 build: use -isysroot over --sysroot on macOS (fanquake) Pull request description: Not only does this seem to be the more correct behaviour when targeting Darwin, but if you use `-isysroot`, Clangs Darwin driver will [infer the deployment target](https://github.com/llvm/llvm-project/blob/3e037f8f0e26acab8cc784ea4c7d05da79f7c22e/clang/lib/Driver/ToolChains/Darwin.cpp#L1652) from the SDK and use other SDK info when parsing arguments to the linker. In the case of [`-platform_version`](https://github.com/llvm/llvm-project/blob/3e037f8f0e26acab8cc784ea4c7d05da79f7c22e/clang/lib/Driver/ToolChains/Darwin.cpp#L2656), which is added if the linker is [new enough](https://github.com/llvm/llvm-project/blob/3e037f8f0e26acab8cc784ea4c7d05da79f7c22e/clang/lib/Driver/ToolChains/Darwin.cpp#L342), the version tuple is constructed from the SDKInfo, and SDKInfo, as far as I can tell, only exists when `-isysroot` has been passed, see [parseSDKSettings](https://github.com/llvm/llvm-project/blob/3e037f8f0e26acab8cc784ea4c7d05da79f7c22e/clang/lib/Driver/ToolChains/Darwin.cpp#L1785) As a result, the SDK version field in the `LC_BUILD_VERSION` command is filled out. i.e when building master: ```bash cmd LC_BUILD_VERSION cmdsize 32 platform 1 minos 10.14 sdk n/a ntools 1 tool 3 version 650.9 ``` vs this PR: ```bash cmd LC_BUILD_VERSION cmdsize 32 platform 1 minos 10.14 sdk 11.3 ntools 1 tool 3 version 650.9 ``` This, from what I understand, will fix the issue we are having with Qt deciding wether or not to enable features like "Dark mode" on macOS, see #21771, however I have not tested that. Thus this is an alternative to #21782. Our usage of `--sysroot` was added in #17118. ```bash -isysroot <dir> Set the system root directory (usually /) ``` ACKs for top commit: Sjors: tACK cf971c9 hebasto: re-ACK cf971c9, only rebased and addressed comments since my [previous](#21793 (review)) review. Tree-SHA512: f01138179fb85083b5505bbaa48810451098ffa4da5d3c9b673785448790aa76f2e64b2aab6e698f6ee378a21f70626445a3fabee7c61dbfc44e96f3e3964656
cf971c9 build: use -isysroot over --sysroot on macOS (fanquake) Pull request description: Not only does this seem to be the more correct behaviour when targeting Darwin, but if you use `-isysroot`, Clangs Darwin driver will [infer the deployment target](https://github.com/llvm/llvm-project/blob/3e037f8f0e26acab8cc784ea4c7d05da79f7c22e/clang/lib/Driver/ToolChains/Darwin.cpp#L1652) from the SDK and use other SDK info when parsing arguments to the linker. In the case of [`-platform_version`](https://github.com/llvm/llvm-project/blob/3e037f8f0e26acab8cc784ea4c7d05da79f7c22e/clang/lib/Driver/ToolChains/Darwin.cpp#L2656), which is added if the linker is [new enough](https://github.com/llvm/llvm-project/blob/3e037f8f0e26acab8cc784ea4c7d05da79f7c22e/clang/lib/Driver/ToolChains/Darwin.cpp#L342), the version tuple is constructed from the SDKInfo, and SDKInfo, as far as I can tell, only exists when `-isysroot` has been passed, see [parseSDKSettings](https://github.com/llvm/llvm-project/blob/3e037f8f0e26acab8cc784ea4c7d05da79f7c22e/clang/lib/Driver/ToolChains/Darwin.cpp#L1785) As a result, the SDK version field in the `LC_BUILD_VERSION` command is filled out. i.e when building master: ```bash cmd LC_BUILD_VERSION cmdsize 32 platform 1 minos 10.14 sdk n/a ntools 1 tool 3 version 650.9 ``` vs this PR: ```bash cmd LC_BUILD_VERSION cmdsize 32 platform 1 minos 10.14 sdk 11.3 ntools 1 tool 3 version 650.9 ``` This, from what I understand, will fix the issue we are having with Qt deciding wether or not to enable features like "Dark mode" on macOS, see bitcoin#21771, however I have not tested that. Thus this is an alternative to bitcoin#21782. Our usage of `--sysroot` was added in bitcoin#17118. ```bash -isysroot <dir> Set the system root directory (usually /) ``` ACKs for top commit: Sjors: tACK cf971c9 hebasto: re-ACK cf971c9, only rebased and addressed comments since my [previous](bitcoin#21793 (review)) review. Tree-SHA512: f01138179fb85083b5505bbaa48810451098ffa4da5d3c9b673785448790aa76f2e64b2aab6e698f6ee378a21f70626445a3fabee7c61dbfc44e96f3e3964656
6 participants
From
##bitcoin-core-guiIRC channel:Assuming, that fanquake's solution does not involve linker flags, I'm suggesting an alternative one that does use them.
When doing static builds, for some reasons, SDK version is not handled correctly when Darwin Driver passes it to a linker via the
-platform_versionoption.With this PR we pass the
-platform_versionoption to a linker explicitly.Fix #21771.
This change causes harmless warnings:
Building with depends on macOS Big Sur 11.2.3 (20D91):