-
Notifications
You must be signed in to change notification settings - Fork 988
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
Fix autodetect CMAKE_SYSTEM_VERSION to convert to Darwin version #16335
Merged
memsharded
merged 8 commits into
conan-io:develop2
from
juansblanco:jsb/cmake-system-version
May 30, 2024
Merged
Fix autodetect CMAKE_SYSTEM_VERSION to convert to Darwin version #16335
memsharded
merged 8 commits into
conan-io:develop2
from
juansblanco:jsb/cmake-system-version
May 30, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
juansblanco
commented
May 24, 2024
czoido
reviewed
May 24, 2024
czoido
reviewed
May 24, 2024
czoido
reviewed
May 28, 2024
czoido
reviewed
May 28, 2024
czoido
reviewed
May 28, 2024
czoido
approved these changes
May 29, 2024
memsharded
approved these changes
May 29, 2024
jcar87
approved these changes
May 30, 2024
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.
LGTM!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changelog: Fix: Change autodetect of
CMAKE_SYSTEM_VERSION
to use the Darwin version.Docs: conan-io/docs#3755
CMAKE_SYSTEM_NAME
andCMAKE_SYSTEM_VERSION
are required to be set when cross-compiling.https://cmake.org/cmake/help/latest/variable/CMAKE_SYSTEM_VERSION.html
We recommend that users set them in the toolchain or through confs, but if they are not set we autodetect them based on the profile settings.
In the case of macOS systems, the
CMAKE_SYSTEM_VERSION
variable was set to the macOS version, but it needs to be the Darwin version. We need to set the Darwin version for each Apple system: https://en.wikipedia.org/wiki/Darwin_(operating_system)#Darwin_20_onwardsThis PR fixes it by converting the macOS versions to Darwin versions when the user doesn't set the variable.
Other option could be to stop autodetecting this variables and force users to set it themselves.
CMake states that the
CMAKE_SYSTEM_VERSION
is not usually set for Darwin systems and is not particularly meaningful, and users should be careful when making use of it in their projects.It has almost no utility on the CMake side for Darwin systems
https://github.com/Kitware/CMake/blob/f0074159d75601145b6ab0a98d636ec4c17a4370/Modules/Platform/Darwin.cmake#L18
Closes: #16282