Skip to content
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

cmake: Adjust macOS SDK detection #11893

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jcm93
Copy link
Contributor

@jcm93 jcm93 commented Feb 24, 2025

Description

Adjust the macOS CMake initialization logic to use xcrun to derive our SDK version information, rather than relying on the presence of ${CMAKE_OSX_SYSROOT}.

Motivation and Context

CMake 4.0.0 (currently in the release candidate phase) breaks OBS configuration/generation on macOS. This is because CMake 4.0.0 no longer defines CMAKE_OSX_SYSROOT by default for macOS builds, which we currently rely on to make sure the user has an SDK supported by OBS.

This CMake change was apparently made because specifying CMAKE_OSX_SYSROOT seems to translate under the hood to passing -isysroot to the compiler, which apparently had some undesired impacts in some situations (further context here and here). Letting the compiler wrapper figure out its sysroot on its own seems to now be the preferred pattern.

As we would still like to check what SDK version the user has installed, we can just derive it ourselves using xcrun to populate a variable with the user's environment's currently configured SDK path. This way we can make sure that the user has an SDK version we support, while still letting CMake not define a sysroot.

How Has This Been Tested?

Installed CMake 4.0.0 rc1 and configured/generated/built OBS successfully; also made sure configuration/generation/building proceeded successfully on CMake 3.31.4.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@jcm93 jcm93 changed the title build: Adjust macOS SDK detection cmake: Adjust macOS SDK detection Feb 24, 2025
@WizardCM WizardCM added Bug Fix Non-breaking change which fixes an issue macOS Affects macOS labels Feb 26, 2025
@RytoEX RytoEX requested review from PatTheMav and gxalpha March 4, 2025 19:05
endif()
message(DEBUG "macOS SDK Path: ${_obs_macos_sdk_path}")

string(REGEX MATCH ".+/MacOSX.platform/Developer/SDKs/MacOSX([0-9]+\\.[0-9])+\\.sdk$" _ ${_obs_macos_sdk_path})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will xcrun always yield the numeric variant of the SDK? By default they are just symlinks to MacOSX.sdk anyway.

Copy link
Contributor Author

@jcm93 jcm93 Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The xcrun man page doesn't specifically indicate that it provides the fully resolved versioned symlink path always.

Adding a file(REAL_PATH ...) call to resolve the path seems like probably a good idea just in case.

We could also alternately use the --show-sdk-platform-version command provided by xcrun instead of --show-sdk-path, and short circuit the regex matching that we perform to determine the version. Though that regex specifically matches on Xcode paths and not Xcode Command Line Tools paths, so we might have to do something else in that case to detect that the user is specifically using Xcode and not just the command line tools.

edit: As you indicated, the xcrun-derived path is indeed a symlink; and we want the symlink path in this case rather than the absolute path.

I doubt that the existing versioning system in Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs is going anywhere, but it might be slightly more robust/future-proof to use --show-sdk-platform-version and then a separate check to verify that the user has Xcode and not just the CLT.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do something like separately run xcrun --find xcodebuild to detect Xcode.

Copy link
Contributor Author

@jcm93 jcm93 Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# Ensure recent enough Xcode and platform SDK
set(_obs_macos_minimum_sdk 15.0) # Keep in sync with Xcode
set(_obs_macos_minimum_xcode 16.0) # Keep in sync with SDK
execute_process(
  COMMAND xcrun --sdk macosx --show-sdk-platform-version
  OUTPUT_VARIABLE _obs_macos_current_sdk
  RESULT_VARIABLE _result
  OUTPUT_STRIP_TRAILING_WHITESPACE
)
if(NOT _result EQUAL 0)
  message(
    FATAL_ERROR
    "Failed to fetch macOS SDK version. "
    "Ensure that the macOS SDK is installed and that xcode-select points at the Xcode developer directory."
  )
endif()
message(DEBUG "macOS SDK version: ${_obs_macos_current_sdk}")
if(_obs_macos_current_sdk VERSION_LESS _obs_macos_minimum_sdk)
  message(
    FATAL_ERROR
    "Your macOS SDK version (${_obs_macos_current_sdk}) is too low. "
    "The macOS ${_obs_macos_minimum_sdk} SDK (Xcode ${_obs_macos_minimum_xcode}) is required to build OBS."
  )
endif()
execute_process(COMMAND xcrun --find xcodebuild OUTPUT_VARIABLE _obs_macos_xcodebuild RESULT_VARIABLE _result)
if(NOT _result EQUAL 0)
  message(
    FATAL_ERROR
    "Xcode was not found. "
    "Ensure you have installed Xcode and that xcode-select points at the Xcode developer directory."
  )
endif()
message(DEBUG "Path to xcodebuild binary: ${_obs_macos_xcodebuild}")
unset(_result)
unset(_obs_macos_xcodebuild)
unset(_obs_macos_current_sdk)
unset(_obs_macos_minimum_sdk)
unset(_obs_macos_minimum_xcode)

Let me know if you think this is a better approach (get the SDK version specifically, then look for xcodebuild specifically, no regex path matching) and I can update if so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Fix Non-breaking change which fixes an issue macOS Affects macOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants