Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc++] Test suite adjustments on macOS #95835

Merged

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Jun 17, 2024

This patch makes a few adjustments to the way we run the tests in the Apple configuration on macOS:

First, we stop using DYLD_LIBRARY_PATH. Using that environment variable leads to libc++.dylib being replaced by the just-built one for the whole process, and that assumes compatibility between the system-provided dylib and the just-built one. Unfortunately, that is not the case anymore due to typed allocation, which is only available in the system one. Instead, we want to layer the just-built libc++ on top of the system-provided one, which seems to be what happens when we set a rpath instead.

Second, add a missing XFAIL for a std::print test that didn't work as expected when building with availability annotations enabled. When we enable these annotations, std::print falls back to a non-unicode and non-terminal output, which breaks the test.

@ldionne ldionne requested review from a team as code owners June 17, 2024 19:55
@llvmbot llvmbot added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. libc++abi libc++abi C++ Runtime Library. Not libc++. labels Jun 17, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 17, 2024

@llvm/pr-subscribers-libcxxabi

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

This patch makes a few adjustments to the way we run the tests in the Apple configuration on macOS:

First, we stop using DYLD_LIBRARY_PATH. Using that environment variable leads to libc++.dylib being replaced by the just-built one for the whole process, and that assumes compatibility between the system-provided dylib and the just-built one. Unfortunately, that is not the case anymore due to typed allocation, which is only available in the system one. Instead, we want to layer the just-built libc++ on top of the system-provided one, which seems to be what happens when we set a rpath instead.

Second, add a missing XFAIL for a std::print test that didn't work as expected when building with availability annotations enabled. When we enable these annotations, std::print falls back to a non-unicode and non-terminal output, which breaks the test.


Full diff: https://github.com/llvm/llvm-project/pull/95835.diff

4 Files Affected:

  • (modified) libcxx/cmake/caches/Apple.cmake (+2-2)
  • (removed) libcxx/test/configs/apple-libc++-shared.cfg.in (-35)
  • (modified) libcxx/test/libcxx/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_unicode.pass.cpp (+5)
  • (removed) libcxxabi/test/configs/apple-libc++abi-shared.cfg.in (-29)
diff --git a/libcxx/cmake/caches/Apple.cmake b/libcxx/cmake/caches/Apple.cmake
index 8768653e620ad..985f0bca8d8fa 100644
--- a/libcxx/cmake/caches/Apple.cmake
+++ b/libcxx/cmake/caches/Apple.cmake
@@ -16,5 +16,5 @@ set(LIBCXXABI_ENABLE_ASSERTIONS OFF CACHE BOOL "")
 set(LIBCXXABI_ENABLE_FORGIVING_DYNAMIC_CAST ON CACHE BOOL "")
 set(LIBCXXABI_USE_LLVM_UNWINDER OFF CACHE BOOL "") # libunwind is built separately
 
