[libc++abi][libunwind] Run c++abi and unwind tests against a fake install root #110171
Conversation
This file contains 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
|
@llvm/pr-subscribers-libcxx @llvm/pr-subscribers-libcxxabi Author: Louis Dionne (ldionne) ChangesThis is what we started doing in libc++ and it straightens up a lot of things that only happened to work before, notably the presence of relative rpaths in dylibs when running from the build tree. This unlocks the ability to link against a just-built dylib but run against another version of the dylib (for example the system-provided one), which is necessary for proper backdeployment testing. This patch adds a lot of code duplication between the libc++ and libc++abi testing setups. However, there is already a large amount of duplication and the only real way to get rid of it is to merge libc++abi into libc++. In a way, this patch is a step in that direction because it closes the gap between the two libraries' testing setup. Full diff: https://github.com/llvm/llvm-project/pull/110171.diff 5 Files Affected:
diff --git a/libcxxabi/src/CMakeLists.txt b/libcxxabi/src/CMakeLists.txt
index c1a7bcb14eb199..e8c459d1c9cae6 100644
--- a/libcxxabi/src/CMakeLists.txt
+++ b/libcxxabi/src/CMakeLists.txt
@@ -306,7 +306,6 @@ endif()
# Add a meta-target for both libraries.
add_custom_target(cxxabi DEPENDS ${LIBCXXABI_BUILD_TARGETS})
-add_dependencies(cxxabi-test-depends cxxabi cxx)
if (LIBCXXABI_INSTALL_LIBRARY)
install(TARGETS ${LIBCXXABI_INSTALL_TARGETS}
diff --git a/libcxxabi/test/CMakeLists.txt b/libcxxabi/test/CMakeLists.txt
index 8e3048f2ffe8a1..9eabfb08240b60 100644
--- a/libcxxabi/test/CMakeLists.txt
+++ b/libcxxabi/test/CMakeLists.txt
@@ -8,6 +8,57 @@ macro(pythonize_bool var)
endif()
endmacro()
+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)
+
+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)
+endif()
+
pythonize_bool(LIBCXXABI_USE_LLVM_UNWINDER)
set(AUTO_GEN_COMMENT "## Autogenerated by libcxxabi configuration.\n# Do not edit!")
diff --git a/libcxxabi/test/configs/cmake-bridge.cfg.in b/libcxxabi/test/configs/cmake-bridge.cfg.in
index 3fefc6a7fdc88a..09f324933817e5 100644
--- a/libcxxabi/test/configs/cmake-bridge.cfg.in
+++ b/libcxxabi/test/configs/cmake-bridge.cfg.in
@@ -27,10 +27,10 @@ config.test_exec_root = os.path.join('@CMAKE_BINARY_DIR@', 'test')
config.host_triple = '@LLVM_HOST_TRIPLE@'
config.substitutions.append(('%{libcxx}', '@LIBCXXABI_LIBCXX_PATH@'))
-config.substitutions.append(('%{include}', '@LIBCXXABI_SOURCE_DIR@/include'))
-config.substitutions.append(('%{cxx-include}', '@LIBCXXABI_HEADER_DIR@/include/c++/v1'))
-config.substitutions.append(('%{cxx-target-include}', '@LIBCXXABI_HEADER_DIR@/include/%{triple}/c++/v1'))
-config.substitutions.append(('%{lib}', '@LIBCXXABI_LIBRARY_DIR@'))
+config.substitutions.append(('%{include}', '@LIBCXXABI_TESTING_INSTALL_PREFIX@/include'))
+config.substitutions.append(('%{cxx-include}', '@LIBCXXABI_TESTING_INSTALL_PREFIX@/@LIBCXXABI_INSTALL_INCLUDE_DIR@'))
+config.substitutions.append(('%{cxx-target-include}', '@LIBCXXABI_TESTING_INSTALL_PREFIX@/@LIBCXX_INSTALL_INCLUDE_TARGET_DIR@'))
+config.substitutions.append(('%{lib}', '@LIBCXXABI_TESTING_INSTALL_PREFIX@/@LIBCXXABI_INSTALL_LIBRARY_DIR@'))
if @LIBCXXABI_USE_LLVM_UNWINDER@:
config.substitutions.append(('%{maybe-include-libunwind}', '-I "@LIBCXXABI_LIBUNWIND_INCLUDES_INTERNAL@"'))
diff --git a/libunwind/test/CMakeLists.txt b/libunwind/test/CMakeLists.txt
index c7b1b3d01d8c7d..c222c0bdbf5af1 100644
--- a/libunwind/test/CMakeLists.txt
+++ b/libunwind/test/CMakeLists.txt
@@ -8,6 +8,20 @@ macro(pythonize_bool var)
endif()
endmacro()
+set(LIBUNWIND_TESTING_INSTALL_PREFIX "${LIBUNWIND_BINARY_DIR}/test-suite-install")
+add_custom_target(libunwind-install-unwind-for-testing
+ DEPENDS unwind-headers
+ unwind
+ COMMAND ${CMAKE_COMMAND} -E make_directory "${LIBUNWIND_TESTING_INSTALL_PREFIX}"
+ COMMAND "${CMAKE_COMMAND}"
+ -DCMAKE_INSTALL_COMPONENT=unwind-headers
+ -DCMAKE_INSTALL_PREFIX="${LIBUNWIND_TESTING_INSTALL_PREFIX}"
+ -P "${CMAKE_BINARY_DIR}/cmake_install.cmake"
+ COMMAND "${CMAKE_COMMAND}"
+ -DCMAKE_INSTALL_COMPONENT=unwind
+ -DCMAKE_INSTALL_PREFIX="${LIBUNWIND_TESTING_INSTALL_PREFIX}"
+ -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
+
pythonize_bool(LIBUNWIND_ENABLE_CET)
pythonize_bool(LIBUNWIND_ENABLE_GCS)
pythonize_bool(LIBUNWIND_ENABLE_THREADS)
@@ -48,4 +62,4 @@ configure_lit_site_cfg(
add_lit_testsuite(check-unwind "Running libunwind tests"
${CMAKE_CURRENT_BINARY_DIR}
- DEPENDS unwind)
+ DEPENDS libunwind-install-unwind-for-testing)
diff --git a/libunwind/test/configs/cmake-bridge.cfg.in b/libunwind/test/configs/cmake-bridge.cfg.in
index 7fc7a3da424629..5ccdd1772a8b3e 100644
--- a/libunwind/test/configs/cmake-bridge.cfg.in
+++ b/libunwind/test/configs/cmake-bridge.cfg.in
@@ -29,5 +29,5 @@ if not @LIBUNWIND_ENABLE_THREADS@:
config.available_features.add('libunwind-no-threads')
# Add substitutions for bootstrapping the test suite configuration
-config.substitutions.append(('%{include}', '@LIBUNWIND_SOURCE_DIR@/include'))
-config.substitutions.append(('%{lib}', '@LIBUNWIND_LIBRARY_DIR@'))
+config.substitutions.append(('%{include}', '@LIBUNWIND_TESTING_INSTALL_PREFIX@/include'))
+config.substitutions.append(('%{lib}', '@LIBUNWIND_TESTING_INSTALL_PREFIX@/lib'))
|
…tall root This is what we started doing in libc++ and it straightens up a lot of things that only happened to work before, notably the presence of relative rpaths in dylibs when running from the build tree. This unlocks the ability to link against a just-built dylib but run against another version of the dylib (for example the system-provided one), which is necessary for proper backdeployment testing. This patch adds a lot of code duplication between the libc++ and libc++abi testing setups. However, there is already a large amount of duplication and the only real way to get rid of it is to merge libc++abi into libc++. In a way, this patch is a step in that direction because it closes the gap between the two libraries' testing setup.
ldionne
force-pushed
the
review/test-cxxabi-and-unwind-against-fake-install
branch
from
554066d to
f6c1590
Compare
llvmbot
added
the
libc++
ldionne
deleted the
review/test-cxxabi-and-unwind-against-fake-install
branch
|
We are seeing a test failure on linux-x64 runtime after this patch landed. Error message: It looks like cmake pointed a wrong libunwind path to the clang. We are still investigating. |
|
Seeing same fault as @zeroomega . I think problem is related to LLVM_ENABLE_PER_TARGET_RUNTIME_DIR. When that is set to ON the install path is extended (in libunwind/CMakeLIsts.txt): And then the substitution in libunwind/test/configs/cmake-bridge.cfg.in is wrong: |
|
Thanks for the analysis. Per-target runtime dir strikes again! That setting keeps on creating issues (by no fault of its own) because it's not the default. Can we do something like this instead, then? We should fix this one forward since there's a complicated dependency chain on top of this (we switched to Github-hosted runners for macOS testing after this patch and they will start failing if we revert this). Hopefully it's not too complicated to fix forward. |
That seems to fix the test: |
VitaNuo
pushed a commit
to VitaNuo/llvm-project
that referenced
this pull request
Oct 2, 2024
…tall root (llvm#110171) This is what we started doing in libc++ and it straightens up a lot of things that only happened to work before, notably the presence of relative rpaths in dylibs when running from the build tree. This unlocks the ability to link against a just-built dylib but run against another version of the dylib (for example the system-provided one), which is necessary for proper backdeployment testing. This patch adds a lot of code duplication between the libc++ and libc++abi testing setups. However, there is already a large amount of duplication and the only real way to get rid of it is to merge libc++abi into libc++. In a way, this patch is a step in that direction because it closes the gap between the two libraries' testing setup.
VitaNuo
pushed a commit
to VitaNuo/llvm-project
that referenced
this pull request
Oct 2, 2024
…tall root (llvm#110171) This is what we started doing in libc++ and it straightens up a lot of things that only happened to work before, notably the presence of relative rpaths in dylibs when running from the build tree. This unlocks the ability to link against a just-built dylib but run against another version of the dylib (for example the system-provided one), which is necessary for proper backdeployment testing. This patch adds a lot of code duplication between the libc++ and libc++abi testing setups. However, there is already a large amount of duplication and the only real way to get rid of it is to merge libc++abi into libc++. In a way, this patch is a step in that direction because it closes the gap between the two libraries' testing setup.
xgupta
pushed a commit
to xgupta/llvm-project
that referenced
this pull request
Oct 4, 2024
…tall root (llvm#110171) This is what we started doing in libc++ and it straightens up a lot of things that only happened to work before, notably the presence of relative rpaths in dylibs when running from the build tree. This unlocks the ability to link against a just-built dylib but run against another version of the dylib (for example the system-provided one), which is necessary for proper backdeployment testing. This patch adds a lot of code duplication between the libc++ and libc++abi testing setups. However, there is already a large amount of duplication and the only real way to get rid of it is to merge libc++abi into libc++. In a way, this patch is a step in that direction because it closes the gap between the two libraries' testing setup.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is what we started doing in libc++ and it straightens up a lot of things that only happened to work before, notably the presence of relative rpaths in dylibs when running from the build tree.
This unlocks the ability to link against a just-built dylib but run against another version of the dylib (for example the system-provided one), which is necessary for proper backdeployment testing.
This patch adds a lot of code duplication between the libc++ and libc++abi testing setups. However, there is already a large amount of duplication and the only real way to get rid of it is to merge libc++abi into libc++. In a way, this patch is a step in that direction because it closes the gap between the two libraries' testing setup.