depends: Set CMAKE_SYSTEM_VERSION for CMake builds
#30465
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. 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. |
|
My Guix build: |
|
What change in behaviour should be expected here? |
For example, when building for macOS, the |
There was a problem hiding this comment.
Concept ACK, but we need more docs here.
- I can't tell (even from looking at the CMake docs) what macOS should be set to and why you've chosen this value
- I can only assume that the Linux value means "minimum kernel version" for the sake of kernel headers, but I don't see that in the docs either
- Agree with @fanquake that it's not clear how this affects our other version vars. Which take precedence?
| @@ -82,3 +82,6 @@ darwin_debug_CFLAGS=-O1 -g | |||
| darwin_debug_CXXFLAGS=$(darwin_debug_CFLAGS) | |||
|
|
|||
| darwin_cmake_system_name=Darwin | |||
| # Darwin version, which corresponds to OSX_MIN_VERSION. | |||
| # See https://en.wikipedia.org/wiki/Darwin_(operating_system) | |||
| darwin_cmake_system_version=20.1 | |||
There was a problem hiding this comment.
Where/how does CMake translate this to OSX_MIN_VERSION? Ideally we'd compose these somehow.
There was a problem hiding this comment.
Where/how does CMake translate this to
OSX_MIN_VERSION?
... the
CMAKE_SYSTEM_VERSIONvalue is used to defineDARWIN_MAJOR_VERSIONandDARWIN_MINOR_VERSION...
It happens here:
# Darwin versions:
# 6.x == Mac OSX 10.2 (Jaguar)
# 7.x == Mac OSX 10.3 (Panther)
# 8.x == Mac OSX 10.4 (Tiger)
# 9.x == Mac OSX 10.5 (Leopard)
# 10.x == Mac OSX 10.6 (Snow Leopard)
# 11.x == Mac OSX 10.7 (Lion)
# 12.x == Mac OSX 10.8 (Mountain Lion)
string(REGEX REPLACE "^([0-9]+)\\.([0-9]+).*$" "\\1" DARWIN_MAJOR_VERSION "${CMAKE_SYSTEM_VERSION}")
string(REGEX REPLACE "^([0-9]+)\\.([0-9]+).*$" "\\2" DARWIN_MINOR_VERSION "${CMAKE_SYSTEM_VERSION}")The chosen Darwin version 20.1 is associated with macOS 11.0:
bitcoin/depends/hosts/darwin.mk
Line 1 in 3799224
I agree that the documentation is not comprehensive. As for However, I assume that the |
|
Here is the previous discussion on this topic: hebasto#3 (comment). In particular, @ryanofsky noted:
|
Given, for example, that if you cross-compile for macOS, with this PR, you end up with different linker flags, this clearly has effect outside of Windows and Android. |
|
Here is an opinion of a CMake maintainer from Professional CMake: A Practical Guide 18th Edition:
|
There was a problem hiding this comment.
ACK 5ff8074
As a reference, this was tested in hebasto/pull/125 (not triggering cross-compiling unnecessarily) and macOS cross build CI (hebasto/pull/94) is running with it since.
All changes to |
|
🐙 This pull request conflicts with the target branch and needs rebase. |
Where did this get followed up on? |
From CMake docs:
This PR is split from #30454 and improves the current depends build subsystem.