-set(LIBCXX_TEST_CONFIG "apple-libc++-shared.cfg.in" CACHE STRING "")
-set(LIBCXXABI_TEST_CONFIG "apple-libc++abi-shared.cfg.in" CACHE STRING "")
+set(LIBCXX_TEST_PARAMS "stdlib=apple-libc++" CACHE STRING "")
+set(LIBCXXABI_TEST_PARAMS "${LIBCXX_TEST_PARAMS}" CACHE STRING "")
diff --git a/libcxx/test/configs/apple-libc++-shared.cfg.in b/libcxx/test/configs/apple-libc++-shared.cfg.in
deleted file mode 100644
index 2d0aee3ae905e..0000000000000
--- a/libcxx/test/configs/apple-libc++-shared.cfg.in
+++ /dev/null
@@ -1,35 +0,0 @@
-# Testing configuration for Apple's system libc++.
-#
-# This configuration differs from a normal LLVM shared library configuration in
-# that we must use DYLD_LIBRARY_PATH to run the tests against the just-built library,
-# since Apple's libc++ has an absolute install_name.
-#
-# We also don't use a per-target include directory layout, so we have only one
-# include directory for the libc++ headers.
-
-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(('%{compile_flags}',
-    '-nostdinc++ -I %{include-dir} -I %{libcxx-dir}/test/support'
-))
-config.substitutions.append(('%{link_flags}',
-    '-nostdlib++ -L %{lib-dir} -lc++'
-))
-config.substitutions.append(('%{exec}',
-    '%{executor} --execdir %T --env DYLD_LIBRARY_PATH=%{lib-dir} -- '
-))
-
-config.stdlib = 'apple-libc++'
-
-import os, site
-site.addsitedir(os.path.join('@LIBCXX_SOURCE_DIR@', 'utils'))
-import libcxx.test.params, libcxx.test.config
-libcxx.test.config.configure(
-    libcxx.test.params.DEFAULT_PARAMETERS,
-    libcxx.test.features.DEFAULT_FEATURES,
-    config,
-    lit_config
-)
diff --git a/libcxx/test/libcxx/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_unicode.pass.cpp b/libcxx/test/libcxx/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_unicode.pass.cpp
index 5438d31bca33f..572c6ca474014 100644
--- a/libcxx/test/libcxx/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_unicode.pass.cpp
+++ b/libcxx/test/libcxx/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_unicode.pass.cpp
@@ -11,6 +11,11 @@
 
 // XFAIL: availability-fp_to_chars-missing
 
+// When std::print is unavailable, we don't rely on an implementation of
+// std::__is_terminal and we always assume a non-unicode and non-terminal
+// output.
+// XFAIL: availability-print-missing
+
 // Clang modules do not work with the definiton of _LIBCPP_TESTING_PRINT_IS_TERMINAL
 // XFAIL: clang-modules-build
 // <ostream>
diff --git a/libcxxabi/test/configs/apple-libc++abi-shared.cfg.in b/libcxxabi/test/configs/apple-libc++abi-shared.cfg.in
deleted file mode 100644
index ec0c93b0134a4..0000000000000
--- a/libcxxabi/test/configs/apple-libc++abi-shared.cfg.in
+++ /dev/null
@@ -1,29 +0,0 @@
-# Testing configuration for Apple's system libc++abi.
-
-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(('%{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'
-))
-config.substitutions.append(('%{link_flags}',
-    '-nostdlib++ -L %{lib} -lc++ -lc++abi'
-))
-config.substitutions.append(('%{exec}',
-    '%{executor} --execdir %T --env DYLD_LIBRARY_PATH=%{lib} -- '
-))
-
-config.stdlib = 'apple-libc++'
-
-import os, site
-site.addsitedir(os.path.join('@LIBCXXABI_LIBCXX_PATH@', 'utils'))
-import libcxx.test.params, libcxx.test.config
-libcxx.test.config.configure(
-    libcxx.test.params.DEFAULT_PARAMETERS,
-    libcxx.test.features.DEFAULT_FEATURES,
-    config,
-    lit_config
-)

This patch makes a few adjustments to the way we run the tests in the
Apple configuration on macOS:

First, we stop using DYLD_LIBRARY_PATH. Using that environment variable
leads to libc++.dylib being replaced by the just-built one for the whole
process, and that assumes compatibility between the system-provided dylib
and the just-built one. Unfortunately, that is not the case anymore due
to typed allocation, which is only available in the system one. Instead,
we want to layer the just-built libc++ on top of the system-provided one,
which seems to be what happens when we set a rpath instead.

Second, add a missing XFAIL for a std::print test that didn't work as
expected when building with availability annotations enabled. When we
enable these annotations, std::print falls back to a non-unicode and
non-terminal output, which breaks the test.
@ldionne ldionne force-pushed the review/ci-use-llvm-shared-config-on-apple branch from 2a88ce3 to f691762 Compare June 17, 2024 20:13
@ldionne ldionne merged commit 2cf2f1b into llvm:main Jun 18, 2024
52 of 54 checks passed
@ldionne ldionne deleted the review/ci-use-llvm-shared-config-on-apple branch June 18, 2024 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++abi libc++abi C++ Runtime Library. Not libc++. libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants