-
Notifications
You must be signed in to change notification settings - Fork 15.5k
[libunwind] Make sure libunwind test dependencies are installed before running tests #171474
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
[libunwind] Make sure libunwind test dependencies are installed before running tests #171474
Conversation
…e running tests This patch adds an installation step where we install libc++ in a fake installation tree before testing libunwind. This is necessary because some configurations (in particular "generic-merged") require libc++ to be installed, since the libunwind tests are actually linking libc++.so in which libc++abi.a and libunwind.a have been merged.
|
@llvm/pr-subscribers-libunwind Author: Louis Dionne (ldionne) ChangesThis patch adds an installation step where we install libc++ in a fake installation tree before testing libunwind. This is necessary because some configurations (in particular "generic-merged") require libc++ to be installed, since the libunwind tests are actually linking libc++.so in which libc++abi.a and libunwind.a have been merged. Full diff: https://github.com/llvm/llvm-project/pull/171474.diff 2 Files Affected:
diff --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt
index 97edff0b87ea3..fbef71f3f7446 100644
--- a/libunwind/CMakeLists.txt
+++ b/libunwind/CMakeLists.txt
@@ -336,6 +336,9 @@ if (RUNTIMES_EXECUTE_ONLY_CODE)
add_compile_definitions(_LIBUNWIND_EXECUTE_ONLY_CODE)
endif()
+add_custom_target(unwind-test-depends
+ COMMENT "Build dependencies required to run the libunwind test suite.")
+
#===============================================================================
# Setup Source Code
#===============================================================================
diff --git a/libunwind/test/CMakeLists.txt b/libunwind/test/CMakeLists.txt
index c222c0bdbf5af..815bc86692860 100644
--- a/libunwind/test/CMakeLists.txt
+++ b/libunwind/test/CMakeLists.txt
@@ -9,6 +9,26 @@ macro(pythonize_bool var)
endmacro()
set(LIBUNWIND_TESTING_INSTALL_PREFIX "${LIBUNWIND_BINARY_DIR}/test-suite-install")
+add_custom_target(libunwind-install-cxx-for-testing
+ DEPENDS cxx-headers
+ cxx
+ cxx_experimental
+ cxx-modules
+ COMMAND ${CMAKE_COMMAND} -E make_directory "${LIBUNWIND_TESTING_INSTALL_PREFIX}"
+ COMMAND "${CMAKE_COMMAND}"
+ -DCMAKE_INSTALL_COMPONENT=cxx-headers
+ -DCMAKE_INSTALL_PREFIX="${LIBUNWIND_TESTING_INSTALL_PREFIX}"
+ -P "${CMAKE_BINARY_DIR}/cmake_install.cmake"
+ COMMAND "${CMAKE_COMMAND}"
+ -DCMAKE_INSTALL_COMPONENT=cxx-modules
+ -DCMAKE_INSTALL_PREFIX="${LIBUNWIND_TESTING_INSTALL_PREFIX}"
+ -P "${CMAKE_BINARY_DIR}/cmake_install.cmake"
+ COMMAND "${CMAKE_COMMAND}"
+ -DCMAKE_INSTALL_COMPONENT=cxx
+ -DCMAKE_INSTALL_PREFIX="${LIBUNWIND_TESTING_INSTALL_PREFIX}"
+ -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
+add_dependencies(unwind-test-depends libunwind-install-cxx-for-testing)
+
add_custom_target(libunwind-install-unwind-for-testing
DEPENDS unwind-headers
unwind
@@ -21,6 +41,7 @@ add_custom_target(libunwind-install-unwind-for-testing
-DCMAKE_INSTALL_COMPONENT=unwind
-DCMAKE_INSTALL_PREFIX="${LIBUNWIND_TESTING_INSTALL_PREFIX}"
-P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
+add_dependencies(unwind-test-depends libunwind-install-unwind-for-testing)
pythonize_bool(LIBUNWIND_ENABLE_CET)
pythonize_bool(LIBUNWIND_ENABLE_GCS)
@@ -62,4 +83,4 @@ configure_lit_site_cfg(
add_lit_testsuite(check-unwind "Running libunwind tests"
${CMAKE_CURRENT_BINARY_DIR}
- DEPENDS libunwind-install-unwind-for-testing)
+ DEPENDS unwind-test-depends)
|
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.
Turns out I don't think this is helping at all, so remove it to avoid increasing complexity.
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.
This patch adds an installation step where we install libc++ in a fake installation tree before testing libunwind. This is necessary because some configurations (in particular "generic-merged") require libc++ to be installed, since the libunwind tests are actually linking libc++.so in which libc++abi.a and libunwind.a have been merged.
Without this, we were actually failing to find
libc++.soto link against and then linking against whatever system library we'd find in the provided search directories. While this happens to work in the current CI configuration, this breaks down when updating to newer build tools.