[lldb][cmake] Set CMAKE_OSX_SYSROOT when building debugserver with CMake 4
#138020
Conversation
|
@llvm/pr-subscribers-lldb Author: Chelsea Cassanova (chelcassanova) ChangesCMake 4 no longer sets the These files are supposed to be generated by the This commit will fatal error out of config when building debugserver without having set CMAKE_OSX_SYSROOT. Full diff: https://github.com/llvm/llvm-project/pull/138020.diff 1 Files Affected:
diff --git a/lldb/tools/debugserver/source/CMakeLists.txt b/lldb/tools/debugserver/source/CMakeLists.txt
index 1a433898f6aa4..45b16c74f5197 100644
--- a/lldb/tools/debugserver/source/CMakeLists.txt
+++ b/lldb/tools/debugserver/source/CMakeLists.txt
@@ -154,6 +154,10 @@ endif()
add_definitions(-DLLDB_USE_OS_LOG)
+if(NOT CMAKE_OSX_SYSROOT)
+ message(FATAL_ERROR "debugserver needs the macOS SDK root. Please pass in -DCMAKE_OSX_SYSROOT in your CMake invocation")
+endif()
+
if(${CMAKE_OSX_SYSROOT} MATCHES ".Internal.sdk$")
message(STATUS "LLDB debugserver energy support is enabled")
add_definitions(-DLLDB_ENERGY)
|
|
Shouldn't we just pre-populate that cmake variable when it's not set ? That would be a better user experience. |
df5306b to
4e7c209
Compare
|
Just updated the patch to set the |
CMAKRE_OSX_SYSROOT when building debugserver with CMake 4
CMAKRE_OSX_SYSROOT when building debugserver with CMake 4CMAKE_OSX_SYSROOT when building debugserver with CMake 4
| if(NOT CMAKE_OSX_SYSROOT) | ||
| execute_process(COMMAND xcodebuild -version -sdk macosx Path | ||
| OUTPUT_VARIABLE SDKROOT | ||
| ERROR_QUIET | ||
| OUTPUT_STRIP_TRAILING_WHITESPACE) | ||
|
|
||
| if(NOT EXISTS ${SDKROOT}) | ||
| message(FATAL_ERROR "Unable to obtain macOS SDK root, debugserver cannot be built.") | ||
| endif() | ||
|
|
||
| message(STATUS "Using macOS SDK root: ${SDKROOT}") | ||
| set(CMAKE_OSX_SYSROOT ${SDKROOT}) | ||
| endif() |
There was a problem hiding this comment.
I don't think we should try to compute CMAKE_OSX_SYSROOT ourselves. If we need an -isysroot for the mig invocation, it's fine to compute it, but we shouldn't reuse CMAKE_OSX_SYSROOT for that. The documentation also says that it should be set before the project() call (i.e. you're not supposed to compute it).
Here's what I think we should do instead:
if (CMAKE_OSX_SYSROOT)
set(MIG_SYSROOT ${CMAKE_OSX_SYSROOT})
else()
# Set the sysroot based on the output of `xcrun --show-sdk-path`
endif()
And then the mig invocation should use ${MIG_SYSROOT} instead of ${CMAKE_OSX_SYSROOT}.
xcrun is the usual way to get the SDK path and by not specifying the SDK, it will honor the usual ways of changing the SDK (which matters to us internally).
There was a problem hiding this comment.
The documentation also says that it should be set before the project()
Ah, I wasn't aware of that. In that case then I can update this to use a local variable instead of messing with the CMake variable itself.
Though I do also have a question. If CMake no longer computes this value and we're not computing it ourselves, is this now Clang's responsibility? The CMake 4 release notes note that Instead, compilers are expected to choose a default macOS SDK on their own so I guess it is?
Either way, we can get the path we need from xcrun as you said.
4e7c209 to
350648f
Compare
| @@ -154,6 +154,19 @@ endif() | ||
|
|
||
| add_definitions(-DLLDB_USE_OS_LOG) | ||
|
|
||
| if(CMAKE_OSX_SYSROOT) | ||
| set(${MIG_SYSROOT} CMAKE_OSX_SYSROOT) |
There was a problem hiding this comment.
Shouldn't this be set(MIG_SYSROOT ${CMAKE_OSX_SYSROOT})? Does this work as-is?
There was a problem hiding this comment.
Good catch, this doesn't work as-is. I'll update to correct this.
350648f to
6a100fa
Compare
CMake 4 no longer sets the CMAKE_OSX_SYSROOT variable by default, causing the lldb build to fail after all GitHub-hosted runners have been upgraded to CMake 4. As a workaround, the variable is set using CMake command line options. There is a PR to fix this issue in the llvm-project: llvm/llvm-project#138020. We might want to remove this workaround after that PR has been merged.
CMake 4 no longer sets the CMAKE_OSX_SYSROOT variable by default, causing the lldb build to fail after all GitHub-hosted runners have been upgraded to CMake 4. As a workaround, the variable is set using CMake command line options. There is a PR to fix this issue in the llvm-project: llvm/llvm-project#138020. We might want to remove this workaround after that PR has been merged.
|
@bulbazord @JDevlieghere Is it possible to take another pass over at this? I think this should be good to land now. |
| endif() | ||
|
|
||
| if(NOT MIG_SYSROOT) | ||
| message(FATAL_ERROR "Unable to obtain macOS SDK root, debugserver cannot be built.") |
There was a problem hiding this comment.
Let's make this more specific and actionable.
| message(FATAL_ERROR "Unable to obtain macOS SDK root, debugserver cannot be built.") | |
| message(FATAL_ERROR "Unable to obtain sysroot required by mig (Mach Interface Generator). Set CMAKE_OSX_SYSROOT to explicitly specify a sysroot.") |
CMake 4 no longer sets the `CMAKE_OSX_SYSROOT` variable by default. If you've updated to CMake 4 on macOS (e.g. with brew) and try building LLDB with CMake/ninja, this will yield an error when building debugserver that clang is unable to run since it tries to compile files that don't exist. These files are supposed to be generated by the `mig` process. `mig` needs the `CMAKE_OSX_SYSROOT` variable in order to work and without it, it silently fails to generate the files that later on need to be compiled. This commit sets this SDK path for mig and will fatal error out of config when building debugserver without having set CMAKE_OSX_SYSROOT.
6a100fa to
f6f7c91
Compare
CMake 4 no longer sets the
CMAKE_OSX_SYSROOTvariable by default. If you've updated to CMake 4 on macOS (e.g. with brew) and try building LLDB with CMake/ninja, this will yield an error when building debugserver that clang is unable to run since it tries to compile files that don't exist.These files are supposed to be generated by the
migprocess.migneeds theCMAKE_OSX_SYSROOTvariable in order to work and without it, it silently fails to generate the files that later on need to be compiled.This commit will fatal error out of config when building debugserver without having set CMAKE_OSX_SYSROOT.