[libc++] Simplify how we install test-suite dependencies #171504
Merged
+28
−94
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Based on comments in llvm#171474, it was brought to my attention that we can modernize and simplify how we perform the test suite installation in libc++ and libc++abi.
llvmbot
added
libc++
Member
|
@llvm/pr-subscribers-libcxx @llvm/pr-subscribers-libcxxabi Author: Louis Dionne (ldionne) ChangesBased on comments in #171474, it was brought to my attention that we can modernize and simplify how we perform the test suite installation in libc++ and libc++abi. Full diff: https://github.com/llvm/llvm-project/pull/171504.diff 2 Files Affected:
diff --git a/libcxx/test/CMakeLists.txt b/libcxx/test/CMakeLists.txt
index f4e577aed57de..57474f92a4bfd 100644
--- a/libcxx/test/CMakeLists.txt
+++ b/libcxx/test/CMakeLists.txt
@@ -5,57 +5,20 @@ add_subdirectory(tools)
# 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")
+set(libcxx_test_suite_install_targets cxx-headers cxx cxx_experimental cxx-modules)
if (LIBCXX_CXX_ABI STREQUAL "libcxxabi")
- 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)
+ list(APPEND libcxx_test_suite_install_targets cxxabi-headers cxxabi)
endif()
-
if (LIBCXXABI_USE_LLVM_UNWINDER AND TARGET unwind)
- 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)
+ list(APPEND libcxx_test_suite_install_targets unwind-headers unwind)
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)
+foreach(target IN LISTS libcxx_test_suite_install_targets)
+ add_custom_target(libcxx-test-suite-install-${target} DEPENDS "${target}"
+ COMMAND "${CMAKE_COMMAND}" --install "${CMAKE_BINARY_DIR}"
+ --prefix "${LIBCXX_TESTING_INSTALL_PREFIX}"
+ --component "${target}")
+ add_dependencies(cxx-test-depends libcxx-test-suite-install-${target})
+endforeach()
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/libcxxabi/test/CMakeLists.txt b/libcxxabi/test/CMakeLists.txt
index 9eabfb08240b6..021bd7cc959e2 100644
--- a/libcxxabi/test/CMakeLists.txt
+++ b/libcxxabi/test/CMakeLists.txt
@@ -8,56 +8,21 @@ macro(pythonize_bool var)
endif()
endmacro()
+# Install the library and its dependencies 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(LIBCXXABI_TESTING_INSTALL_PREFIX "${LIBCXXABI_BINARY_DIR}/test-suite-install")
-add_custom_target(libcxxabi-install-cxx-for-testing
- DEPENDS cxx-headers
- cxx
- cxx_experimental
- cxx-modules
- COMMAND ${CMAKE_COMMAND} -E make_directory "${LIBCXXABI_TESTING_INSTALL_PREFIX}"
- COMMAND "${CMAKE_COMMAND}"
- -DCMAKE_INSTALL_COMPONENT=cxx-headers
- -DCMAKE_INSTALL_PREFIX="${LIBCXXABI_TESTING_INSTALL_PREFIX}"
- -P "${CMAKE_BINARY_DIR}/cmake_install.cmake"
- COMMAND "${CMAKE_COMMAND}"
- -DCMAKE_INSTALL_COMPONENT=cxx-modules
- -DCMAKE_INSTALL_PREFIX="${LIBCXXABI_TESTING_INSTALL_PREFIX}"
- -P "${CMAKE_BINARY_DIR}/cmake_install.cmake"
- COMMAND "${CMAKE_COMMAND}"
- -DCMAKE_INSTALL_COMPONENT=cxx
- -DCMAKE_INSTALL_PREFIX="${LIBCXXABI_TESTING_INSTALL_PREFIX}"
- -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
-add_dependencies(cxxabi-test-depends libcxxabi-install-cxx-for-testing)
-
-add_custom_target(libcxxabi-install-cxxabi-for-testing
- DEPENDS cxxabi-headers
- cxxabi
- COMMAND ${CMAKE_COMMAND} -E make_directory "${LIBCXXABI_TESTING_INSTALL_PREFIX}"
- COMMAND "${CMAKE_COMMAND}"
- -DCMAKE_INSTALL_COMPONENT=cxxabi-headers
- -DCMAKE_INSTALL_PREFIX="${LIBCXXABI_TESTING_INSTALL_PREFIX}"
- -P "${CMAKE_BINARY_DIR}/cmake_install.cmake"
- COMMAND "${CMAKE_COMMAND}"
- -DCMAKE_INSTALL_COMPONENT=cxxabi
- -DCMAKE_INSTALL_PREFIX="${LIBCXXABI_TESTING_INSTALL_PREFIX}"
- -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
-add_dependencies(cxxabi-test-depends libcxxabi-install-cxxabi-for-testing)
-
+set(libcxxabi_test_suite_install_targets cxx-headers cxx cxx_experimental cxx-modules cxxabi-headers cxxabi)
if (LIBCXXABI_USE_LLVM_UNWINDER AND TARGET unwind)
- add_custom_target(libcxxabi-install-unwind-for-testing
- DEPENDS unwind-headers
- unwind
- COMMAND ${CMAKE_COMMAND} -E make_directory "${LIBCXXABI_TESTING_INSTALL_PREFIX}"
- COMMAND "${CMAKE_COMMAND}"
- -DCMAKE_INSTALL_COMPONENT=unwind-headers
- -DCMAKE_INSTALL_PREFIX="${LIBCXXABI_TESTING_INSTALL_PREFIX}"
- -P "${CMAKE_BINARY_DIR}/cmake_install.cmake"
- COMMAND "${CMAKE_COMMAND}"
- -DCMAKE_INSTALL_COMPONENT=unwind
- -DCMAKE_INSTALL_PREFIX="${LIBCXXABI_TESTING_INSTALL_PREFIX}"
- -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
- add_dependencies(cxxabi-test-depends libcxxabi-install-unwind-for-testing)
+ list(APPEND libcxxabi_test_suite_install_targets unwind-headers unwind)
endif()
+foreach(target IN LISTS libcxxabi_test_suite_install_targets)
+ add_custom_target(libcxxabi-test-suite-install-${target} DEPENDS "${target}"
+ COMMAND "${CMAKE_COMMAND}" --install "${CMAKE_BINARY_DIR}"
+ --prefix "${LIBCXXABI_TESTING_INSTALL_PREFIX}"
+ --component "${target}")
+ add_dependencies(cxxabi-test-depends libcxxabi-test-suite-install-${target})
+endforeach()
pythonize_bool(LIBCXXABI_USE_LLVM_UNWINDER)
|
arichardson
approved these changes
Dec 9, 2025
Member
arichardson
left a comment
arichardson
left a comment
There was a problem hiding this comment.
Looks much simpler. Nice! Not tested it locally but lgtm assuming the tests pass
Member
Author
I think I am running into race conditions between the install targets in the CI. I may have to artificially serialize these installations, which IMO is OK. |
Member
Does that mean we have multiple install steps trying to write the same file? The one CI failure I see is This suggests to me that we are just missing a dependency somewhere? |
arichardson
reviewed
Dec 10, 2025
arichardson
reviewed
Dec 10, 2025
arichardson
approved these changes
Dec 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on comments in #171474, it was brought to my attention that we can modernize and simplify how we perform the test suite installation in libc++ and libc++abi.