[libunwind] [runtimes] Remove explicit -isysroot from the testing configurations on macOS (PR #66265)
Louis Dionne via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 14 07:07:30 PDT 2023
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 <ldionne.2 at gmail.com>
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 %{target-include} -I %{libcxx}/test/support'
))
diff --git a/libcxx/test/configs/llvm-libc++-static.cfg.in b/libcxx/test/configs/llvm-libc++-static.cfg.in
index e866d4f52a0629d..3ef4647bfb2ebe4 100644
--- a/libcxx/test/configs/llvm-libc++-static.cfg.in
+++ b/libcxx/test/configs/llvm-libc++-static.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 %{target-include} -I %{libcxx}/test/support'
))
diff --git a/libcxx/utils/libcxx/test/format.py b/libcxx/utils/libcxx/test/format.py
index ddd88f25646eaa8..8e75285848a0b85 100644
--- a/libcxx/utils/libcxx/test/format.py
+++ b/libcxx/utils/libcxx/test/format.py
@@ -424,3 +424,18 @@ def _splitFile(self, input):
thisFileContent.append(line)
if currentFile is not None:
yield (currentFile, '\n'.join(thisFileContent))
+
+# 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
+# doesn't work because it doesn't have knowledge of the SDK root. To work
+# around that, we run the compiler binary under `xcrun`, which sets the
+# SDK root properly via the SDKROOT environment variable. Ideally, CMake
+# would not resolve the compiler to its final binary and CMAKE_CXX_COMPILER
+# would be a shim in /usr/bin, which sets the SDK root properly.
+def cmake_workaround_for_cxx_on_Apple(cxx):
+ import sys
+ if sys.platform == 'darwin':
+ return f'xcrun {cxx}'
+ return cxx
diff --git a/libcxxabi/test/configs/apple-libc++abi-backdeployment.cfg.in b/libcxxabi/test/configs/apple-libc++abi-backdeployment.cfg.in
index 2e9472163b9d59c..ea2d90f58f49f73 100644
--- a/libcxxabi/test/configs/apple-libc++abi-backdeployment.cfg.in
+++ b/libcxxabi/test/configs/apple-libc++abi-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 %{cxx-include} -I %{cxx-target-include} %{maybe-include-libunwind} -D_LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS ' +
'-I %{libcxx}/test/support -I %{libcxx}/src'
diff --git a/libcxxabi/test/configs/apple-libc++abi-shared.cfg.in b/libcxxabi/test/configs/apple-libc++abi-shared.cfg.in
index ec0c93b0134a45f..bed2172b1ae0772 100644
--- a/libcxxabi/test/configs/apple-libc++abi-shared.cfg.in
+++ b/libcxxabi/test/configs/apple-libc++abi-shared.cfg.in
@@ -2,9 +2,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 %{cxx-include} -I %{cxx-target-include} %{maybe-include-libunwind} -D_LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS ' +
'-I %{libcxx}/test/support -I %{libcxx}/src'
diff --git a/libcxxabi/test/configs/cmake-bridge.cfg.in b/libcxxabi/test/configs/cmake-bridge.cfg.in
index 89b2dca6e530592..914f6bdb3c42f56 100644
--- a/libcxxabi/test/configs/cmake-bridge.cfg.in
+++ b/libcxxabi/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('@LIBCXXABI_LIBCXX_PATH@', 'utils'))
site.addsitedir(os.path.join('@LIBCXXABI_SOURCE_DIR@', 'test'))
import libcxx.test.format
@@ -26,7 +26,8 @@ config.test_exec_root = os.path.join('@CMAKE_BINARY_DIR@', 'test')
# TODO: This is a non-standard Lit attribute and we should have another way of accessing this.
config.host_triple = '@LLVM_HOST_TRIPLE@'
-config.substitutions.append(('%{cxx}', '@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}', '@LIBCXXABI_LIBCXX_PATH@'))
config.substitutions.append(('%{include}', '@LIBCXXABI_SOURCE_DIR@/include'))
config.substitutions.append(('%{cxx-include}', '@LIBCXXABI_HEADER_DIR@/include/c++/v1'))
diff --git a/libcxxabi/test/configs/llvm-libc++abi-merged.cfg.in b/libcxxabi/test/configs/llvm-libc++abi-merged.cfg.in
index c6fa4301b459ce9..1dd7d59e9abc6c8 100644
--- a/libcxxabi/test/configs/llvm-libc++abi-merged.cfg.in
+++ b/libcxxabi/test/configs/llvm-libc++abi-merged.cfg.in
@@ -2,9 +2,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 %{cxx-include} -I %{cxx-target-include} %{maybe-include-libunwind} -D_LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS ' +
'-I %{libcxx}/test/support -I %{libcxx}/src'
diff --git a/libcxxabi/test/configs/llvm-libc++abi-shared.cfg.in b/libcxxabi/test/configs/llvm-libc++abi-shared.cfg.in
index 6b69205da77c992..19528d9c9bd9440 100644
--- a/libcxxabi/test/configs/llvm-libc++abi-shared.cfg.in
+++ b/libcxxabi/test/configs/llvm-libc++abi-shared.cfg.in
@@ -3,9 +3,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 %{cxx-include} -I %{cxx-target-include} %{maybe-include-libunwind} -I %{libcxx}/test/support -I %{libcxx}/src -D_LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS'
))
diff --git a/libcxxabi/test/configs/llvm-libc++abi-static.cfg.in b/libcxxabi/test/configs/llvm-libc++abi-static.cfg.in
index 352e2c39586e1ea..89c43bc9c24c8b2 100644
--- a/libcxxabi/test/configs/llvm-libc++abi-static.cfg.in
+++ b/libcxxabi/test/configs/llvm-libc++abi-static.cfg.in
@@ -3,9 +3,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 %{cxx-include} -I %{cxx-target-include} %{maybe-include-libunwind} -I %{libcxx}/test/support -I %{libcxx}/src -D_LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS'
))
diff --git a/libunwind/test/configs/apple-libunwind-backdeployment.cfg.in b/libunwind/test/configs/apple-libunwind-backdeployment.cfg.in
index 4484573801bd2ba..af2e54c3f8189e4 100644
--- a/libunwind/test/configs/apple-libunwind-backdeployment.cfg.in
+++ b/libunwind/test/configs/apple-libunwind-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}'
))
diff --git a/libunwind/test/configs/cmake-bridge.cfg.in b/libunwind/test/configs/cmake-bridge.cfg.in
index 7860301c367fbc5..1792638d340f0dd 100644
--- a/libunwind/test/configs/cmake-bridge.cfg.in
+++ b/libunwind/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('@LIBUNWIND_LIBCXX_PATH@', 'utils'))
import libcxx.test.format
@@ -29,8 +29,8 @@ if not @LIBUNWIND_ENABLE_THREADS@:
config.available_features.add('libunwind-no-threads')
# 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(('%{executor}', '@LIBUNWIND_EXECUTOR@'))
config.substitutions.append(('%{include}', '@LIBUNWIND_SOURCE_DIR@/include'))
config.substitutions.append(('%{lib}', '@LIBUNWIND_LIBRARY_DIR@'))
diff --git a/libunwind/test/configs/llvm-libunwind-merged.cfg.in b/libunwind/test/configs/llvm-libunwind-merged.cfg.in
index 10650f7edf66a2f..8ebceb7d2f51a57 100644
--- a/libunwind/test/configs/llvm-libunwind-merged.cfg.in
+++ b/libunwind/test/configs/llvm-libunwind-merged.cfg.in
@@ -20,10 +20,7 @@ if '@CMAKE_DL_LIBS@':
# Stack unwinding tests need unwinding tables and these are not generated by default on all targets.
compile_flags.append('-funwind-tables')
-local_sysroot = '@CMAKE_OSX_SYSROOT@' or '@CMAKE_SYSROOT@'
-config.substitutions.append(('%{flags}',
- '-isysroot {}'.format(local_sysroot) if local_sysroot else ''
-))
+config.substitutions.append(('%{flags}', ''))
config.substitutions.append(('%{compile_flags}',
'-nostdinc++ -I %{{include}} {}'.format(' '.join(compile_flags))
))
diff --git a/libunwind/test/configs/llvm-libunwind-shared.cfg.in b/libunwind/test/configs/llvm-libunwind-shared.cfg.in
index 97185e57234ff7f..fa678f75bb48481 100644
--- a/libunwind/test/configs/llvm-libunwind-shared.cfg.in
+++ b/libunwind/test/configs/llvm-libunwind-shared.cfg.in
@@ -19,10 +19,7 @@ if '@CMAKE_DL_LIBS@':
# Stack unwinding tests need unwinding tables and these are not generated by default on all targets.
compile_flags.append('-funwind-tables')
-local_sysroot = '@CMAKE_OSX_SYSROOT@' or '@CMAKE_SYSROOT@'
-config.substitutions.append(('%{flags}',
- '-isysroot {}'.format(local_sysroot) if local_sysroot else ''
-))
+config.substitutions.append(('%{flags}', ''))
config.substitutions.append(('%{compile_flags}',
'-nostdinc++ -I %{{include}} {}'.format(' '.join(compile_flags))
))
diff --git a/libunwind/test/configs/llvm-libunwind-static.cfg.in b/libunwind/test/configs/llvm-libunwind-static.cfg.in
index fc6a18d057f3884..235f32a8b6d7ff4 100644
--- a/libunwind/test/configs/llvm-libunwind-static.cfg.in
+++ b/libunwind/test/configs/llvm-libunwind-static.cfg.in
@@ -22,10 +22,7 @@ if '@CMAKE_DL_LIBS@':
# Stack unwinding tests need unwinding tables and these are not generated by default on all targets.
compile_flags.append('-funwind-tables')
-local_sysroot = '@CMAKE_OSX_SYSROOT@' or '@CMAKE_SYSROOT@'
-config.substitutions.append(('%{flags}',
- '-isysroot {}'.format(local_sysroot) if local_sysroot else ''
-))
+config.substitutions.append(('%{flags}', ''))
config.substitutions.append(('%{compile_flags}',
'-nostdinc++ -I %{{include}} {}'.format(' '.join(compile_flags))
))
More information about the cfe-commits
mailing list