[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
https://github.com/carlocab edited https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
@@ -43,8 +43,8 @@ // RUN: %clang -target x86_64-apple-macos10.13 -mlinker-version=520 \ // RUN: -### %t.o 2>&1 \ -// RUN: | FileCheck --check-prefix=NOSDK %s +// RUN: | FileCheck --check-prefix=INFERRED-SDK %s carlocab wrote: I think it does provide some value, particularly for builds with `CLANG_USE_XCSELECT=ON` (since there is a stricter test that was separated out for builds where it is disabled). That is, we check that the first two arguments to `-platform_version`, "macos" and "10.13.0", are correctly inferred from `-target`, and that the last argument looks like a version scheme, even when no `-isysroot` flag is provided. That still seems valuable to me, even if it's not the same as the original test. However, if you're still not convinced, I can remove it. https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
@@ -43,8 +43,8 @@ // RUN: %clang -target x86_64-apple-macos10.13 -mlinker-version=520 \ // RUN: -### %t.o 2>&1 \ -// RUN: | FileCheck --check-prefix=NOSDK %s +// RUN: | FileCheck --check-prefix=INFERRED-SDK %s cachemeifyoucan wrote: I see. The value of the test is to check the behavior of matching the version when SDK is not provided. I don't think current test provide any value. https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
@@ -43,8 +43,8 @@ // RUN: %clang -target x86_64-apple-macos10.13 -mlinker-version=520 \ // RUN: -### %t.o 2>&1 \ -// RUN: | FileCheck --check-prefix=NOSDK %s +// RUN: | FileCheck --check-prefix=INFERRED-SDK %s carlocab wrote: I think they'll pass since I've just relaxed the text to match against. But I'll double check locally for both enabling and disabling `CLANG_USE_XCSELECT` and delete them if it fails. https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
https://github.com/cachemeifyoucan approved this pull request. LGTM after addressing the last comment. https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
@@ -43,8 +43,8 @@ // RUN: %clang -target x86_64-apple-macos10.13 -mlinker-version=520 \ // RUN: -### %t.o 2>&1 \ -// RUN: | FileCheck --check-prefix=NOSDK %s +// RUN: | FileCheck --check-prefix=INFERRED-SDK %s cachemeifyoucan wrote: I think you want to delete those two. The test will fail if no CLANG_USE_XCSELECT is used. https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
carlocab wrote: Thanks, done. https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
https://github.com/carlocab updated https://github.com/llvm/llvm-project/pull/119670 >From 9db768ec1be84d6bf647b85f052776720822f64f Mon Sep 17 00:00:00 2001 From: Carlo Cabrera Date: Tue, 10 Dec 2024 01:45:22 +0800 Subject: [PATCH 1/7] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK This is a scaled down version of https://reviews.llvm.org/D136315. The intent is largely the same as before[^1], but I've scaled down the scope to try to avoid the issues that the previous patch caused: - the changes are now opt-in based on enabling `CLANG_USE_XCSELECT` - this only works when targeting macOS on a macOS host (this is the only case supported by `libxcselect`[^2]) We also introduce an environment variable `CLANG_NO_XCSELECT` that disables this behaviour if Clang is configured with `CLANG_USE_XCSELECT=ON`. This is needed to avoid breaking tests. Another reason to leave this as opt-in for now is that there are some bugs in libxcselect that need fixing before it is safe to use by default for all users. This has been reported to Apple as FB16081077. [^1]: See also https://reviews.llvm.org/D109460 and #45225. [^2]: https://developer.apple.com/documentation/xcselect?language=objc --- clang/CMakeLists.txt | 33 +++ clang/include/clang/Config/config.h.cmake | 6 + clang/lib/Driver/CMakeLists.txt | 4 +++ clang/lib/Driver/ToolChains/Darwin.cpp| 31 ++--- clang/test/lit.cfg.py | 4 +++ 5 files changed, 69 insertions(+), 9 deletions(-) diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt index 27e8095534a65c..3c7096a27c509b 100644 --- a/clang/CMakeLists.txt +++ b/clang/CMakeLists.txt @@ -214,6 +214,39 @@ if(GCC_INSTALL_PREFIX AND NOT USE_DEPRECATED_GCC_INSTALL_PREFIX) "See https://github.com/llvm/llvm-project/pull/77537 for detail.") endif() +if(APPLE) + check_include_file(xcselect.h CLANG_HAVE_XCSELECT_H) + if(CLANG_HAVE_XCSELECT_H) +include(CheckSymbolExists) +list(APPEND CMAKE_REQUIRED_LIBRARIES xcselect) +check_symbol_exists(xcselect_host_sdk_path xcselect.h CLANG_HAVE_XCSELECT_HOST_SDK_PATH) +list(REMOVE_ITEM CMAKE_REQUIRED_LIBRARIES xcselect) + endif() +endif() + +cmake_dependent_option(CLANG_USE_XCSELECT "Use libxcselect to find the macOS SDK." OFF + "APPLE;CLANG_HAVE_XCSELECT_HOST_SDK_PATH" OFF) + +if(DEFAULT_SYSROOT AND CLANG_USE_XCSELECT) + message(FATAL_ERROR "Setting DEFAULT_SYSROOT is incompatible with CLANG_USE_XCSELECT.") +endif() + +if(CLANG_USE_XCSELECT) + set(XCSELECT_VALID_POLICIES LATEST MATCHING_ONLY MATCHING_PREFERRED) + set(CLANG_XCSELECT_HOST_SDK_POLICY "LATEST" CACHE STRING +"Policy to use for xcselect. One of: ${XCSELECT_VALID_POLICIES}") + set_property(CACHE CLANG_XCSELECT_HOST_SDK_POLICY PROPERTY STRINGS ${XCSELECT_VALID_POLICIES}) + string(TOUPPER ${CLANG_XCSELECT_HOST_SDK_POLICY} CLANG_XCSELECT_HOST_SDK_POLICY) + list(JOIN XCSELECT_VALID_POLICIES "|" XCSELECT_POLICY_REGEX) + if(NOT CLANG_XCSELECT_HOST_SDK_POLICY MATCHES "^XCSELECT_HOST_SDK_POLICY_(${XCSELECT_POLICY_REGEX})$") +if(NOT CLANG_XCSELECT_HOST_SDK_POLICY IN_LIST XCSELECT_VALID_POLICIES) + message(FATAL_ERROR +"CLANG_XCSELECT_HOST_SDK_POLICY (${CLANG_XCSELECT_HOST_SDK_POLICY}) must be one of: ${XCSELECT_VALID_POLICIES}") +endif() +set(CLANG_XCSELECT_HOST_SDK_POLICY "XCSELECT_HOST_SDK_POLICY_${CLANG_XCSELECT_HOST_SDK_POLICY}") + endif() +endif() + set(ENABLE_LINKER_BUILD_ID OFF CACHE BOOL "pass --build-id to ld") set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL diff --git a/clang/include/clang/Config/config.h.cmake b/clang/include/clang/Config/config.h.cmake index 27ed69e21562bf..9ae0c6a4b9e11d 100644 --- a/clang/include/clang/Config/config.h.cmake +++ b/clang/include/clang/Config/config.h.cmake @@ -86,4 +86,10 @@ /* Whether CIR is built into Clang */ #cmakedefine01 CLANG_ENABLE_CIR +/* Whether to use xcselect to find the macOS SDK */ +#cmakedefine CLANG_USE_XCSELECT + +/* Policy to use for xcselect */ +#cmakedefine CLANG_XCSELECT_HOST_SDK_POLICY ${CLANG_XCSELECT_HOST_SDK_POLICY} + #endif diff --git a/clang/lib/Driver/CMakeLists.txt b/clang/lib/Driver/CMakeLists.txt index 4fd10bf671512f..299de2ef30470c 100644 --- a/clang/lib/Driver/CMakeLists.txt +++ b/clang/lib/Driver/CMakeLists.txt @@ -14,6 +14,10 @@ if(WIN32) set(system_libs version) endif() +if(CLANG_USE_XCSELECT) + set(system_libs xcselect) +endif() + add_clang_library(clangDriver Action.cpp Compilation.cpp diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index 87380869f6fdab..8d3e0130a5a432 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -29,6 +29,10 @@ #include "llvm/TargetParser/Triple.h" #include // ::getenv +#ifdef CLANG_USE_XCSELECT +#include // ::xcselect_host_sdk_path +#endif + using namespace clang::driver; using namespace clang::driver::tools;
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
cachemeifyoucan wrote: > For reference, the failure from dropping `XFAIL` from > `darwin-ld-platform-version-macos.c`: > > Failure Log > ``` > + /Users/carlocab/github/llvm-project/build-xcselect/bin/FileCheck > --check-prefix=NOSDK > /Users/carlocab/github/llvm-project/clang/test/Driver/darwin-ld-platform-version-macos.c > /Users/carlocab/github/llvm-project/clang/test/Driver/darwin-ld-platform-version-macos.c:50:11: > error: NOSDK: expected string not found in input > // NOSDK: "-platform_version" "macos" "10.13.0" "10.13.0" > ^ > :1:1: note: scanning from here > clang version 20.0.0git (https://github.com/carlocab/llvm-project.git > dc195bd7668c09b7ec94abf771b618170cfd4b78) > ^ > :6:141: note: possible intended match here > "/usr/bin/ld" "-demangle" "-lto_library" > "/Users/carlocab/github/llvm-project/build-xcselect/lib/libLTO.dylib" > "-dynamic" "-arch" "x86_64" "-platform_version" "macos" "10.13.0" "15.2" > "-syslibroot" > "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX15.2.sdk" > "-mllvm" "-enable-linkonceodr-outlining" "-o" "a.out" > "/Users/carlocab/github/llvm-project/build-xcselect/tools/clang/test/Driver/Output/darwin-ld-platform-version-macos.c.tmp.o" > "-lSystem" > "/Users/carlocab/github/llvm-project/build-xcselect/lib/clang/20/lib/darwin/libclang_rt.osx.a" > > ^ > > Input file: > Check file: > /Users/carlocab/github/llvm-project/clang/test/Driver/darwin-ld-platform-version-macos.c > > -dump-input=help explains the following input dump. > > Input was: > << > 1: clang version 20.0.0git > (https://github.com/carlocab/llvm-project.git > dc195bd7668c09b7ec94abf771b618170cfd4b78) > check:50'0 > X~~~ > error: no match found > 2: Target: x86_64-apple-macos10.13 > check:50'0 > 3: Thread model: posix > check:50'0 > 4: InstalledDir: > /Users/carlocab/github/llvm-project/build-xcselect/bin > check:50'0 > ~ > 5: Build config: +assertions > check:50'0 ~~ > 6: "/usr/bin/ld" "-demangle" "-lto_library" > "/Users/carlocab/github/llvm-project/build-xcselect/lib/libLTO.dylib" > "-dynamic" "-arch" "x86_64" "-platform_version" "macos" "10.13.0" "15.2" > "-syslibroot" > "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX15.2.sdk" > "-mllvm" "-enable-linkonceodr-outlining" "-o" "a.out" > "/Users/carlocab/github/llvm-project/build-xcselect/tools/clang/test/Driver/Output/darwin-ld-platform-version-macos.c.tmp.o" > "-lSystem" > "/Users/carlocab/github/llvm-project/build-xcselect/lib/clang/20/lib/darwin/libclang_rt.osx.a" > check:50'0 > ~ > check:50'1 > > ? > > > > > possible intended > match > >> > ``` That looks like if that no SDK is provided, the SDK version is going to match deployment target. Let's break the NOSDK test into a separate file and mark that file as `UNSUPPORTED:` https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
https://github.com/carlocab updated https://github.com/llvm/llvm-project/pull/119670 >From 9db768ec1be84d6bf647b85f052776720822f64f Mon Sep 17 00:00:00 2001 From: Carlo Cabrera Date: Tue, 10 Dec 2024 01:45:22 +0800 Subject: [PATCH 1/6] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK This is a scaled down version of https://reviews.llvm.org/D136315. The intent is largely the same as before[^1], but I've scaled down the scope to try to avoid the issues that the previous patch caused: - the changes are now opt-in based on enabling `CLANG_USE_XCSELECT` - this only works when targeting macOS on a macOS host (this is the only case supported by `libxcselect`[^2]) We also introduce an environment variable `CLANG_NO_XCSELECT` that disables this behaviour if Clang is configured with `CLANG_USE_XCSELECT=ON`. This is needed to avoid breaking tests. Another reason to leave this as opt-in for now is that there are some bugs in libxcselect that need fixing before it is safe to use by default for all users. This has been reported to Apple as FB16081077. [^1]: See also https://reviews.llvm.org/D109460 and #45225. [^2]: https://developer.apple.com/documentation/xcselect?language=objc --- clang/CMakeLists.txt | 33 +++ clang/include/clang/Config/config.h.cmake | 6 + clang/lib/Driver/CMakeLists.txt | 4 +++ clang/lib/Driver/ToolChains/Darwin.cpp| 31 ++--- clang/test/lit.cfg.py | 4 +++ 5 files changed, 69 insertions(+), 9 deletions(-) diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt index 27e8095534a65c..3c7096a27c509b 100644 --- a/clang/CMakeLists.txt +++ b/clang/CMakeLists.txt @@ -214,6 +214,39 @@ if(GCC_INSTALL_PREFIX AND NOT USE_DEPRECATED_GCC_INSTALL_PREFIX) "See https://github.com/llvm/llvm-project/pull/77537 for detail.") endif() +if(APPLE) + check_include_file(xcselect.h CLANG_HAVE_XCSELECT_H) + if(CLANG_HAVE_XCSELECT_H) +include(CheckSymbolExists) +list(APPEND CMAKE_REQUIRED_LIBRARIES xcselect) +check_symbol_exists(xcselect_host_sdk_path xcselect.h CLANG_HAVE_XCSELECT_HOST_SDK_PATH) +list(REMOVE_ITEM CMAKE_REQUIRED_LIBRARIES xcselect) + endif() +endif() + +cmake_dependent_option(CLANG_USE_XCSELECT "Use libxcselect to find the macOS SDK." OFF + "APPLE;CLANG_HAVE_XCSELECT_HOST_SDK_PATH" OFF) + +if(DEFAULT_SYSROOT AND CLANG_USE_XCSELECT) + message(FATAL_ERROR "Setting DEFAULT_SYSROOT is incompatible with CLANG_USE_XCSELECT.") +endif() + +if(CLANG_USE_XCSELECT) + set(XCSELECT_VALID_POLICIES LATEST MATCHING_ONLY MATCHING_PREFERRED) + set(CLANG_XCSELECT_HOST_SDK_POLICY "LATEST" CACHE STRING +"Policy to use for xcselect. One of: ${XCSELECT_VALID_POLICIES}") + set_property(CACHE CLANG_XCSELECT_HOST_SDK_POLICY PROPERTY STRINGS ${XCSELECT_VALID_POLICIES}) + string(TOUPPER ${CLANG_XCSELECT_HOST_SDK_POLICY} CLANG_XCSELECT_HOST_SDK_POLICY) + list(JOIN XCSELECT_VALID_POLICIES "|" XCSELECT_POLICY_REGEX) + if(NOT CLANG_XCSELECT_HOST_SDK_POLICY MATCHES "^XCSELECT_HOST_SDK_POLICY_(${XCSELECT_POLICY_REGEX})$") +if(NOT CLANG_XCSELECT_HOST_SDK_POLICY IN_LIST XCSELECT_VALID_POLICIES) + message(FATAL_ERROR +"CLANG_XCSELECT_HOST_SDK_POLICY (${CLANG_XCSELECT_HOST_SDK_POLICY}) must be one of: ${XCSELECT_VALID_POLICIES}") +endif() +set(CLANG_XCSELECT_HOST_SDK_POLICY "XCSELECT_HOST_SDK_POLICY_${CLANG_XCSELECT_HOST_SDK_POLICY}") + endif() +endif() + set(ENABLE_LINKER_BUILD_ID OFF CACHE BOOL "pass --build-id to ld") set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL diff --git a/clang/include/clang/Config/config.h.cmake b/clang/include/clang/Config/config.h.cmake index 27ed69e21562bf..9ae0c6a4b9e11d 100644 --- a/clang/include/clang/Config/config.h.cmake +++ b/clang/include/clang/Config/config.h.cmake @@ -86,4 +86,10 @@ /* Whether CIR is built into Clang */ #cmakedefine01 CLANG_ENABLE_CIR +/* Whether to use xcselect to find the macOS SDK */ +#cmakedefine CLANG_USE_XCSELECT + +/* Policy to use for xcselect */ +#cmakedefine CLANG_XCSELECT_HOST_SDK_POLICY ${CLANG_XCSELECT_HOST_SDK_POLICY} + #endif diff --git a/clang/lib/Driver/CMakeLists.txt b/clang/lib/Driver/CMakeLists.txt index 4fd10bf671512f..299de2ef30470c 100644 --- a/clang/lib/Driver/CMakeLists.txt +++ b/clang/lib/Driver/CMakeLists.txt @@ -14,6 +14,10 @@ if(WIN32) set(system_libs version) endif() +if(CLANG_USE_XCSELECT) + set(system_libs xcselect) +endif() + add_clang_library(clangDriver Action.cpp Compilation.cpp diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index 87380869f6fdab..8d3e0130a5a432 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -29,6 +29,10 @@ #include "llvm/TargetParser/Triple.h" #include // ::getenv +#ifdef CLANG_USE_XCSELECT +#include // ::xcselect_host_sdk_path +#endif + using namespace clang::driver; using namespace clang::driver::tools;
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
carlocab wrote: For reference, the failure from dropping `XFAIL` from `darwin-ld-platform-version-macos.c`: Failure Log ``` + /Users/carlocab/github/llvm-project/build-xcselect/bin/FileCheck --check-prefix=NOSDK /Users/carlocab/github/llvm-project/clang/test/Driver/darwin-ld-platform-version-macos.c /Users/carlocab/github/llvm-project/clang/test/Driver/darwin-ld-platform-version-macos.c:50:11: error: NOSDK: expected string not found in input // NOSDK: "-platform_version" "macos" "10.13.0" "10.13.0" ^ :1:1: note: scanning from here clang version 20.0.0git (https://github.com/carlocab/llvm-project.git dc195bd7668c09b7ec94abf771b618170cfd4b78) ^ :6:141: note: possible intended match here "/usr/bin/ld" "-demangle" "-lto_library" "/Users/carlocab/github/llvm-project/build-xcselect/lib/libLTO.dylib" "-dynamic" "-arch" "x86_64" "-platform_version" "macos" "10.13.0" "15.2" "-syslibroot" "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX15.2.sdk" "-mllvm" "-enable-linkonceodr-outlining" "-o" "a.out" "/Users/carlocab/github/llvm-project/build-xcselect/tools/clang/test/Driver/Output/darwin-ld-platform-version-macos.c.tmp.o" "-lSystem" "/Users/carlocab/github/llvm-project/build-xcselect/lib/clang/20/lib/darwin/libclang_rt.osx.a" ^ Input file: Check file: /Users/carlocab/github/llvm-project/clang/test/Driver/darwin-ld-platform-version-macos.c -dump-input=help explains the following input dump. Input was: << 1: clang version 20.0.0git (https://github.com/carlocab/llvm-project.git dc195bd7668c09b7ec94abf771b618170cfd4b78) check:50'0 X~~~ error: no match found 2: Target: x86_64-apple-macos10.13 check:50'0 3: Thread model: posix check:50'0 4: InstalledDir: /Users/carlocab/github/llvm-project/build-xcselect/bin check:50'0 ~ 5: Build config: +assertions check:50'0 ~~ 6: "/usr/bin/ld" "-demangle" "-lto_library" "/Users/carlocab/github/llvm-project/build-xcselect/lib/libLTO.dylib" "-dynamic" "-arch" "x86_64" "-platform_version" "macos" "10.13.0" "15.2" "-syslibroot" "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX15.2.sdk" "-mllvm" "-enable-linkonceodr-outlining" "-o" "a.out" "/Users/carlocab/github/llvm-project/build-xcselect/tools/clang/test/Driver/Output/darwin-ld-platform-version-macos.c.tmp.o" "-lSystem" "/Users/carlocab/github/llvm-project/build-xcselect/lib/clang/20/lib/darwin/libclang_rt.osx.a" check:50'0 ~ check:50'1 ? possible intended match >> ``` https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
carlocab wrote: After excluding the `-darwin*` spelling from using `xcselect`, most of the failures go away, but one remains. I'll investigate it tomorrow. I've left it as `XFAIL` for now, since it seems to be the least bad option (compared to `CLANG_NO_XCSELECT` or `UNSUPPORTED`), given: > > I'm wondering if it's worth reviving the CLANG_NO_XCSELECT environment > > variable and then just setting them in these tests (and slowly getting rid > > of instances of CLANG_NO_XCSELECT as we find better fixes). > > The downside here is that when investigating running each command, it might > be confusing that the error is not reproducing unless an env is set. > > Alternative is just `UNSUPPORTED:` but that will reduce the coverage when > XCSELECT is configured. https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
@@ -2257,17 +2261,25 @@ void Darwin::AddDeploymentTarget(DerivedArgList &Args) const { // Warn if the path does not exist. if (!getVFS().exists(A->getValue())) getDriver().Diag(clang::diag::warn_missing_sysroot) << A->getValue(); - } else { -if (char *env = ::getenv("SDKROOT")) { - // We only use this value as the default if it is an absolute path, - // exists, and it is not the root path. - if (llvm::sys::path::is_absolute(env) && getVFS().exists(env) && - StringRef(env) != "/") { -Args.append(Args.MakeSeparateArg( -nullptr, Opts.getOption(options::OPT_isysroot), env)); - } + } else if (const char *env = ::getenv("SDKROOT")) { +// We only use this value as the default if it is an absolute path, +// exists, and it is not the root path. +if (llvm::sys::path::is_absolute(env) && getVFS().exists(env) && +StringRef(env) != "/") { + Args.append(Args.MakeSeparateArg( + nullptr, Opts.getOption(options::OPT_isysroot), env)); } } +#ifdef CLANG_USE_XCSELECT + else if (getTriple().isMacOSX()) { carlocab wrote: Yes, thanks, applied in c51df70d1380c3c7476f5314b0d34605e6fa8ac6 https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
https://github.com/carlocab updated https://github.com/llvm/llvm-project/pull/119670 >From 9db768ec1be84d6bf647b85f052776720822f64f Mon Sep 17 00:00:00 2001 From: Carlo Cabrera Date: Tue, 10 Dec 2024 01:45:22 +0800 Subject: [PATCH 1/6] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK This is a scaled down version of https://reviews.llvm.org/D136315. The intent is largely the same as before[^1], but I've scaled down the scope to try to avoid the issues that the previous patch caused: - the changes are now opt-in based on enabling `CLANG_USE_XCSELECT` - this only works when targeting macOS on a macOS host (this is the only case supported by `libxcselect`[^2]) We also introduce an environment variable `CLANG_NO_XCSELECT` that disables this behaviour if Clang is configured with `CLANG_USE_XCSELECT=ON`. This is needed to avoid breaking tests. Another reason to leave this as opt-in for now is that there are some bugs in libxcselect that need fixing before it is safe to use by default for all users. This has been reported to Apple as FB16081077. [^1]: See also https://reviews.llvm.org/D109460 and #45225. [^2]: https://developer.apple.com/documentation/xcselect?language=objc --- clang/CMakeLists.txt | 33 +++ clang/include/clang/Config/config.h.cmake | 6 + clang/lib/Driver/CMakeLists.txt | 4 +++ clang/lib/Driver/ToolChains/Darwin.cpp| 31 ++--- clang/test/lit.cfg.py | 4 +++ 5 files changed, 69 insertions(+), 9 deletions(-) diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt index 27e8095534a65c..3c7096a27c509b 100644 --- a/clang/CMakeLists.txt +++ b/clang/CMakeLists.txt @@ -214,6 +214,39 @@ if(GCC_INSTALL_PREFIX AND NOT USE_DEPRECATED_GCC_INSTALL_PREFIX) "See https://github.com/llvm/llvm-project/pull/77537 for detail.") endif() +if(APPLE) + check_include_file(xcselect.h CLANG_HAVE_XCSELECT_H) + if(CLANG_HAVE_XCSELECT_H) +include(CheckSymbolExists) +list(APPEND CMAKE_REQUIRED_LIBRARIES xcselect) +check_symbol_exists(xcselect_host_sdk_path xcselect.h CLANG_HAVE_XCSELECT_HOST_SDK_PATH) +list(REMOVE_ITEM CMAKE_REQUIRED_LIBRARIES xcselect) + endif() +endif() + +cmake_dependent_option(CLANG_USE_XCSELECT "Use libxcselect to find the macOS SDK." OFF + "APPLE;CLANG_HAVE_XCSELECT_HOST_SDK_PATH" OFF) + +if(DEFAULT_SYSROOT AND CLANG_USE_XCSELECT) + message(FATAL_ERROR "Setting DEFAULT_SYSROOT is incompatible with CLANG_USE_XCSELECT.") +endif() + +if(CLANG_USE_XCSELECT) + set(XCSELECT_VALID_POLICIES LATEST MATCHING_ONLY MATCHING_PREFERRED) + set(CLANG_XCSELECT_HOST_SDK_POLICY "LATEST" CACHE STRING +"Policy to use for xcselect. One of: ${XCSELECT_VALID_POLICIES}") + set_property(CACHE CLANG_XCSELECT_HOST_SDK_POLICY PROPERTY STRINGS ${XCSELECT_VALID_POLICIES}) + string(TOUPPER ${CLANG_XCSELECT_HOST_SDK_POLICY} CLANG_XCSELECT_HOST_SDK_POLICY) + list(JOIN XCSELECT_VALID_POLICIES "|" XCSELECT_POLICY_REGEX) + if(NOT CLANG_XCSELECT_HOST_SDK_POLICY MATCHES "^XCSELECT_HOST_SDK_POLICY_(${XCSELECT_POLICY_REGEX})$") +if(NOT CLANG_XCSELECT_HOST_SDK_POLICY IN_LIST XCSELECT_VALID_POLICIES) + message(FATAL_ERROR +"CLANG_XCSELECT_HOST_SDK_POLICY (${CLANG_XCSELECT_HOST_SDK_POLICY}) must be one of: ${XCSELECT_VALID_POLICIES}") +endif() +set(CLANG_XCSELECT_HOST_SDK_POLICY "XCSELECT_HOST_SDK_POLICY_${CLANG_XCSELECT_HOST_SDK_POLICY}") + endif() +endif() + set(ENABLE_LINKER_BUILD_ID OFF CACHE BOOL "pass --build-id to ld") set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL diff --git a/clang/include/clang/Config/config.h.cmake b/clang/include/clang/Config/config.h.cmake index 27ed69e21562bf..9ae0c6a4b9e11d 100644 --- a/clang/include/clang/Config/config.h.cmake +++ b/clang/include/clang/Config/config.h.cmake @@ -86,4 +86,10 @@ /* Whether CIR is built into Clang */ #cmakedefine01 CLANG_ENABLE_CIR +/* Whether to use xcselect to find the macOS SDK */ +#cmakedefine CLANG_USE_XCSELECT + +/* Policy to use for xcselect */ +#cmakedefine CLANG_XCSELECT_HOST_SDK_POLICY ${CLANG_XCSELECT_HOST_SDK_POLICY} + #endif diff --git a/clang/lib/Driver/CMakeLists.txt b/clang/lib/Driver/CMakeLists.txt index 4fd10bf671512f..299de2ef30470c 100644 --- a/clang/lib/Driver/CMakeLists.txt +++ b/clang/lib/Driver/CMakeLists.txt @@ -14,6 +14,10 @@ if(WIN32) set(system_libs version) endif() +if(CLANG_USE_XCSELECT) + set(system_libs xcselect) +endif() + add_clang_library(clangDriver Action.cpp Compilation.cpp diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index 87380869f6fdab..8d3e0130a5a432 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -29,6 +29,10 @@ #include "llvm/TargetParser/Triple.h" #include // ::getenv +#ifdef CLANG_USE_XCSELECT +#include // ::xcselect_host_sdk_path +#endif + using namespace clang::driver; using namespace clang::driver::tools;
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
https://github.com/cachemeifyoucan edited https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
@@ -2257,17 +2261,25 @@ void Darwin::AddDeploymentTarget(DerivedArgList &Args) const { // Warn if the path does not exist. if (!getVFS().exists(A->getValue())) getDriver().Diag(clang::diag::warn_missing_sysroot) << A->getValue(); - } else { -if (char *env = ::getenv("SDKROOT")) { - // We only use this value as the default if it is an absolute path, - // exists, and it is not the root path. - if (llvm::sys::path::is_absolute(env) && getVFS().exists(env) && - StringRef(env) != "/") { -Args.append(Args.MakeSeparateArg( -nullptr, Opts.getOption(options::OPT_isysroot), env)); - } + } else if (const char *env = ::getenv("SDKROOT")) { +// We only use this value as the default if it is an absolute path, +// exists, and it is not the root path. +if (llvm::sys::path::is_absolute(env) && getVFS().exists(env) && +StringRef(env) != "/") { + Args.append(Args.MakeSeparateArg( + nullptr, Opts.getOption(options::OPT_isysroot), env)); } } +#ifdef CLANG_USE_XCSELECT + else if (getTriple().isMacOSX()) { cachemeifyoucan wrote: The fix I am talking about is to use `getOS() == Darwin` instead of `isMacOSX()` https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
cachemeifyoucan wrote: > Is there any reason why default builds don't just use the macOS spelling, at > least for modern versions of macOS? I don't think anyone has looked into changing the host guess script. I am ok to switch to macOS spelling. > I'm wondering if it's worth reviving the CLANG_NO_XCSELECT environment > variable and then just setting them in these tests (and slowly getting rid of > instances of CLANG_NO_XCSELECT as we find better fixes). The downside here is that when investigating running each command, it might be confusing that the error is not reproducing unless an env is set. Alternative is just `UNSUPPORTED:` but that will reduce the coverage when XCSELECT is configured. https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
carlocab wrote: > Maybe it is because the `darwin` legacy OS in the triple? This is a good guess, it seems to help: ```diff diff --git a/clang/test/Driver/arc.c b/clang/test/Driver/arc.c index 7f6ac81aad17..70cd66899dff 100644 --- a/clang/test/Driver/arc.c +++ b/clang/test/Driver/arc.c @@ -1,11 +1,8 @@ -// XFAIL: xcselect -// FIXME: There's no reason why this should fail with CLANG_USE_XCSELECT. - -// RUN: not %clang -ObjC -target i386-apple-darwin10 -stdlib=libstdc++ -m32 -fobjc-arc %s -fsyntax-only 2>&1 | FileCheck %s -// RUN: not %clang -x objective-c -target i386-apple-darwin10 -stdlib=libstdc++ -m32 -fobjc-arc %s -fsyntax-only 2>&1 | FileCheck %s -// RUN: not %clang -x objective-c++ -target i386-apple-darwin10 -stdlib=libstdc++ -m32 -fobjc-arc %s -fsyntax-only 2>&1 | FileCheck %s -// RUN: not %clang -x c -target i386-apple-darwin10 -stdlib=libstdc++ -m32 -fobjc-arc %s -fsyntax-only 2>&1 | FileCheck -check-prefix NOTOBJC %s -// RUN: not %clang -x c++ -target i386-apple-darwin10 -stdlib=libstdc++ -m32 -fobjc-arc %s -fsyntax-only 2>&1 | FileCheck -check-prefix NOTOBJC %s +// RUN: not %clang -ObjC -target i386-apple-macosx10.6 -stdlib=libstdc++ -m32 -fobjc-arc %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: not %clang -x objective-c -target i386-apple-macosx10.6 -stdlib=libstdc++ -m32 -fobjc-arc %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: not %clang -x objective-c++ -target i386-apple-macosx10.6 -stdlib=libstdc++ -m32 -fobjc-arc %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: not %clang -x c -target i386-apple-macosx10.6 -stdlib=libstdc++ -m32 -fobjc-arc %s -fsyntax-only 2>&1 | FileCheck -check-prefix NOTOBJC %s +// RUN: not %clang -x c++ -target i386-apple-macosx10.6 -stdlib=libstdc++ -m32 -fobjc-arc %s -fsyntax-only 2>&1 | FileCheck -check-prefix NOTOBJC %s // RUN: not %clang -x objective-c -target x86_64-apple-darwin11 -mmacos-version-min=10.5 -fobjc-arc %s -fsyntax-only 2>&1 | FileCheck -check-prefix NOTSUPPORTED %s // Just to test clang is working. ``` This change makes `arc.c` succeed. > Maybe don't use `xcselect` when it is darwin since the modern spelling should > `macOS` family? This is an option, but not the ideal one since the default target triple still uses the `-darwin` spelling. Though I suppose one can just change this at configure time. Is there any reason why default builds don't just use the macOS spelling, at least for modern versions of macOS? https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
https://github.com/carlocab edited https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
@@ -1,3 +1,6 @@ +// XFAIL: xcselect carlocab wrote: In theory, yes. In practice, it looks to me like all these failing test cases[^1] have deployment targets that pre-date macOS 10.14, so it seems very likely that anyone who can enable `CLANG_USE_XCSELECT` will probably see these tests fail as soon as they do. But, point taken. Do you have an alternative approach in mind? I'm wondering if it's worth reviving the `CLANG_NO_XCSELECT` environment variable and then just setting them in these tests (and slowly getting rid of instances of `CLANG_NO_XCSELECT` as we find better fixes). [^1]: I haven't checked each individually, so I'm not sure yet. https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
https://github.com/cachemeifyoucan commented: Let's investigate to see all those test failed (many of them have versioned deployment target). Maybe it is because the `darwin` legacy OS in the triple? Maybe don't use `xcselect` when it is darwin since the modern spelling should `macOS` family? https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
https://github.com/cachemeifyoucan edited https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
@@ -1,3 +1,6 @@ +// XFAIL: xcselect cachemeifyoucan wrote: I don't feel XFAIL is correct to use here in general. It is possible to have a different host SDK and the test will actually pass, right? https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
carlocab wrote: > > I don't quite understand why the failing tests are caused to fail when > > `CLANG_USE_XCSELECT` is enabled. I'll have to put more time into > > investigating them -- I consider this another blocker for enabling this > > feature more widely. > > I think the issue is that the implicit `-isysroot` flag from `libxcselect` > changes the deployment target, which then also changes the behaviour of > `clang` in these tests. But I'll need to verify this locally to make sure. Yea, this seems right. Adding appropriate `-mmacosx-version-min=` flags to the failing tests seems to make them succeed again. Not sure if that's the right move, though. https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
carlocab wrote: > I don't quite understand why the failing tests are caused to fail when > `CLANG_USE_XCSELECT` is enabled. I'll have to put more time into > investigating them -- I consider this another blocker for enabling this > feature more widely. I think the issue is that the implicit `-isysroot` flag from `libxcselect` changes the deployment target, which then also changes the behaviour of `clang` in these tests. But I'll need to verify this locally to make sure. https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
carlocab wrote: > Need to add test. You should add `CLANG_USE_XCSELECT` as a requirement and > label all the tests properly (including those will break). Maybe you can also > remove the hack for `CLANG_NO_XCSELECT` but I am neutral on that. All done, thanks. I don't quite understand why the failing tests are caused to fail when `CLANG_USE_XCSELECT` is enabled. I'll have to put more time into investigating them -- I consider this another blocker for enabling this feature more widely. https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
https://github.com/carlocab updated https://github.com/llvm/llvm-project/pull/119670 >From 9db768ec1be84d6bf647b85f052776720822f64f Mon Sep 17 00:00:00 2001 From: Carlo Cabrera Date: Tue, 10 Dec 2024 01:45:22 +0800 Subject: [PATCH 1/5] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK This is a scaled down version of https://reviews.llvm.org/D136315. The intent is largely the same as before[^1], but I've scaled down the scope to try to avoid the issues that the previous patch caused: - the changes are now opt-in based on enabling `CLANG_USE_XCSELECT` - this only works when targeting macOS on a macOS host (this is the only case supported by `libxcselect`[^2]) We also introduce an environment variable `CLANG_NO_XCSELECT` that disables this behaviour if Clang is configured with `CLANG_USE_XCSELECT=ON`. This is needed to avoid breaking tests. Another reason to leave this as opt-in for now is that there are some bugs in libxcselect that need fixing before it is safe to use by default for all users. This has been reported to Apple as FB16081077. [^1]: See also https://reviews.llvm.org/D109460 and #45225. [^2]: https://developer.apple.com/documentation/xcselect?language=objc --- clang/CMakeLists.txt | 33 +++ clang/include/clang/Config/config.h.cmake | 6 + clang/lib/Driver/CMakeLists.txt | 4 +++ clang/lib/Driver/ToolChains/Darwin.cpp| 31 ++--- clang/test/lit.cfg.py | 4 +++ 5 files changed, 69 insertions(+), 9 deletions(-) diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt index 27e8095534a65c..3c7096a27c509b 100644 --- a/clang/CMakeLists.txt +++ b/clang/CMakeLists.txt @@ -214,6 +214,39 @@ if(GCC_INSTALL_PREFIX AND NOT USE_DEPRECATED_GCC_INSTALL_PREFIX) "See https://github.com/llvm/llvm-project/pull/77537 for detail.") endif() +if(APPLE) + check_include_file(xcselect.h CLANG_HAVE_XCSELECT_H) + if(CLANG_HAVE_XCSELECT_H) +include(CheckSymbolExists) +list(APPEND CMAKE_REQUIRED_LIBRARIES xcselect) +check_symbol_exists(xcselect_host_sdk_path xcselect.h CLANG_HAVE_XCSELECT_HOST_SDK_PATH) +list(REMOVE_ITEM CMAKE_REQUIRED_LIBRARIES xcselect) + endif() +endif() + +cmake_dependent_option(CLANG_USE_XCSELECT "Use libxcselect to find the macOS SDK." OFF + "APPLE;CLANG_HAVE_XCSELECT_HOST_SDK_PATH" OFF) + +if(DEFAULT_SYSROOT AND CLANG_USE_XCSELECT) + message(FATAL_ERROR "Setting DEFAULT_SYSROOT is incompatible with CLANG_USE_XCSELECT.") +endif() + +if(CLANG_USE_XCSELECT) + set(XCSELECT_VALID_POLICIES LATEST MATCHING_ONLY MATCHING_PREFERRED) + set(CLANG_XCSELECT_HOST_SDK_POLICY "LATEST" CACHE STRING +"Policy to use for xcselect. One of: ${XCSELECT_VALID_POLICIES}") + set_property(CACHE CLANG_XCSELECT_HOST_SDK_POLICY PROPERTY STRINGS ${XCSELECT_VALID_POLICIES}) + string(TOUPPER ${CLANG_XCSELECT_HOST_SDK_POLICY} CLANG_XCSELECT_HOST_SDK_POLICY) + list(JOIN XCSELECT_VALID_POLICIES "|" XCSELECT_POLICY_REGEX) + if(NOT CLANG_XCSELECT_HOST_SDK_POLICY MATCHES "^XCSELECT_HOST_SDK_POLICY_(${XCSELECT_POLICY_REGEX})$") +if(NOT CLANG_XCSELECT_HOST_SDK_POLICY IN_LIST XCSELECT_VALID_POLICIES) + message(FATAL_ERROR +"CLANG_XCSELECT_HOST_SDK_POLICY (${CLANG_XCSELECT_HOST_SDK_POLICY}) must be one of: ${XCSELECT_VALID_POLICIES}") +endif() +set(CLANG_XCSELECT_HOST_SDK_POLICY "XCSELECT_HOST_SDK_POLICY_${CLANG_XCSELECT_HOST_SDK_POLICY}") + endif() +endif() + set(ENABLE_LINKER_BUILD_ID OFF CACHE BOOL "pass --build-id to ld") set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL diff --git a/clang/include/clang/Config/config.h.cmake b/clang/include/clang/Config/config.h.cmake index 27ed69e21562bf..9ae0c6a4b9e11d 100644 --- a/clang/include/clang/Config/config.h.cmake +++ b/clang/include/clang/Config/config.h.cmake @@ -86,4 +86,10 @@ /* Whether CIR is built into Clang */ #cmakedefine01 CLANG_ENABLE_CIR +/* Whether to use xcselect to find the macOS SDK */ +#cmakedefine CLANG_USE_XCSELECT + +/* Policy to use for xcselect */ +#cmakedefine CLANG_XCSELECT_HOST_SDK_POLICY ${CLANG_XCSELECT_HOST_SDK_POLICY} + #endif diff --git a/clang/lib/Driver/CMakeLists.txt b/clang/lib/Driver/CMakeLists.txt index 4fd10bf671512f..299de2ef30470c 100644 --- a/clang/lib/Driver/CMakeLists.txt +++ b/clang/lib/Driver/CMakeLists.txt @@ -14,6 +14,10 @@ if(WIN32) set(system_libs version) endif() +if(CLANG_USE_XCSELECT) + set(system_libs xcselect) +endif() + add_clang_library(clangDriver Action.cpp Compilation.cpp diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index 87380869f6fdab..8d3e0130a5a432 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -29,6 +29,10 @@ #include "llvm/TargetParser/Triple.h" #include // ::getenv +#ifdef CLANG_USE_XCSELECT +#include // ::xcselect_host_sdk_path +#endif + using namespace clang::driver; using namespace clang::driver::tools;
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
https://github.com/carlocab updated https://github.com/llvm/llvm-project/pull/119670 >From 9db768ec1be84d6bf647b85f052776720822f64f Mon Sep 17 00:00:00 2001 From: Carlo Cabrera Date: Tue, 10 Dec 2024 01:45:22 +0800 Subject: [PATCH 1/3] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK This is a scaled down version of https://reviews.llvm.org/D136315. The intent is largely the same as before[^1], but I've scaled down the scope to try to avoid the issues that the previous patch caused: - the changes are now opt-in based on enabling `CLANG_USE_XCSELECT` - this only works when targeting macOS on a macOS host (this is the only case supported by `libxcselect`[^2]) We also introduce an environment variable `CLANG_NO_XCSELECT` that disables this behaviour if Clang is configured with `CLANG_USE_XCSELECT=ON`. This is needed to avoid breaking tests. Another reason to leave this as opt-in for now is that there are some bugs in libxcselect that need fixing before it is safe to use by default for all users. This has been reported to Apple as FB16081077. [^1]: See also https://reviews.llvm.org/D109460 and #45225. [^2]: https://developer.apple.com/documentation/xcselect?language=objc --- clang/CMakeLists.txt | 33 +++ clang/include/clang/Config/config.h.cmake | 6 + clang/lib/Driver/CMakeLists.txt | 4 +++ clang/lib/Driver/ToolChains/Darwin.cpp| 31 ++--- clang/test/lit.cfg.py | 4 +++ 5 files changed, 69 insertions(+), 9 deletions(-) diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt index 27e8095534a65c..3c7096a27c509b 100644 --- a/clang/CMakeLists.txt +++ b/clang/CMakeLists.txt @@ -214,6 +214,39 @@ if(GCC_INSTALL_PREFIX AND NOT USE_DEPRECATED_GCC_INSTALL_PREFIX) "See https://github.com/llvm/llvm-project/pull/77537 for detail.") endif() +if(APPLE) + check_include_file(xcselect.h CLANG_HAVE_XCSELECT_H) + if(CLANG_HAVE_XCSELECT_H) +include(CheckSymbolExists) +list(APPEND CMAKE_REQUIRED_LIBRARIES xcselect) +check_symbol_exists(xcselect_host_sdk_path xcselect.h CLANG_HAVE_XCSELECT_HOST_SDK_PATH) +list(REMOVE_ITEM CMAKE_REQUIRED_LIBRARIES xcselect) + endif() +endif() + +cmake_dependent_option(CLANG_USE_XCSELECT "Use libxcselect to find the macOS SDK." OFF + "APPLE;CLANG_HAVE_XCSELECT_HOST_SDK_PATH" OFF) + +if(DEFAULT_SYSROOT AND CLANG_USE_XCSELECT) + message(FATAL_ERROR "Setting DEFAULT_SYSROOT is incompatible with CLANG_USE_XCSELECT.") +endif() + +if(CLANG_USE_XCSELECT) + set(XCSELECT_VALID_POLICIES LATEST MATCHING_ONLY MATCHING_PREFERRED) + set(CLANG_XCSELECT_HOST_SDK_POLICY "LATEST" CACHE STRING +"Policy to use for xcselect. One of: ${XCSELECT_VALID_POLICIES}") + set_property(CACHE CLANG_XCSELECT_HOST_SDK_POLICY PROPERTY STRINGS ${XCSELECT_VALID_POLICIES}) + string(TOUPPER ${CLANG_XCSELECT_HOST_SDK_POLICY} CLANG_XCSELECT_HOST_SDK_POLICY) + list(JOIN XCSELECT_VALID_POLICIES "|" XCSELECT_POLICY_REGEX) + if(NOT CLANG_XCSELECT_HOST_SDK_POLICY MATCHES "^XCSELECT_HOST_SDK_POLICY_(${XCSELECT_POLICY_REGEX})$") +if(NOT CLANG_XCSELECT_HOST_SDK_POLICY IN_LIST XCSELECT_VALID_POLICIES) + message(FATAL_ERROR +"CLANG_XCSELECT_HOST_SDK_POLICY (${CLANG_XCSELECT_HOST_SDK_POLICY}) must be one of: ${XCSELECT_VALID_POLICIES}") +endif() +set(CLANG_XCSELECT_HOST_SDK_POLICY "XCSELECT_HOST_SDK_POLICY_${CLANG_XCSELECT_HOST_SDK_POLICY}") + endif() +endif() + set(ENABLE_LINKER_BUILD_ID OFF CACHE BOOL "pass --build-id to ld") set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL diff --git a/clang/include/clang/Config/config.h.cmake b/clang/include/clang/Config/config.h.cmake index 27ed69e21562bf..9ae0c6a4b9e11d 100644 --- a/clang/include/clang/Config/config.h.cmake +++ b/clang/include/clang/Config/config.h.cmake @@ -86,4 +86,10 @@ /* Whether CIR is built into Clang */ #cmakedefine01 CLANG_ENABLE_CIR +/* Whether to use xcselect to find the macOS SDK */ +#cmakedefine CLANG_USE_XCSELECT + +/* Policy to use for xcselect */ +#cmakedefine CLANG_XCSELECT_HOST_SDK_POLICY ${CLANG_XCSELECT_HOST_SDK_POLICY} + #endif diff --git a/clang/lib/Driver/CMakeLists.txt b/clang/lib/Driver/CMakeLists.txt index 4fd10bf671512f..299de2ef30470c 100644 --- a/clang/lib/Driver/CMakeLists.txt +++ b/clang/lib/Driver/CMakeLists.txt @@ -14,6 +14,10 @@ if(WIN32) set(system_libs version) endif() +if(CLANG_USE_XCSELECT) + set(system_libs xcselect) +endif() + add_clang_library(clangDriver Action.cpp Compilation.cpp diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index 87380869f6fdab..8d3e0130a5a432 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -29,6 +29,10 @@ #include "llvm/TargetParser/Triple.h" #include // ::getenv +#ifdef CLANG_USE_XCSELECT +#include // ::xcselect_host_sdk_path +#endif + using namespace clang::driver; using namespace clang::driver::tools;
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
@@ -2257,17 +2261,26 @@ void Darwin::AddDeploymentTarget(DerivedArgList &Args) const { // Warn if the path does not exist. if (!getVFS().exists(A->getValue())) getDriver().Diag(clang::diag::warn_missing_sysroot) << A->getValue(); - } else { -if (char *env = ::getenv("SDKROOT")) { - // We only use this value as the default if it is an absolute path, - // exists, and it is not the root path. - if (llvm::sys::path::is_absolute(env) && getVFS().exists(env) && - StringRef(env) != "/") { -Args.append(Args.MakeSeparateArg( -nullptr, Opts.getOption(options::OPT_isysroot), env)); - } + } else if (const char *env = ::getenv("SDKROOT"); env && *env) { +// We only use this value as the default if it is an absolute path, +// exists, and it is not the root path. +if (llvm::sys::path::is_absolute(env) && getVFS().exists(env) && +StringRef(env) != "/") { + Args.append(Args.MakeSeparateArg( + nullptr, Opts.getOption(options::OPT_isysroot), env)); } } +#ifdef CLANG_USE_XCSELECT + else if (const char *env = ::getenv("CLANG_NO_XCSELECT"); cachemeifyoucan wrote: Don't write if conditions with multiple statements. Same as below. https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
https://github.com/cachemeifyoucan commented: The idea looks good. Need to add test. You should add `CLANG_USE_XCSELECT` as a requirement and label all the tests properly (including those will break). Maybe you can also remove the hack for `CLANG_NO_XCSELECT` but I am neutral on that. https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
@@ -214,6 +214,39 @@ if(GCC_INSTALL_PREFIX AND NOT USE_DEPRECATED_GCC_INSTALL_PREFIX) "See https://github.com/llvm/llvm-project/pull/77537 for detail.") endif() +if(APPLE) + check_include_file(xcselect.h CLANG_HAVE_XCSELECT_H) cachemeifyoucan wrote: All the configuration in this block should only be done if `CLANG_USE_XCSELECT` is enabled. https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
https://github.com/cachemeifyoucan edited https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
carlocab wrote: > We also introduce an environment variable `CLANG_NO_XCSELECT` that disables > this behaviour if Clang is configured with `CLANG_USE_XCSELECT=ON`. This is > needed to avoid breaking tests. It might also be nicer to set this only for the tests that break instead of globally. https://github.com/llvm/llvm-project/pull/119670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
llvmbot wrote: @llvm/pr-subscribers-clang-driver Author: Carlo Cabrera (carlocab) Changes This is a scaled down version of https://reviews.llvm.org/D136315. The intent is largely the same as before[^1], but I've scaled down the scope to try to avoid the issues that the previous patch caused: - the changes are now opt-in based on enabling `CLANG_USE_XCSELECT` - this only works when targeting macOS on a macOS host (this is the only case supported by `libxcselect`[^2]) We also introduce an environment variable `CLANG_NO_XCSELECT` that disables this behaviour if Clang is configured with `CLANG_USE_XCSELECT=ON`. This is needed to avoid breaking tests. Another reason to leave this as opt-in for now is that there are some bugs in libxcselect that need fixing before it is safe to use by default for all users. This has been reported to Apple as FB16081077. [^1]: See also https://reviews.llvm.org/D109460 and #45225. [^2]: https://developer.apple.com/documentation/xcselect?language=objc --- Full diff: https://github.com/llvm/llvm-project/pull/119670.diff 5 Files Affected: - (modified) clang/CMakeLists.txt (+33) - (modified) clang/include/clang/Config/config.h.cmake (+6) - (modified) clang/lib/Driver/CMakeLists.txt (+4) - (modified) clang/lib/Driver/ToolChains/Darwin.cpp (+22-9) - (modified) clang/test/lit.cfg.py (+4) ``diff diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt index 27e8095534a65c..3c7096a27c509b 100644 --- a/clang/CMakeLists.txt +++ b/clang/CMakeLists.txt @@ -214,6 +214,39 @@ if(GCC_INSTALL_PREFIX AND NOT USE_DEPRECATED_GCC_INSTALL_PREFIX) "See https://github.com/llvm/llvm-project/pull/77537 for detail.") endif() +if(APPLE) + check_include_file(xcselect.h CLANG_HAVE_XCSELECT_H) + if(CLANG_HAVE_XCSELECT_H) +include(CheckSymbolExists) +list(APPEND CMAKE_REQUIRED_LIBRARIES xcselect) +check_symbol_exists(xcselect_host_sdk_path xcselect.h CLANG_HAVE_XCSELECT_HOST_SDK_PATH) +list(REMOVE_ITEM CMAKE_REQUIRED_LIBRARIES xcselect) + endif() +endif() + +cmake_dependent_option(CLANG_USE_XCSELECT "Use libxcselect to find the macOS SDK." OFF + "APPLE;CLANG_HAVE_XCSELECT_HOST_SDK_PATH" OFF) + +if(DEFAULT_SYSROOT AND CLANG_USE_XCSELECT) + message(FATAL_ERROR "Setting DEFAULT_SYSROOT is incompatible with CLANG_USE_XCSELECT.") +endif() + +if(CLANG_USE_XCSELECT) + set(XCSELECT_VALID_POLICIES LATEST MATCHING_ONLY MATCHING_PREFERRED) + set(CLANG_XCSELECT_HOST_SDK_POLICY "LATEST" CACHE STRING +"Policy to use for xcselect. One of: ${XCSELECT_VALID_POLICIES}") + set_property(CACHE CLANG_XCSELECT_HOST_SDK_POLICY PROPERTY STRINGS ${XCSELECT_VALID_POLICIES}) + string(TOUPPER ${CLANG_XCSELECT_HOST_SDK_POLICY} CLANG_XCSELECT_HOST_SDK_POLICY) + list(JOIN XCSELECT_VALID_POLICIES "|" XCSELECT_POLICY_REGEX) + if(NOT CLANG_XCSELECT_HOST_SDK_POLICY MATCHES "^XCSELECT_HOST_SDK_POLICY_(${XCSELECT_POLICY_REGEX})$") +if(NOT CLANG_XCSELECT_HOST_SDK_POLICY IN_LIST XCSELECT_VALID_POLICIES) + message(FATAL_ERROR +"CLANG_XCSELECT_HOST_SDK_POLICY (${CLANG_XCSELECT_HOST_SDK_POLICY}) must be one of: ${XCSELECT_VALID_POLICIES}") +endif() +set(CLANG_XCSELECT_HOST_SDK_POLICY "XCSELECT_HOST_SDK_POLICY_${CLANG_XCSELECT_HOST_SDK_POLICY}") + endif() +endif() + set(ENABLE_LINKER_BUILD_ID OFF CACHE BOOL "pass --build-id to ld") set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL diff --git a/clang/include/clang/Config/config.h.cmake b/clang/include/clang/Config/config.h.cmake index 27ed69e21562bf..9ae0c6a4b9e11d 100644 --- a/clang/include/clang/Config/config.h.cmake +++ b/clang/include/clang/Config/config.h.cmake @@ -86,4 +86,10 @@ /* Whether CIR is built into Clang */ #cmakedefine01 CLANG_ENABLE_CIR +/* Whether to use xcselect to find the macOS SDK */ +#cmakedefine CLANG_USE_XCSELECT + +/* Policy to use for xcselect */ +#cmakedefine CLANG_XCSELECT_HOST_SDK_POLICY ${CLANG_XCSELECT_HOST_SDK_POLICY} + #endif diff --git a/clang/lib/Driver/CMakeLists.txt b/clang/lib/Driver/CMakeLists.txt index 4fd10bf671512f..299de2ef30470c 100644 --- a/clang/lib/Driver/CMakeLists.txt +++ b/clang/lib/Driver/CMakeLists.txt @@ -14,6 +14,10 @@ if(WIN32) set(system_libs version) endif() +if(CLANG_USE_XCSELECT) + set(system_libs xcselect) +endif() + add_clang_library(clangDriver Action.cpp Compilation.cpp diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index 87380869f6fdab..8d3e0130a5a432 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -29,6 +29,10 @@ #include "llvm/TargetParser/Triple.h" #include // ::getenv +#ifdef CLANG_USE_XCSELECT +#include // ::xcselect_host_sdk_path +#endif + using namespace clang::driver; using namespace clang::driver::tools; using namespace clang::driver::toolchains; @@ -2257,17 +2261,26 @@ void Darwin::AddDeploymentTarget(DerivedArgList &Args) const { // Warn if the path does not exist. if (!getVFS().exists
[clang] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK (PR #119670)
https://github.com/carlocab created https://github.com/llvm/llvm-project/pull/119670 This is a scaled down version of https://reviews.llvm.org/D136315. The intent is largely the same as before[^1], but I've scaled down the scope to try to avoid the issues that the previous patch caused: - the changes are now opt-in based on enabling `CLANG_USE_XCSELECT` - this only works when targeting macOS on a macOS host (this is the only case supported by `libxcselect`[^2]) We also introduce an environment variable `CLANG_NO_XCSELECT` that disables this behaviour if Clang is configured with `CLANG_USE_XCSELECT=ON`. This is needed to avoid breaking tests. Another reason to leave this as opt-in for now is that there are some bugs in libxcselect that need fixing before it is safe to use by default for all users. This has been reported to Apple as FB16081077. [^1]: See also https://reviews.llvm.org/D109460 and #45225. [^2]: https://developer.apple.com/documentation/xcselect?language=objc >From 9db768ec1be84d6bf647b85f052776720822f64f Mon Sep 17 00:00:00 2001 From: Carlo Cabrera Date: Tue, 10 Dec 2024 01:45:22 +0800 Subject: [PATCH] [clang][Driver][Darwin] Optionally use xcselect to find macOS SDK This is a scaled down version of https://reviews.llvm.org/D136315. The intent is largely the same as before[^1], but I've scaled down the scope to try to avoid the issues that the previous patch caused: - the changes are now opt-in based on enabling `CLANG_USE_XCSELECT` - this only works when targeting macOS on a macOS host (this is the only case supported by `libxcselect`[^2]) We also introduce an environment variable `CLANG_NO_XCSELECT` that disables this behaviour if Clang is configured with `CLANG_USE_XCSELECT=ON`. This is needed to avoid breaking tests. Another reason to leave this as opt-in for now is that there are some bugs in libxcselect that need fixing before it is safe to use by default for all users. This has been reported to Apple as FB16081077. [^1]: See also https://reviews.llvm.org/D109460 and #45225. [^2]: https://developer.apple.com/documentation/xcselect?language=objc --- clang/CMakeLists.txt | 33 +++ clang/include/clang/Config/config.h.cmake | 6 + clang/lib/Driver/CMakeLists.txt | 4 +++ clang/lib/Driver/ToolChains/Darwin.cpp| 31 ++--- clang/test/lit.cfg.py | 4 +++ 5 files changed, 69 insertions(+), 9 deletions(-) diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt index 27e8095534a65c..3c7096a27c509b 100644 --- a/clang/CMakeLists.txt +++ b/clang/CMakeLists.txt @@ -214,6 +214,39 @@ if(GCC_INSTALL_PREFIX AND NOT USE_DEPRECATED_GCC_INSTALL_PREFIX) "See https://github.com/llvm/llvm-project/pull/77537 for detail.") endif() +if(APPLE) + check_include_file(xcselect.h CLANG_HAVE_XCSELECT_H) + if(CLANG_HAVE_XCSELECT_H) +include(CheckSymbolExists) +list(APPEND CMAKE_REQUIRED_LIBRARIES xcselect) +check_symbol_exists(xcselect_host_sdk_path xcselect.h CLANG_HAVE_XCSELECT_HOST_SDK_PATH) +list(REMOVE_ITEM CMAKE_REQUIRED_LIBRARIES xcselect) + endif() +endif() + +cmake_dependent_option(CLANG_USE_XCSELECT "Use libxcselect to find the macOS SDK." OFF + "APPLE;CLANG_HAVE_XCSELECT_HOST_SDK_PATH" OFF) + +if(DEFAULT_SYSROOT AND CLANG_USE_XCSELECT) + message(FATAL_ERROR "Setting DEFAULT_SYSROOT is incompatible with CLANG_USE_XCSELECT.") +endif() + +if(CLANG_USE_XCSELECT) + set(XCSELECT_VALID_POLICIES LATEST MATCHING_ONLY MATCHING_PREFERRED) + set(CLANG_XCSELECT_HOST_SDK_POLICY "LATEST" CACHE STRING +"Policy to use for xcselect. One of: ${XCSELECT_VALID_POLICIES}") + set_property(CACHE CLANG_XCSELECT_HOST_SDK_POLICY PROPERTY STRINGS ${XCSELECT_VALID_POLICIES}) + string(TOUPPER ${CLANG_XCSELECT_HOST_SDK_POLICY} CLANG_XCSELECT_HOST_SDK_POLICY) + list(JOIN XCSELECT_VALID_POLICIES "|" XCSELECT_POLICY_REGEX) + if(NOT CLANG_XCSELECT_HOST_SDK_POLICY MATCHES "^XCSELECT_HOST_SDK_POLICY_(${XCSELECT_POLICY_REGEX})$") +if(NOT CLANG_XCSELECT_HOST_SDK_POLICY IN_LIST XCSELECT_VALID_POLICIES) + message(FATAL_ERROR +"CLANG_XCSELECT_HOST_SDK_POLICY (${CLANG_XCSELECT_HOST_SDK_POLICY}) must be one of: ${XCSELECT_VALID_POLICIES}") +endif() +set(CLANG_XCSELECT_HOST_SDK_POLICY "XCSELECT_HOST_SDK_POLICY_${CLANG_XCSELECT_HOST_SDK_POLICY}") + endif() +endif() + set(ENABLE_LINKER_BUILD_ID OFF CACHE BOOL "pass --build-id to ld") set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL diff --git a/clang/include/clang/Config/config.h.cmake b/clang/include/clang/Config/config.h.cmake index 27ed69e21562bf..9ae0c6a4b9e11d 100644 --- a/clang/include/clang/Config/config.h.cmake +++ b/clang/include/clang/Config/config.h.cmake @@ -86,4 +86,10 @@ /* Whether CIR is built into Clang */ #cmakedefine01 CLANG_ENABLE_CIR +/* Whether to use xcselect to find the macOS SDK */ +#cmakedefine CLANG_USE_XCSELECT + +/* Policy to use for xcselect */ +#cmakedefine CLANG_XCSELECT_HOST_