[libcxx-commits] [libcxx] [libc++] Run the Lit test suite against an installed version of the library (PR #96910)

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Tue Jul 2 13:50:59 PDT 2024


https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/96910

>From b96fdd92a608af32c9f6cd6193b3087791f98b5d Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Wed, 26 Jun 2024 15:17:42 -0500
Subject: [PATCH 1/4] [libc++] Run the Lit test suite against an installed
 version of the library

We always strive to test libc++ as close as possible to the way we are
actually shipping it. This was approximated reasonably well by setting
up the minimal driver flags when running the test suite, however we were
running the test suite against the library located in the build directory.

This patch improves the situation by installing the library (the headers,
the built library, modules, etc) into a fake location and then running
the test suite against that fake "installation root".

This should open the door to getting rid of the temporary copy of the
headers we make during the build process, however this is left for a
future improvement.

Note that this adds quite a bit of verbosity whenever running the test
suite because we install the headers beforehand every time. We should
be able to override this to silence it, however CMake doesn't currently
give us a way to do that, see https://gitlab.kitware.com/cmake/cmake/-/issues/26085.
---
 libcxx/modules/CMakeLists.txt           |  1 -
 libcxx/src/CMakeLists.txt               |  2 -
 libcxx/test/CMakeLists.txt              | 56 +++++++++++++++++++++++++
 libcxx/test/configs/cmake-bridge.cfg.in |  8 ++--
 4 files changed, 60 insertions(+), 7 deletions(-)

diff --git a/libcxx/modules/CMakeLists.txt b/libcxx/modules/CMakeLists.txt
index 82cd7b66beb7a..d47d19a475531 100644
--- a/libcxx/modules/CMakeLists.txt
+++ b/libcxx/modules/CMakeLists.txt
@@ -202,7 +202,6 @@ add_custom_target(generate-cxx-modules
   ALL DEPENDS
     ${_all_modules}
 )
-add_dependencies(cxx-test-depends generate-cxx-modules)
 
 # Configure the modules manifest.
 # Use the relative path between the installation and the module in the json
diff --git a/libcxx/src/CMakeLists.txt b/libcxx/src/CMakeLists.txt
index 8ba6f00bf9dad..d26c09111ff96 100644
--- a/libcxx/src/CMakeLists.txt
+++ b/libcxx/src/CMakeLists.txt
@@ -327,7 +327,6 @@ endif()
 
 # Add a meta-target for both libraries.
 add_custom_target(cxx DEPENDS ${LIBCXX_BUILD_TARGETS})
-add_dependencies(cxx-test-depends cxx)
 
 set(LIBCXX_EXPERIMENTAL_SOURCES
   experimental/keep.cpp
@@ -375,7 +374,6 @@ set_target_properties(cxx_experimental
 )
 cxx_add_common_build_flags(cxx_experimental)
 target_compile_options(cxx_experimental PUBLIC -D_LIBCPP_ENABLE_EXPERIMENTAL)
-add_dependencies(cxx-test-depends cxx_experimental)
 
 if (LIBCXX_INSTALL_SHARED_LIBRARY)
   install(TARGETS cxx_shared
diff --git a/libcxx/test/CMakeLists.txt b/libcxx/test/CMakeLists.txt
index cdd1c2d90fbcf..33451223e63bf 100644
--- a/libcxx/test/CMakeLists.txt
+++ b/libcxx/test/CMakeLists.txt
@@ -7,6 +7,62 @@ if (NOT LIBCXX_CXX_ABI_LIBRARY_PATH)
       "The path to libc++abi library.")
 endif()
 
+# Install the library at a fake location so we can run the test suite against it.
+# This ensures that we run the test suite against a setup that matches what we ship
+# in production as closely as possible (in terms of file paths, rpaths, etc).
+set(LIBCXX_TESTING_INSTALL_PREFIX "${LIBCXX_BINARY_DIR}/test-suite-install")
+if (LIBCXX_CXX_ABI STREQUAL "cxxabi")
+  add_custom_target(install-cxxabi-test-suite-prefix
+                        DEPENDS cxxabi-headers
+                                cxxabi
+                        COMMAND ${CMAKE_COMMAND} -E make_directory "${LIBCXX_TESTING_INSTALL_PREFIX}"
+                        COMMAND "${CMAKE_COMMAND}"
+                                -DCMAKE_INSTALL_COMPONENT=cxxabi-headers
+                                -DCMAKE_INSTALL_PREFIX="${LIBCXX_TESTING_INSTALL_PREFIX}"
+                                -P "${CMAKE_BINARY_DIR}/cmake_install.cmake"
+                        COMMAND "${CMAKE_COMMAND}"
+                                -DCMAKE_INSTALL_COMPONENT=cxxabi
+                                -DCMAKE_INSTALL_PREFIX="${LIBCXX_TESTING_INSTALL_PREFIX}"
+                                -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
+  add_dependencies(cxx-test-depends install-cxxabi-test-suite-prefix)
+endif()
+
+if (LIBCXXABI_USE_LLVM_UNWINDER)
+  add_custom_target(install-unwind-test-suite-prefix
+  DEPENDS unwind-headers
+          unwind
+  COMMAND ${CMAKE_COMMAND} -E make_directory "${LIBCXX_TESTING_INSTALL_PREFIX}"
+  COMMAND "${CMAKE_COMMAND}"
+          -DCMAKE_INSTALL_COMPONENT=unwind-headers
+          -DCMAKE_INSTALL_PREFIX="${LIBCXX_TESTING_INSTALL_PREFIX}"
+          -P "${CMAKE_BINARY_DIR}/cmake_install.cmake"
+  COMMAND "${CMAKE_COMMAND}"
+          -DCMAKE_INSTALL_COMPONENT=unwind
+          -DCMAKE_INSTALL_PREFIX="${LIBCXX_TESTING_INSTALL_PREFIX}"
+          -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
+  add_dependencies(cxx-test-depends install-unwind-test-suite-prefix)
+endif()
+
+add_custom_target(install-cxx-test-suite-prefix
+                      DEPENDS cxx-headers
+                              cxx
+                              cxx_experimental
+                              cxx-modules
+                      COMMAND ${CMAKE_COMMAND} -E make_directory "${LIBCXX_TESTING_INSTALL_PREFIX}"
+                      COMMAND "${CMAKE_COMMAND}"
+                              -DCMAKE_INSTALL_COMPONENT=cxx-headers
+                              -DCMAKE_INSTALL_PREFIX="${LIBCXX_TESTING_INSTALL_PREFIX}"
+                              -P "${CMAKE_BINARY_DIR}/cmake_install.cmake"
+                      COMMAND "${CMAKE_COMMAND}"
+                              -DCMAKE_INSTALL_COMPONENT=cxx-modules
+                              -DCMAKE_INSTALL_PREFIX="${LIBCXX_TESTING_INSTALL_PREFIX}"
+                              -P "${CMAKE_BINARY_DIR}/cmake_install.cmake"
+                      COMMAND "${CMAKE_COMMAND}"
+                              -DCMAKE_INSTALL_COMPONENT=cxx
+                              -DCMAKE_INSTALL_PREFIX="${LIBCXX_TESTING_INSTALL_PREFIX}"
+                              -P "${LIBCXX_BINARY_DIR}/cmake_install.cmake")
+add_dependencies(cxx-test-depends install-cxx-test-suite-prefix)
+
 set(AUTO_GEN_COMMENT "## Autogenerated by libcxx configuration.\n# Do not edit!")
 set(SERIALIZED_LIT_PARAMS "# Lit parameters serialized here for llvm-lit to pick them up\n")
 
diff --git a/libcxx/test/configs/cmake-bridge.cfg.in b/libcxx/test/configs/cmake-bridge.cfg.in
index 78d0cb5a25748..fe93e0bfa25e3 100644
--- a/libcxx/test/configs/cmake-bridge.cfg.in
+++ b/libcxx/test/configs/cmake-bridge.cfg.in
@@ -24,8 +24,8 @@ config.test_exec_root = os.path.join('@CMAKE_BINARY_DIR@', 'test')
 
 # Add substitutions for bootstrapping the test suite configuration
 config.substitutions.append(('%{libcxx-dir}', '@LIBCXX_SOURCE_DIR@'))
-config.substitutions.append(('%{include-dir}', '@LIBCXX_GENERATED_INCLUDE_DIR@'))
-config.substitutions.append(('%{target-include-dir}', '@LIBCXX_GENERATED_INCLUDE_TARGET_DIR@'))
-config.substitutions.append(('%{lib-dir}', '@LIBCXX_LIBRARY_DIR@'))
-config.substitutions.append(('%{module-dir}', '@LIBCXX_GENERATED_MODULE_DIR@'))
+config.substitutions.append(('%{include-dir}', '@LIBCXX_TESTING_INSTALL_PREFIX@/@LIBCXX_INSTALL_INCLUDE_DIR@'))
+config.substitutions.append(('%{target-include-dir}', '@LIBCXX_TESTING_INSTALL_PREFIX@/@LIBCXX_INSTALL_INCLUDE_TARGET_DIR@'))
+config.substitutions.append(('%{lib-dir}', '@LIBCXX_TESTING_INSTALL_PREFIX@/@LIBCXX_INSTALL_LIBRARY_DIR@'))
+config.substitutions.append(('%{module-dir}', '@LIBCXX_TESTING_INSTALL_PREFIX@/@LIBCXX_INSTALL_MODULES_DIR@'))
 config.substitutions.append(('%{test-tools-dir}', '@LIBCXX_TEST_TOOLS_PATH@'))

>From b7b115d3a7b686d76acec8d86d90fab73b6a004c Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Thu, 27 Jun 2024 15:39:21 -0500
Subject: [PATCH 2/4] Fix typo

---
 libcxx/test/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libcxx/test/CMakeLists.txt b/libcxx/test/CMakeLists.txt
index 33451223e63bf..ee41529aa6526 100644
--- a/libcxx/test/CMakeLists.txt
+++ b/libcxx/test/CMakeLists.txt
@@ -11,7 +11,7 @@ endif()
 # This ensures that we run the test suite against a setup that matches what we ship
 # in production as closely as possible (in terms of file paths, rpaths, etc).
 set(LIBCXX_TESTING_INSTALL_PREFIX "${LIBCXX_BINARY_DIR}/test-suite-install")
-if (LIBCXX_CXX_ABI STREQUAL "cxxabi")
+if (LIBCXX_CXX_ABI STREQUAL "libcxxabi")
   add_custom_target(install-cxxabi-test-suite-prefix
                         DEPENDS cxxabi-headers
                                 cxxabi

>From 83b3d10f57fbadc1283f6e077ed6dc5fe02b783e Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Thu, 27 Jun 2024 15:42:45 -0500
Subject: [PATCH 3/4] Remove explicit installation step from run-buildbot since
 we now always do it before we run the test suite

---
 libcxx/utils/ci/run-buildbot | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/libcxx/utils/ci/run-buildbot b/libcxx/utils/ci/run-buildbot
index f1c20b9d72190..093731d9389e9 100755
--- a/libcxx/utils/ci/run-buildbot
+++ b/libcxx/utils/ci/run-buildbot
@@ -154,19 +154,6 @@ function check-runtimes() {
 
     echo "+++ Running the libunwind tests"
     ${NINJA} -vC "${BUILD_DIR}" check-unwind
-
-    # TODO: On macOS 13.5, the linker seems to have an issue where it will pick up
-    #       a library if it exists inside a -L search path, even if we don't link
-    #       against that library. This happens with libunwind.dylib if it is built
-    #       at the point when we run the libc++ tests, which causes issues cause we
-    #       are also linking against the system unwinder.
-    #
-    #       I believe this is a linker regression and I reported it as rdar://115842730.
-    #       It should be possible to move this installation step back to the top once
-    #       that issue has been resolved, but in the meantime it doesn't really hurt to
-    #       have it here.
-    echo "--- Installing libc++, libc++abi and libunwind to a fake location"
-    ${NINJA} -vC "${BUILD_DIR}" install-cxx install-cxxabi install-unwind
 }
 
 # TODO: The goal is to test this against all configurations. We should also move

>From 132ef17704e0156208db3dcce71a221248c106e5 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Tue, 2 Jul 2024 16:50:47 -0400
Subject: [PATCH 4/4] Check for the existence of the unwind target

---
 libcxx/test/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libcxx/test/CMakeLists.txt b/libcxx/test/CMakeLists.txt
index ee41529aa6526..4190e6ca3b3a6 100644
--- a/libcxx/test/CMakeLists.txt
+++ b/libcxx/test/CMakeLists.txt
@@ -27,7 +27,7 @@ if (LIBCXX_CXX_ABI STREQUAL "libcxxabi")
   add_dependencies(cxx-test-depends install-cxxabi-test-suite-prefix)
 endif()
 
-if (LIBCXXABI_USE_LLVM_UNWINDER)
+if (LIBCXXABI_USE_LLVM_UNWINDER and TARGET unwind)
   add_custom_target(install-unwind-test-suite-prefix
   DEPENDS unwind-headers
           unwind



More information about the libcxx-commits mailing list