[libcxx] [libcxxabi] [libunwind] [runtimes] Remove explicit -isysroot from the testing configurations on macOS (PR #66265)
ldionne wrote: Not pursuing anymore, closing to clean up the queue. https://github.com/llvm/llvm-project/pull/66265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] [libcxxabi] [libunwind] [runtimes] Remove explicit -isysroot from the testing configurations on macOS (PR #66265)
https://github.com/ldionne closed https://github.com/llvm/llvm-project/pull/66265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [runtimes] Remove explicit -isysroot from the testing configurations on macOS (PR #66265)
mstorsjo wrote: > But even regardless of #67201, the intended way to use the libc++ test suite > is to create a `.cfg.in` file that suits your needs if the general ones > don't, not to add more configuration options to the general ones. IMO, I think this is something I fundamentally disagree with. Yes, the previous test setup with long intricate python classes was probably too messy/complex. But the other extreme - only ever having entirely static `.cfg` files isn't the best either (if that's required, I'm quite sure there'll be cases where people just skip caring about running the tests, rather than adding a config). Some level of configurability for allowing injecting compiler flags in the existing test configs would be very welcome, IMO. It shouldn't evolve into the old mess of course, but just allowing adding something like `LIBCXX_TEST_CXXFLAGS`, for adding flags specifically to the test runs, would be quite useful for some test configs I would expect. (I currently don't have a need for the sysroot stuff touched upon here, but I get the demand for it.) E.g. kinda like building libc++ with `-std=c++23` but testing compiling the user code with `-std=c++17`. I guess that particular case is supported via some of the testsuite parameters, but users may want to build/test in other configurations as well - and as long as it's fundamentally the same as existing configs, having to create a new test config for it feels very much unnecessary. https://github.com/llvm/llvm-project/pull/66265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [runtimes] Remove explicit -isysroot from the testing configurations on macOS (PR #66265)
petrhosek wrote: > How do you build on Apple platforms then? Don't you need access to a SDK? I'm > a bit confused about what you're doing -- do you need to use the native > platform SDK when you're building Fuchsia? What SDK are you passing as > `--sysroot` when testing libc++? We use `-isysroot` with the Xcode SDK. That doesn't require selecting active developer directory. You only need to select active developer directory if you are going to run binaries from the Xcode SDK (or other binaries that assume so like `xcrun`) which we don't since all our tools come from LLVM. In the future, we hope to switch to LLVM libc on all platforms to make our build truly hermetic. At that point, we won't be using Xcode SDK at all and at that point forcing the use of `xcrun` is going to be even more undesirable. > I know that, but as I already explained over in > https://reviews.llvm.org/D151056, making everyone's config more complicated > isn't the path we want to take. We should never have been setting the > sysroot, it was only a workaround for > https://gitlab.kitware.com/cmake/cmake/-/issues/19180 and that should have > been done another way initially. > > These config files were designed to be really simple and to be easily created > for custom needs. This was an answer to the incredible amount of > platform-specific complexity that the previous test configuration system had, > and what I'm doing right now is push back to avoid going down that path again. That's reasonable, but these configs still implicitly encode assumptions, such as the fact your host environment is usable for building binaries. For example, if you don't specify `--sysroot=`, the compiler will use `/`, but if your host sysroot doesn't have the necessary files (i.e. headers and libraries), building tests will fail. In our case, the fact that the host environment isn't usable for building binaries is intentional, we always build against an explicit sysroot (using `--sysroot`, `-isysroot` or `/winsysroot` depending on a platform) to ensure our build is hermetic as was suggested in https://blog.llvm.org/2019/11/deterministic-builds-with-clang-and-lld.html, see the "Getting to universal determinism" which says: > Getting to universal determinism > You then need to modify your build files to use `--sysroot` (Linux), > `-isysroot` (macOS), `-imsvc` (Windows) to use these hermetic SDKs for > builds. They need to be somewhere below your source root to not regress build > directory name invariance. > You also want to make sure your build doesn’t depend on environment > variables, as already mentioned in the “Getting to incremental determinism”, > since environments between different machines can be very different and > difficult to control. > Is there a reason why you don't have a `fuchsia-*.cfg.in` configuration file? We plan to introduce one, but this issue is not about building and running tests on Fuchsia, the issue is building and running tests for other platforms we build and distribute runtimes for like Linux, macOS and Windows. > > We also need to restrict the `xcrun` change only to `apple-*.cfg.in` > > configurations. > > No, it is needed whenever the build host is an Apple platform, which is more > general than the `apple-*.cfg.in` configurations. The normal LLVM > configuration also needs to run under xcrun when it is being built on macOS, > cause that's how you get to the SDK. You seem to be assuming there's a single way to build the binaries for any given platform, but that's generally not the case: * On macOS, you can either (1) run the binary under `xcrun`, which sets the `SDKROOT` environment variable, or you can (2) use the `-isysroot` flag. These can be used interchangeably, see: https://github.com/llvm/llvm-project/blob/5d86176f4868771527e2b7469c1bc87f09c96f0a/clang/lib/Driver/ToolChains/Darwin.cpp#L2144-L2161 * On Windows, you can (1) set the environment variables by running the `vcvars*.bat` file that's provided by VS SDK, or you can (2) use the `/winsysroot` flag which was created to simplify using hermetic SDK and avoid relying on environment variables: https://reviews.llvm.org/D95534 I'm fine choosing picking defaults, like using `xcrun`, but I don't think we should be forcing those onto all users with no way to opt out as is currently the case in this patch. > I will land this tomorrow unless there is additional discussion. It looks > like #67201 would allow you to solve your problem without creating a custom > `.cfg.in` file, which is great. But even regardless of #67201, the intended > way to use the libc++ test suite is to create a `.cfg.in` file that suits > your needs if the general ones don't, not to add more configuration options > to the general ones. #67201 is an improvement but is not an idiomatic CMake. The idiomatic way to set sysroot in CMake is through `CMAKE_SYSROOT=/path/to/sysroot`, not `CMAKE_CXX_FLAGS_INIT="--sysroot=/path/to/sysroot"`.
[libunwind] [runtimes] Remove explicit -isysroot from the testing configurations on macOS (PR #66265)
petrhosek wrote: > @petrhosek Please take a look -- this is passing CI and I believe this should > support your use case. And it's definitely the way these configuration files > should have been set up from the start, I guess I just didn't think of doing > it that way when I hit the sysroot issue back in the days. This doesn't address our use case and would in fact break our infrastructure. We always build using an explicit sysroot that's deployed as part of the CI job (in other words, the default sysroot `/` is not usable for building binaries) and we need to pass a path to the sysroot we're using to the compiler through the `--sysroot` flag (or `-isysroot` flag on Darwin) which is what https://reviews.llvm.org/D151056 implements. In case you're wondering "well how come you're not broken right now since https://reviews.llvm.org/D151056 hasn't landed", the answer is that we an ugly hack... err workaround which is only needed for libunwind, libc++abi and libc++ and we would like to remove it as part for [SLSA](https://slsa.dev/) adoption. We also cannot run the compiler on Darwin through `xcrun` since `xcrun` requires selecting an active developer directory (that is running `xcode-select`) and that's not something we do on our bots; doing so would be a significant change since running `xcode-select` requires root privileges and would break our CI model. I'd prefer landing https://reviews.llvm.org/D151056, which would fully address our use cases and I'm not aware of any downsides. Alternatively, we could create a separate test configuration that would be effectively what's in https://reviews.llvm.org/D151056. We also need to restrict the `xcrun` change only to `apple-*.cfg.in` configurations. https://github.com/llvm/llvm-project/pull/66265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [runtimes] Remove explicit -isysroot from the testing configurations on macOS (PR #66265)
https://github.com/ldionne updated
https://github.com/llvm/llvm-project/pull/66265:
>From dfca35edb3eb6c0cd2e7d3292f854dcd9bdc6cb7 Mon Sep 17 00:00:00 2001
From: Louis Dionne
Date: Wed, 13 Sep 2023 13:30:30 -0400
Subject: [PATCH] [runtimes] Remove explicit -isysroot from the testing
configurations on macOS
This shouldn't be necessary if we set the SDKROOT environment variable
properly when we run the test suite, as it is expected on Apple platforms.
To set the SDKROOT environment variable properly, we have to use a slight
hack to work around a CMake issue [1], but at least it makes the hack
self contained and it avoids polluting all the testing configs.
This change was prompted by https://reviews.llvm.org/D151056.
[1]: https://gitlab.kitware.com/cmake/cmake/-/issues/19180
---
.../configs/apple-libc++-backdeployment.cfg.in| 4 +---
libcxx/test/configs/apple-libc++-shared.cfg.in| 4 +---
libcxx/test/configs/cmake-bridge.cfg.in | 6 +++---
libcxx/test/configs/llvm-libc++-shared.cfg.in | 4 +---
libcxx/test/configs/llvm-libc++-static.cfg.in | 4 +---
libcxx/utils/libcxx/test/format.py| 15 +++
.../configs/apple-libc++abi-backdeployment.cfg.in | 4 +---
.../test/configs/apple-libc++abi-shared.cfg.in| 4 +---
libcxxabi/test/configs/cmake-bridge.cfg.in| 5 +++--
.../test/configs/llvm-libc++abi-merged.cfg.in | 4 +---
.../test/configs/llvm-libc++abi-shared.cfg.in | 4 +---
.../test/configs/llvm-libc++abi-static.cfg.in | 4 +---
.../configs/apple-libunwind-backdeployment.cfg.in | 4 +---
libunwind/test/configs/cmake-bridge.cfg.in| 6 +++---
.../test/configs/llvm-libunwind-merged.cfg.in | 5 +
.../test/configs/llvm-libunwind-shared.cfg.in | 5 +
.../test/configs/llvm-libunwind-static.cfg.in | 5 +
17 files changed, 37 insertions(+), 50 deletions(-)
diff --git a/libcxx/test/configs/apple-libc++-backdeployment.cfg.in
b/libcxx/test/configs/apple-libc++-backdeployment.cfg.in
index b471c02709c060f..85fd5fee7ba1ad3 100644
--- a/libcxx/test/configs/apple-libc++-backdeployment.cfg.in
+++ b/libcxx/test/configs/apple-libc++-backdeployment.cfg.in
@@ -41,9 +41,7 @@ BACKDEPLOYMENT_PARAMETERS = [
"""),
]
-config.substitutions.append(('%{flags}',
-'-isysroot {}'.format('@CMAKE_OSX_SYSROOT@') if '@CMAKE_OSX_SYSROOT@' else
''
-))
+config.substitutions.append(('%{flags}', ''))
config.substitutions.append(('%{compile_flags}',
'-nostdinc++ -I %{include} -I %{libcxx}/test/support'
))
diff --git a/libcxx/test/configs/apple-libc++-shared.cfg.in
b/libcxx/test/configs/apple-libc++-shared.cfg.in
index af1926e3859b9e5..165469c674900eb 100644
--- a/libcxx/test/configs/apple-libc++-shared.cfg.in
+++ b/libcxx/test/configs/apple-libc++-shared.cfg.in
@@ -9,9 +9,7 @@
lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
-config.substitutions.append(('%{flags}',
-'-isysroot {}'.format('@CMAKE_OSX_SYSROOT@') if '@CMAKE_OSX_SYSROOT@' else
''
-))
+config.substitutions.append(('%{flags}', ''))
config.substitutions.append(('%{compile_flags}',
'-nostdinc++ -I %{include} -I %{libcxx}/test/support'
))
diff --git a/libcxx/test/configs/cmake-bridge.cfg.in
b/libcxx/test/configs/cmake-bridge.cfg.in
index 9067115598abd8a..5004c78cb3d834a 100644
--- a/libcxx/test/configs/cmake-bridge.cfg.in
+++ b/libcxx/test/configs/cmake-bridge.cfg.in
@@ -11,7 +11,7 @@
# loading the file and then setting up the remaining Lit substitutions.
#
-import os, site
+import os, site, shlex
site.addsitedir(os.path.join('@LIBCXX_SOURCE_DIR@', 'utils'))
import libcxx.test.format
@@ -23,8 +23,8 @@ config.recursiveExpansionLimit = 10
config.test_exec_root = os.path.join('@CMAKE_BINARY_DIR@', 'test')
# Add substitutions for bootstrapping the test suite configuration
-import shlex
-config.substitutions.append(('%{cxx}', shlex.quote('@CMAKE_CXX_COMPILER@')))
+cxx =
libcxx.test.format.cmake_workaround_for_cxx_on_Apple(shlex.quote('@CMAKE_CXX_COMPILER@'))
+config.substitutions.append(('%{cxx}', cxx))
config.substitutions.append(('%{libcxx}', '@LIBCXX_SOURCE_DIR@'))
config.substitutions.append(('%{include}', '@LIBCXX_GENERATED_INCLUDE_DIR@'))
config.substitutions.append(('%{target-include}',
'@LIBCXX_GENERATED_INCLUDE_TARGET_DIR@'))
diff --git a/libcxx/test/configs/llvm-libc++-shared.cfg.in
b/libcxx/test/configs/llvm-libc++-shared.cfg.in
index 143b3b3feae1109..41450d78060e8aa 100644
--- a/libcxx/test/configs/llvm-libc++-shared.cfg.in
+++ b/libcxx/test/configs/llvm-libc++-shared.cfg.in
@@ -3,9 +3,7 @@
lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
-config.substitutions.append(('%{flags}',
-'-pthread' + (' -isysroot {}'.format('@CMAKE_OSX_SYSROOT@') if
'@CMAKE_OSX_SYSROOT@' else '')
-))
+config.substitutions.append(('%{flags}', '-pthread'))
config.substitutions.append(('%{compile_flags}',
'-nostdinc++ -I %{include} -I
[libunwind] [runtimes] Remove explicit -isysroot from the testing configurations on macOS (PR #66265)
https://github.com/ldionne updated
https://github.com/llvm/llvm-project/pull/66265:
>From c325c0ba95e6d6abe7ff1a371853051bab0510af Mon Sep 17 00:00:00 2001
From: Louis Dionne
Date: Wed, 13 Sep 2023 13:30:30 -0400
Subject: [PATCH] [runtimes] Remove explicit -isysroot from the testing
configurations on macOS
This shouldn't be necessary if we set the SDKROOT environment variable
properly when we run the test suite, as it is expected on Apple platforms.
To set the SDKROOT environment variable properly, we have to use a slight
hack to work around a CMake issue [1], but at least it makes the hack
self contained and it avoids polluting all the testing configs.
This change was prompted by https://reviews.llvm.org/D151056.
[1]: https://gitlab.kitware.com/cmake/cmake/-/issues/19180
---
.../configs/apple-libc++-backdeployment.cfg.in| 4 +---
libcxx/test/configs/apple-libc++-shared.cfg.in| 4 +---
libcxx/test/configs/cmake-bridge.cfg.in | 6 +++---
libcxx/test/configs/llvm-libc++-shared.cfg.in | 4 +---
libcxx/test/configs/llvm-libc++-static.cfg.in | 4 +---
libcxx/utils/libcxx/test/format.py| 15 +++
.../configs/apple-libc++abi-backdeployment.cfg.in | 4 +---
.../test/configs/apple-libc++abi-shared.cfg.in| 4 +---
libcxxabi/test/configs/cmake-bridge.cfg.in| 5 +++--
.../test/configs/llvm-libc++abi-merged.cfg.in | 4 +---
.../test/configs/llvm-libc++abi-shared.cfg.in | 4 +---
.../test/configs/llvm-libc++abi-static.cfg.in | 4 +---
.../configs/apple-libunwind-backdeployment.cfg.in | 4 +---
libunwind/test/configs/cmake-bridge.cfg.in| 6 +++---
.../test/configs/llvm-libunwind-merged.cfg.in | 5 +
.../test/configs/llvm-libunwind-shared.cfg.in | 5 +
.../test/configs/llvm-libunwind-static.cfg.in | 5 +
17 files changed, 37 insertions(+), 50 deletions(-)
diff --git a/libcxx/test/configs/apple-libc++-backdeployment.cfg.in
b/libcxx/test/configs/apple-libc++-backdeployment.cfg.in
index b471c02709c060f..85fd5fee7ba1ad3 100644
--- a/libcxx/test/configs/apple-libc++-backdeployment.cfg.in
+++ b/libcxx/test/configs/apple-libc++-backdeployment.cfg.in
@@ -41,9 +41,7 @@ BACKDEPLOYMENT_PARAMETERS = [
"""),
]
-config.substitutions.append(('%{flags}',
-'-isysroot {}'.format('@CMAKE_OSX_SYSROOT@') if '@CMAKE_OSX_SYSROOT@' else
''
-))
+config.substitutions.append(('%{flags}', ''))
config.substitutions.append(('%{compile_flags}',
'-nostdinc++ -I %{include} -I %{libcxx}/test/support'
))
diff --git a/libcxx/test/configs/apple-libc++-shared.cfg.in
b/libcxx/test/configs/apple-libc++-shared.cfg.in
index af1926e3859b9e5..165469c674900eb 100644
--- a/libcxx/test/configs/apple-libc++-shared.cfg.in
+++ b/libcxx/test/configs/apple-libc++-shared.cfg.in
@@ -9,9 +9,7 @@
lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
-config.substitutions.append(('%{flags}',
-'-isysroot {}'.format('@CMAKE_OSX_SYSROOT@') if '@CMAKE_OSX_SYSROOT@' else
''
-))
+config.substitutions.append(('%{flags}', ''))
config.substitutions.append(('%{compile_flags}',
'-nostdinc++ -I %{include} -I %{libcxx}/test/support'
))
diff --git a/libcxx/test/configs/cmake-bridge.cfg.in
b/libcxx/test/configs/cmake-bridge.cfg.in
index 9067115598abd8a..5004c78cb3d834a 100644
--- a/libcxx/test/configs/cmake-bridge.cfg.in
+++ b/libcxx/test/configs/cmake-bridge.cfg.in
@@ -11,7 +11,7 @@
# loading the file and then setting up the remaining Lit substitutions.
#
-import os, site
+import os, site, shlex
site.addsitedir(os.path.join('@LIBCXX_SOURCE_DIR@', 'utils'))
import libcxx.test.format
@@ -23,8 +23,8 @@ config.recursiveExpansionLimit = 10
config.test_exec_root = os.path.join('@CMAKE_BINARY_DIR@', 'test')
# Add substitutions for bootstrapping the test suite configuration
-import shlex
-config.substitutions.append(('%{cxx}', shlex.quote('@CMAKE_CXX_COMPILER@')))
+cxx =
libcxx.test.format.cmake_workaround_for_cxx_on_Apple(shlex.quote('@CMAKE_CXX_COMPILER@'))
+config.substitutions.append(('%{cxx}', cxx))
config.substitutions.append(('%{libcxx}', '@LIBCXX_SOURCE_DIR@'))
config.substitutions.append(('%{include}', '@LIBCXX_GENERATED_INCLUDE_DIR@'))
config.substitutions.append(('%{target-include}',
'@LIBCXX_GENERATED_INCLUDE_TARGET_DIR@'))
diff --git a/libcxx/test/configs/llvm-libc++-shared.cfg.in
b/libcxx/test/configs/llvm-libc++-shared.cfg.in
index 143b3b3feae1109..41450d78060e8aa 100644
--- a/libcxx/test/configs/llvm-libc++-shared.cfg.in
+++ b/libcxx/test/configs/llvm-libc++-shared.cfg.in
@@ -3,9 +3,7 @@
lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
-config.substitutions.append(('%{flags}',
-'-pthread' + (' -isysroot {}'.format('@CMAKE_OSX_SYSROOT@') if
'@CMAKE_OSX_SYSROOT@' else '')
-))
+config.substitutions.append(('%{flags}', '-pthread'))
config.substitutions.append(('%{compile_flags}',
'-nostdinc++ -I %{include} -I
[libunwind] [runtimes] Remove explicit -isysroot from the testing configurations on macOS (PR #66265)
@@ -2,6 +2,24 @@ @SERIALIZED_LIT_PARAMS@ +# Workaround for https://gitlab.kitware.com/cmake/cmake/-/issues/19180. +# +# On Apple platforms, CMake resolves the compiler to the final binary using +# `$(xcrun --find `). Unfortunately, running this compiler as-is +# doesn't work because it doesn't have knowledge of the SDK root. To work +# around that, we transform the resolved compiler back to the corresponding +# /usr/bin shim if we can. +def workaround_cmake_issue_on_Apple(cxx): ldionne wrote: This workaround doesn't change anything non-Apple platforms. `xcrun` doesn't exist, so the whole thing will throw an exception and we just use the `cxx` that was provided by CMake directly (i.e. `CMAKE_CXX_COMPILER`). By the way, this increases the "test as we ship" on Apple platforms because people on Apple platforms don't run `/path/to/compiler -isysroot `, they run `clang++` directly and that shim takes care of setting the `SDKROOT` environment variable which tells Clang where to look for. If the CMake issue linked in the comment were fixed, none of this would be necessary. https://github.com/llvm/llvm-project/pull/66265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [runtimes] Remove explicit -isysroot from the testing configurations on macOS (PR #66265)
petrhosek wrote: This is going to break our builds because we always use the just built Clang (built using the bootstrap build) even on Darwin. I'm fine doing this change for `apple-*.cfg.in` configurations to better match Apple configuration, but for `llvm-*.cfg.in` I'd like to preserve the existing configuration and ideally also land https://reviews.llvm.org/D151056. https://github.com/llvm/llvm-project/pull/66265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [runtimes] Remove explicit -isysroot from the testing configurations on macOS (PR #66265)
llvmbot wrote:
@llvm/pr-subscribers-libunwind
Changes
This shouldn't be necessary if we set the SDKROOT environment variable properly
when we run the test suite, as it is expected on Apple platforms. To set the
SDKROOT environment variable properly, we have to use a slight hack to work
around a CMake issue [1], but at least it makes the hack self contained and it
avoids polluting all the testing configs.
This change was prompted by https://reviews.llvm.org/D151056.
[1]: https://gitlab.kitware.com/cmake/cmake/-/issues/19180
--
Full diff: https://github.com/llvm/llvm-project/pull/66265.diff
14 Files Affected:
- (modified) libcxx/test/configs/apple-libc++-backdeployment.cfg.in (+1-3)
- (modified) libcxx/test/configs/apple-libc++-shared.cfg.in (+1-3)
- (modified) libcxx/test/configs/cmake-bridge.cfg.in (+20-1)
- (modified) libcxx/test/configs/llvm-libc++-shared.cfg.in (+1-3)
- (modified) libcxx/test/configs/llvm-libc++-static.cfg.in (+1-3)
- (modified) libcxxabi/test/configs/apple-libc++abi-backdeployment.cfg.in
(+1-3)
- (modified) libcxxabi/test/configs/apple-libc++abi-shared.cfg.in (+1-3)
- (modified) libcxxabi/test/configs/llvm-libc++abi-merged.cfg.in (+1-3)
- (modified) libcxxabi/test/configs/llvm-libc++abi-shared.cfg.in (+1-3)
- (modified) libcxxabi/test/configs/llvm-libc++abi-static.cfg.in (+1-3)
- (modified) libunwind/test/configs/apple-libunwind-backdeployment.cfg.in
(+1-3)
- (modified) libunwind/test/configs/llvm-libunwind-merged.cfg.in (+1-4)
- (modified) libunwind/test/configs/llvm-libunwind-shared.cfg.in (+1-4)
- (modified) libunwind/test/configs/llvm-libunwind-static.cfg.in (+1-4)
diff --git a/libcxx/test/configs/apple-libc++-backdeployment.cfg.in
b/libcxx/test/configs/apple-libc++-backdeployment.cfg.in
index b471c02709c060f..85fd5fee7ba1ad3 100644
--- a/libcxx/test/configs/apple-libc++-backdeployment.cfg.in
+++ b/libcxx/test/configs/apple-libc++-backdeployment.cfg.in
@@ -41,9 +41,7 @@ BACKDEPLOYMENT_PARAMETERS = [
),
]
-config.substitutions.append((%{flags},
--isysroot {}.format(@CMAKE_OSX_SYSROOT@) if
@CMAKE_OSX_SYSROOT@ else
-))
+config.substitutions.append((%{flags}, ))
config.substitutions.append((%{compile_flags},
-nostdinc++ -I %{include} -I %{libcxx}/test/support
))
diff --git a/libcxx/test/configs/apple-libc++-shared.cfg.in
b/libcxx/test/configs/apple-libc++-shared.cfg.in
index af1926e3859b9e5..165469c674900eb 100644
--- a/libcxx/test/configs/apple-libc++-shared.cfg.in
+++ b/libcxx/test/configs/apple-libc++-shared.cfg.in
@@ -9,9 +9,7 @@
lit_config.load_config(config,
@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg)
-config.substitutions.append((%{flags},
--isysroot {}.format(@CMAKE_OSX_SYSROOT@) if
@CMAKE_OSX_SYSROOT@ else
-))
+config.substitutions.append((%{flags}, ))
config.substitutions.append((%{compile_flags},
-nostdinc++ -I %{include} -I %{libcxx}/test/support
))
diff --git a/libcxx/test/configs/cmake-bridge.cfg.in
b/libcxx/test/configs/cmake-bridge.cfg.in
index 9067115598abd8a..0c19314f55701b3 100644
--- a/libcxx/test/configs/cmake-bridge.cfg.in
+++ b/libcxx/test/configs/cmake-bridge.cfg.in
@@ -2,6 +2,24 @@
@SERIALIZED_LIT_PARAMS@
+# Workaround for https://gitlab.kitware.com/cmake/cmake/-/issues/19180.
+#
+# On Apple platforms, CMake resolves the compiler to the final binary using
+# `$(xcrun --find compiler`). Unfortunately, running this compiler
as-is
+# doesnt work because it doesnt have knowledge of the SDK root. To
work
+# around that, we transform the resolved compiler back to the corresponding
+# /usr/bin shim if we can.
+def workaround_cmake_issue_on_Apple(cxx):
+import subprocess
+for bin in [c++, clang++, g++]:
+try:
+resolved = subprocess.check_output([xcrun,
--find, bin]).decode().strip()
+if cxx == resolved:
+return bin
+except:
+pass
+return cxx
+
#
# This file performs the bridge between the CMake configuration and the Lit
# configuration files by setting up the LitConfig object and various Lit
@@ -24,7 +42,8 @@ config.test_exec_root =
os.path.join(@CMAKE_BINARY_DIR@, test)
# Add substitutions for bootstrapping the test suite configuration
import shlex
-config.substitutions.append((%{cxx},
shlex.quote(@CMAKE_CXX_COMPILER@)))
+cxx = workaround_cmake_issue_on_Apple(@CMAKE_CXX_COMPILER@)
+config.substitutions.append((%{cxx}, shlex.quote(cxx)))
config.substitutions.append((%{libcxx},
@LIBCXX_SOURCE_DIR@))
config.substitutions.append((%{include},
@LIBCXX_GENERATED_INCLUDE_DIR@))
config.substitutions.append((%{target-include},
@LIBCXX_GENERATED_INCLUDE_TARGET_DIR@))
diff --git a/libcxx/test/configs/llvm-libc++-shared.cfg.in
b/libcxx/test/configs/llvm-libc++-shared.cfg.in
index 143b3b3feae1109..41450d78060e8aa 100644
--- a/libcxx/test/configs/llvm-libc++-shared.cfg.in
+++ b/libcxx/test/configs/llvm-libc++-shared.cfg.in
@@ -3,9 +3,7 @@
lit_config.load_config(config,
[libunwind] [runtimes] Remove explicit -isysroot from the testing configurations on macOS (PR #66265)
https://github.com/llvmbot labeled https://github.com/llvm/llvm-project/pull/66265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [runtimes] Remove explicit -isysroot from the testing configurations on macOS (PR #66265)
https://github.com/llvmbot labeled https://github.com/llvm/llvm-project/pull/66265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [runtimes] Remove explicit -isysroot from the testing configurations on macOS (PR #66265)
https://github.com/llvmbot labeled https://github.com/llvm/llvm-project/pull/66265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [runtimes] Remove explicit -isysroot from the testing configurations on macOS (PR #66265)
https://github.com/ldionne review_requested https://github.com/llvm/llvm-project/pull/66265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [runtimes] Remove explicit -isysroot from the testing configurations on macOS (PR #66265)
https://github.com/ldionne review_requested https://github.com/llvm/llvm-project/pull/66265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [runtimes] Remove explicit -isysroot from the testing configurations on macOS (PR #66265)
https://github.com/ldionne review_requested https://github.com/llvm/llvm-project/pull/66265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [runtimes] Remove explicit -isysroot from the testing configurations on macOS (PR #66265)
https://github.com/ldionne created
https://github.com/llvm/llvm-project/pull/66265:
This shouldn't be necessary if we set the SDKROOT environment variable properly
when we run the test suite, as it is expected on Apple platforms. To set the
SDKROOT environment variable properly, we have to use a slight hack to work
around a CMake issue [1], but at least it makes the hack self contained and it
avoids polluting all the testing configs.
This change was prompted by https://reviews.llvm.org/D151056.
[1]: https://gitlab.kitware.com/cmake/cmake/-/issues/19180
>From 186bef879b2943e0953a9df97842008d8daa6234 Mon Sep 17 00:00:00 2001
From: Louis Dionne
Date: Wed, 13 Sep 2023 13:30:30 -0400
Subject: [PATCH] [runtimes] Remove explicit -isysroot from the testing
configurations on macOS
This shouldn't be necessary if we set the SDKROOT environment variable
properly when we run the test suite, as it is expected on Apple platforms.
To set the SDKROOT environment variable properly, we have to use a slight
hack to work around a CMake issue [1], but at least it makes the hack
self contained and it avoids polluting all the testing configs.
This change was prompted by https://reviews.llvm.org/D151056.
[1]: https://gitlab.kitware.com/cmake/cmake/-/issues/19180
---
.../apple-libc++-backdeployment.cfg.in| 4 +---
.../test/configs/apple-libc++-shared.cfg.in | 4 +---
libcxx/test/configs/cmake-bridge.cfg.in | 21 ++-
libcxx/test/configs/llvm-libc++-shared.cfg.in | 4 +---
libcxx/test/configs/llvm-libc++-static.cfg.in | 4 +---
.../apple-libc++abi-backdeployment.cfg.in | 4 +---
.../configs/apple-libc++abi-shared.cfg.in | 4 +---
.../test/configs/llvm-libc++abi-merged.cfg.in | 4 +---
.../test/configs/llvm-libc++abi-shared.cfg.in | 4 +---
.../test/configs/llvm-libc++abi-static.cfg.in | 4 +---
.../apple-libunwind-backdeployment.cfg.in | 4 +---
.../test/configs/llvm-libunwind-merged.cfg.in | 5 +
.../test/configs/llvm-libunwind-shared.cfg.in | 5 +
.../test/configs/llvm-libunwind-static.cfg.in | 5 +
14 files changed, 33 insertions(+), 43 deletions(-)
diff --git a/libcxx/test/configs/apple-libc++-backdeployment.cfg.in
b/libcxx/test/configs/apple-libc++-backdeployment.cfg.in
index b471c02709c060f..85fd5fee7ba1ad3 100644
--- a/libcxx/test/configs/apple-libc++-backdeployment.cfg.in
+++ b/libcxx/test/configs/apple-libc++-backdeployment.cfg.in
@@ -41,9 +41,7 @@ BACKDEPLOYMENT_PARAMETERS = [
"""),
]
-config.substitutions.append(('%{flags}',
-'-isysroot {}'.format('@CMAKE_OSX_SYSROOT@') if '@CMAKE_OSX_SYSROOT@' else
''
-))
+config.substitutions.append(('%{flags}', ''))
config.substitutions.append(('%{compile_flags}',
'-nostdinc++ -I %{include} -I %{libcxx}/test/support'
))
diff --git a/libcxx/test/configs/apple-libc++-shared.cfg.in
b/libcxx/test/configs/apple-libc++-shared.cfg.in
index af1926e3859b9e5..165469c674900eb 100644
--- a/libcxx/test/configs/apple-libc++-shared.cfg.in
+++ b/libcxx/test/configs/apple-libc++-shared.cfg.in
@@ -9,9 +9,7 @@
lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
-config.substitutions.append(('%{flags}',
-'-isysroot {}'.format('@CMAKE_OSX_SYSROOT@') if '@CMAKE_OSX_SYSROOT@' else
''
-))
+config.substitutions.append(('%{flags}', ''))
config.substitutions.append(('%{compile_flags}',
'-nostdinc++ -I %{include} -I %{libcxx}/test/support'
))
diff --git a/libcxx/test/configs/cmake-bridge.cfg.in
b/libcxx/test/configs/cmake-bridge.cfg.in
index 9067115598abd8a..0c19314f55701b3 100644
--- a/libcxx/test/configs/cmake-bridge.cfg.in
+++ b/libcxx/test/configs/cmake-bridge.cfg.in
@@ -2,6 +2,24 @@
@SERIALIZED_LIT_PARAMS@
+# Workaround for https://gitlab.kitware.com/cmake/cmake/-/issues/19180.
+#
+# On Apple platforms, CMake resolves the compiler to the final binary using
+# `$(xcrun --find `). Unfortunately, running this compiler as-is
+# doesn't work because it doesn't have knowledge of the SDK root. To work
+# around that, we transform the resolved compiler back to the corresponding
+# /usr/bin shim if we can.
+def workaround_cmake_issue_on_Apple(cxx):
+import subprocess
+for bin in ['c++', 'clang++', 'g++']:
+try:
+resolved = subprocess.check_output(['xcrun', '--find',
bin]).decode().strip()
+if cxx == resolved:
+return bin
+except:
+pass
+return cxx
+
#
# This file performs the bridge between the CMake configuration and the Lit
# configuration files by setting up the LitConfig object and various Lit
@@ -24,7 +42,8 @@ config.test_exec_root = os.path.join('@CMAKE_BINARY_DIR@',
'test')
# Add substitutions for bootstrapping the test suite configuration
import shlex
-config.substitutions.append(('%{cxx}', shlex.quote('@CMAKE_CXX_COMPILER@')))
+cxx = workaround_cmake_issue_on_Apple('@CMAKE_CXX_COMPILER@')
+config.substitutions.append(('%{cxx}', shlex.quote(cxx)))
