Skip to content

[CMake] Remove overriding CMAKE_OSX_SYSROOT #19718

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

Merged
merged 1 commit into from
Aug 22, 2025

Conversation

guitargeek
Copy link
Contributor

@guitargeek guitargeek commented Aug 21, 2025

The internet says that newer CMake versions got more reliable in figuring out the SDK path. So setting CMAKE_OSX_SYSROOT ourselves is maybe not needed anymore.

Follows up on #19218.

The internet says that newer CMake versions got more reliable in
figuring out the SDK path. So setting `CMAKE_OSX_SYSROOT` ourselves is
maybe not needed anymore.
@guitargeek guitargeek self-assigned this Aug 21, 2025
@guitargeek guitargeek added in:Build System clean build Ask CI to do non-incremental build on PR labels Aug 21, 2025
@pcanal pcanal self-requested a review August 21, 2025 21:10
@guitargeek guitargeek marked this pull request as ready for review August 21, 2025 21:23
@guitargeek guitargeek requested a review from bellenot as a code owner August 21, 2025 21:23
@guitargeek
Copy link
Contributor Author

guitargeek commented Aug 21, 2025

I don't understand what is going on at this point. PR #18318, which introduced this code, said it was to fix build errors with CMake 4. However, the two macOS platforms that use CMake 4.0.3 (mac15 and mac-beta) already succeeded building. @devajithvs, can you maybe elaborate on why #18318 was needed in the first place? As I said already in #19218 (comment), after studying the CMake source code, I don't see the reason why things should have failed with CMake 4 to begin with.

Copy link

Test Results

    21 files      21 suites   3d 13h 32m 13s ⏱️
 3 557 tests  3 556 ✅ 0 💤 1 ❌
72 950 runs  72 949 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit 7946e41.

@devajithvs
Copy link
Contributor

This was needed because, when CMake 4 on macos-15-4 first became available, it failed to set the CMAKE_OSX_SYSROOT variable. As a result, ROOT would fail to compile since the correct modulemap could not be found, as discussed here: https://root-forum.cern.ch/t/issue-with-macos-15-4-xcode-16-3/63327/2 - this was an emergency fix back then.

@guitargeek
Copy link
Contributor Author

guitargeek commented Aug 22, 2025

Thanks for giving the context! Do we still need it now? CMake 4.0 probably went to not setting CMAKE_OSX_SYSROOT probably for a good reason, so maybe we should not reproduce the previous 3.x behavior

@devajithvs
Copy link
Contributor

Yes, I agree. If the build doesn't fail anymore. It's nice to revert to the previous behavior.

@devajithvs devajithvs self-requested a review August 22, 2025 07:56
@guitargeek guitargeek merged commit e3a10ab into root-project:master Aug 22, 2025
28 of 30 checks passed
@guitargeek guitargeek deleted the osx_sysroot branch August 22, 2025 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clean build Ask CI to do non-incremental build on PR in:Build System
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants