[libunwind] [runtimes] Remove explicit -isysroot from the testing configurations on macOS (PR #66265)

Louis Dionne via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 13 11:02:23 PDT 2023


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 <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
---
 .../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 <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 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, '@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/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/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/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