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++] Run the Lit test suite against an installed version of the library #96910

Merged

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Jun 27, 2024

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.

@ldionne ldionne requested a review from a team as a code owner June 27, 2024 13:54
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jun 27, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jun 27, 2024

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

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.


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

4 Files Affected:

  • (modified) libcxx/modules/CMakeLists.txt (-1)
  • (modified) libcxx/src/CMakeLists.txt (-2)
  • (modified) libcxx/test/CMakeLists.txt (+56)
  • (modified) libcxx/test/configs/cmake-bridge.cfg.in (+4-4)
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 9e6c70335a794..caaaeff309f53 100644
--- a/libcxx/src/CMakeLists.txt
+++ b/libcxx/src/CMakeLists.txt
@@ -322,7 +322,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
@@ -370,7 +369,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@'))

@ldionne
Copy link
Member Author

ldionne commented Jun 28, 2024

@mstorsjo The following Windows job is failing: https://github.com/llvm/llvm-project/actions/runs/9703339701/job/26781775745?pr=96910

ninja: error: 'libcxx/test/unwind-headers', needed by 'libcxx/test/CMakeFiles/install-unwind-test-suite-prefix', missing and no known rule to make it
ninja: Entering directory `D:/a/llvm-project/llvm-project/build/clang-cl-dll'
##[error]Process completed with exit code 1.

It is failing because we try installing the install-unwind-test-suite-prefix target which depends on unwind-headers and unwind, however that whole code block is gated on LIBCXXABI_USE_LLVM_UNWINDER. On Windows though, it looks like you're not building libunwind at all, so I don't understand why that code path is taken. Do you understand what's going on?

@mstorsjo
Copy link
Member

It is failing because we try installing the install-unwind-test-suite-prefix target which depends on unwind-headers and unwind, however that whole code block is gated on LIBCXXABI_USE_LLVM_UNWINDER. On Windows though, it looks like you're not building libunwind at all

Side note, we do build libunwind on Windows. We don't build it in clang-cl configs, but we do build it in mingw configs.

so I don't understand why that code path is taken. Do you understand what's going on?

In https://github.com/llvm/llvm-project/blob/main/libcxx/CMakeLists.txt#L279 you have option(LIBCXXABI_USE_LLVM_UNWINDER "Build and use the LLVM unwinder." ON) - which defaults this variable to ON, regardless of whether either libcxxabi or libunwind even are enabled. In this PR, you're checking if (LIBCXXABI_USE_LLVM_UNWINDER) and reference libunwind within that.

@ldionne
Copy link
Member Author

ldionne commented Jul 2, 2024

so I don't understand why that code path is taken. Do you understand what's going on?

In https://github.com/llvm/llvm-project/blob/main/libcxx/CMakeLists.txt#L279 you have option(LIBCXXABI_USE_LLVM_UNWINDER "Build and use the LLVM unwinder." ON) - which defaults this variable to ON, regardless of whether either libcxxabi or libunwind even are enabled. In this PR, you're checking if (LIBCXXABI_USE_LLVM_UNWINDER) and reference libunwind within that.

I feel a bit dumb. Thanks for pointing out the obvious to me 🙂

@ldionne ldionne force-pushed the review/run-test-suite-against-installation branch from 3978ae8 to 132ef17 Compare July 2, 2024 20:50
@ldionne
Copy link
Member Author

ldionne commented Jul 16, 2024

@mstorsjo We are still seeing strange failures on Windows and MinGW after fixing the obvious. Windows and MinGW both fail with different modes. The MinGW failures look like (from https://github.com/llvm/llvm-project/actions/runs/9779406851/job/26999519602?pr=96910):

********************
FAIL: llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.expected/monadic/transform.pass.cpp (8399 of 9666)
******************** TEST 'llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.expected/monadic/transform.pass.cpp' FAILED ********************
Exit Code: 4294967295

Command Output (stdout):
--
# COMPILED WITH
C:/llvm-mingw/bin/c++.exe D:\a\llvm-project\llvm-project\libcxx\test\std\utilities\expected\expected.expected\monadic\transform.pass.cpp  --target=x86_64-w64-windows-gnu -nostdinc++ -I D:/a/llvm-project/llvm-project/build/mingw-dll/libcxx/test-suite-install/include/c++/v1 -I D:/a/llvm-project/llvm-project/build/mingw-dll/libcxx/test-suite-install/include/c++/v1 -I D:/a/llvm-project/llvm-project/libcxx/test/support -std=c++26 -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-noexcept-type -Wno-atomic-alignment -Wno-reserved-module-identifier -Wdeprecated-copy -Wdeprecated-copy-dtor -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -Wno-local-type-template-args -Wno-c++11-extensions -Wno-unknown-pragmas -Wno-pass-failed -Wno-mismatched-new-delete -Wno-redundant-move -Wno-self-move -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_NONE -Werror=thread-safety -Wuser-defined-warnings -DTEST_WINDOWS_DLL  -lc++experimental -nostdlib++ -L D:/a/llvm-project/llvm-project/build/mingw-dll/libcxx/test-suite-install/lib -lc++ -o D:\a\llvm-project\llvm-project\build\mingw-dll\test\std\utilities\expected\expected.expected\monadic\Output\transform.pass.cpp.dir\t.tmp.exe
# note: command had no output on stdout or stderr
# EXECUTED AS
'C:\hostedtoolcache\windows\Python\3.9.13\x64\python3.exe' 'D:\a\llvm-project\llvm-project\libcxx\utils\run.py' --execdir D:\a\llvm-project\llvm-project\build\mingw-dll\test\std\utilities\expected\expected.expected\monadic\Output\transform.pass.cpp.dir --prepend_env PATH=D:/a/llvm-project/llvm-project/build/mingw-dll/libcxx/test-suite-install/lib --  D:\a\llvm-project\llvm-project\build\mingw-dll\test\std\utilities\expected\expected.expected\monadic\Output\transform.pass.cpp.dir\t.tmp.exe
# note: command had no output on stdout or stderr
# error: command failed with exit status: 0xffffffff

On MinGW we see only 33 failures:

  llvm-libc++-mingw.cfg.in :: libcxx/input.output/iostreams.base/ios.base/ios.base.cons/dtor.uninitialized.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/input.output/iostream.format/ext.manip/get_money.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/input.output/iostream.format/ext.manip/put_money.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/input.output/iostream.format/input.streams/istream.unformatted/sync.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/input.output/iostream.format/output.streams/ostream.formatted/ostream.inserters.arithmetic/long_double.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_double.hex.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_long_double.hex.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_long_double.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/strings/string.conversions/to_string.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/strings/string.conversions/to_wstring.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/time/time.duration/time.duration.nonmember/ostream.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.bad/ctor.error.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.bad/error.member.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.bad/void-specialization.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.bad/what.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.expected/assign/assign.U.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.expected/assign/assign.copy.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.expected/assign/assign.move.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.expected/assign/emplace.intializer_list.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.expected/assign/emplace.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.expected/ctor/ctor.convert.copy.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.expected/ctor/ctor.convert.move.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.expected/ctor/ctor.copy.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.expected/ctor/ctor.default.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.expected/ctor/ctor.inplace.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.expected/ctor/ctor.inplace_init_list.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.expected/ctor/ctor.move.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.expected/ctor/ctor.u.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.expected/monadic/transform.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.expected/observers/arrow.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.expected/observers/deref.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.expected/observers/value.pass.cpp
  llvm-libc++-mingw.cfg.in :: std/utilities/expected/expected.void/observers/value.pass.cpp

Can you think about an obvious reason why these tests would fail when we start running against the installed library?

@mstorsjo
Copy link
Member

Can you think about an obvious reason why these tests would fail when we start running against the installed library?

Nothing obvious that comes to mind right now… I can try to have a look at this by the end of the week, or next week - I’m on vacation at the moment, with limited time/access to things, but I’ll try to tend to it when possible.

@ldionne
Copy link
Member Author

ldionne commented Jul 16, 2024

Can you think about an obvious reason why these tests would fail when we start running against the installed library?

Nothing obvious that comes to mind right now… I can try to have a look at this by the end of the week, or next week - I’m on vacation at the moment, with limited time/access to things, but I’ll try to tend to it when possible.

No worries. Please don't work on this while you're on vacation.

@ldionne ldionne force-pushed the review/run-test-suite-against-installation branch from 68ea814 to a45785a Compare July 18, 2024 20:52
@mstorsjo
Copy link
Member

@mstorsjo We are still seeing strange failures on Windows and MinGW after fixing the obvious. Windows and MinGW both fail with different modes. The MinGW failures look like (from https://github.com/llvm/llvm-project/actions/runs/9779406851/job/26999519602?pr=96910):

On MinGW we see only 33 failures:

The reason for this, is that the built libc++ DLL actually isn't found at runtime. In the clang-cl configurations, that means that essentially all tests fail. In the MinGW cases, there's an older libc++ DLL from the toolchain that gets found and used instead, which explains why there's only a handful of failures. Also, in static linking mode, all tests pass.

The reason why the DLL isn't found, is because in an installed tree, the DLL gets installed into <prefix>/bin, while we're adding <prefix>/lib to the search path. In the intermediate build tree, as things were before, the DLL was located in the lib directory.

In llvm-libc++-shared-clangcl.cfg.in and llvm-libc++-mingw.cfg.in, we have --prepend_env PATH=%{lib-dir}; we need to add another substitution in cmake-bridge.cfg.in, e.g. %{runtime-dir} which gets expanded to @LIBCXX_TESTING_INSTALL_PREFIX@/@LIBCXX_INSTALL_RUNTIME_DIR@, and use that instead of %{lib-dir} for the --prepend_env PATH=....

@ldionne
Copy link
Member Author

ldionne commented Jul 24, 2024

Thanks a lot for the clear explanation!

@ldionne ldionne force-pushed the review/run-test-suite-against-installation branch 2 times, most recently from 23fc88b to 54b6392 Compare July 26, 2024 15:02
@ldionne
Copy link
Member Author

ldionne commented Jul 26, 2024

@philnik777 @mordante @EricWF I'd like someone to take a look at this. IMO this is an important improvement to the way we conceptually run the test suite: we run it on the result of installing the library, which means we test exactly what we ship.

However, this adds some verbosity to the build due to some CMake limitations and I'd like someone else to try it out and let me know what they think. Specifically, when running cxx-test-depends, we will now install the full library every time, which leads to a lot of output about installing headers to the "fake install location".

@ldionne ldionne force-pushed the review/run-test-suite-against-installation branch from 54b6392 to 5235278 Compare August 2, 2024 19:41
@mordante
Copy link
Member

mordante commented Aug 3, 2024

I had a quick look at the code, before looking closer and doing some testing I've question.

I see no documentation changes, does this mean there is no difference in the work-flow with this patch? Specifically the information on this page https://libcxx.llvm.org/TestingLibcxx.html.

@ldionne
Copy link
Member Author

ldionne commented Aug 5, 2024

I had a quick look at the code, before looking closer and doing some testing I've question.

I see no documentation changes, does this mean there is no difference in the work-flow with this patch? Specifically the information on this page https://libcxx.llvm.org/TestingLibcxx.html.

That is correct, this doesn't affect the workflow at all. The only difference is that you'll notice that we install the library to a fake location and the paths that we use for testing (e.g. to refer to include/c++/v1) are slightly different, since they point to the "fake installation root" instead of pointing within the build directory.

…ibrary

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.
@ldionne ldionne force-pushed the review/run-test-suite-against-installation branch from 5235278 to ebac193 Compare August 19, 2024 17:04
@ldionne
Copy link
Member Author

ldionne commented Aug 19, 2024

I'd like to merge this soon to unblock other work. @mordante did you have any blocking feedback on this PR?

@ldionne ldionne merged commit 0e8208e into llvm:main Aug 28, 2024
56 checks passed
@ldionne ldionne deleted the review/run-test-suite-against-installation branch August 28, 2024 13:23
@ldionne
Copy link
Member Author

ldionne commented Aug 28, 2024

I merged this -- I will address any feedback post-commit.

@llvm-ci
Copy link
Collaborator

llvm-ci commented Aug 28, 2024

LLVM Buildbot has detected a new failure on builder llvm-clang-win-x-aarch64 running on as-builder-2 while building libcxx at step 15 "test-libc++".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/193/builds/2254

Here is the relevant piece of the build log for the reference
Step 15 (test-libc++) failure: Test just built components: libc++ completed (failure)
******************** TEST 'llvm-libc++-static.cfg.in :: C:\buildbot\as-builder-2\x-aarch64\build\runtimes\runtimes-aarch64-unknown-linux-gnu-bins\test\libcxx\double_include.gen.py\bit.sh.cpp' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 4
C:/buildbot/as-builder-2/x-aarch64/build/./bin/clang++.exe -c C:\buildbot\as-builder-2\x-aarch64\build\runtimes\runtimes-aarch64-unknown-linux-gnu-bins\test\libcxx\double_include.gen.py\bit.sh.cpp -o C:\buildbot\as-builder-2\x-aarch64\build\runtimes\runtimes-aarch64-unknown-linux-gnu-bins\test\libcxx\double_include.gen.py\Output\bit.sh.cpp.dir\t.tmp.first.o -pthread --target=aarch64-unknown-linux-gnu -nostdinc++ -I C:/buildbot/as-builder-2/x-aarch64/build/runtimes/runtimes-aarch64-unknown-linux-gnu-bins/libcxx/test-suite-install/include/c++/v1 -I C:/buildbot/as-builder-2/x-aarch64/build/runtimes/runtimes-aarch64-unknown-linux-gnu-bins/libcxx/test-suite-install/include/aarch64-unknown-linux-gnu/c++/v1 -I C:/buildbot/as-builder-2/x-aarch64/llvm-project/libcxx/test/support -std=c++26 -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-noexcept-type -Wno-atomic-alignment -Wno-reserved-module-identifier -Wdeprecated-copy -Wdeprecated-copy-dtor -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -Wno-local-type-template-args -Wno-c++11-extensions -Wno-unknown-pragmas -Wno-pass-failed -Wno-mismatched-new-delete -Wno-redundant-move -Wno-self-move -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_NONE -Werror=thread-safety -Wuser-defined-warnings
# executed command: C:/buildbot/as-builder-2/x-aarch64/build/./bin/clang++.exe -c 'C:\buildbot\as-builder-2\x-aarch64\build\runtimes\runtimes-aarch64-unknown-linux-gnu-bins\test\libcxx\double_include.gen.py\bit.sh.cpp' -o 'C:\buildbot\as-builder-2\x-aarch64\build\runtimes\runtimes-aarch64-unknown-linux-gnu-bins\test\libcxx\double_include.gen.py\Output\bit.sh.cpp.dir\t.tmp.first.o' -pthread --target=aarch64-unknown-linux-gnu -nostdinc++ -I C:/buildbot/as-builder-2/x-aarch64/build/runtimes/runtimes-aarch64-unknown-linux-gnu-bins/libcxx/test-suite-install/include/c++/v1 -I C:/buildbot/as-builder-2/x-aarch64/build/runtimes/runtimes-aarch64-unknown-linux-gnu-bins/libcxx/test-suite-install/include/aarch64-unknown-linux-gnu/c++/v1 -I C:/buildbot/as-builder-2/x-aarch64/llvm-project/libcxx/test/support -std=c++26 -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-noexcept-type -Wno-atomic-alignment -Wno-reserved-module-identifier -Wdeprecated-copy -Wdeprecated-copy-dtor -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -Wno-local-type-template-args -Wno-c++11-extensions -Wno-unknown-pragmas -Wno-pass-failed -Wno-mismatched-new-delete -Wno-redundant-move -Wno-self-move -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_NONE -Werror=thread-safety -Wuser-defined-warnings
# .---command stderr------------
# | C:\buildbot\as-builder-2\x-aarch64\build\runtimes\runtimes-aarch64-unknown-linux-gnu-bins\test\libcxx\double_include.gen.py\bit.sh.cpp:9:10: fatal error: 'bit' file not found
# |     9 | #include <bit>
# |       |          ^~~~~
# | 1 error generated.
# `-----------------------------
# error: command failed with exit status: 1

--

********************


@ldionne
Copy link
Member Author

ldionne commented Aug 28, 2024

@vvereschaka It looks like that bot might not be building the cxx-test-depends target before it runs the test suite, is that possible?

@vvereschaka
Copy link
Contributor

It is possible. The bot uses the following command to run the test suites

python bin/llvm-lit.py -v -vv --threads=32 runtimes/runtimes-aarch64-unknown-linux-gnu-bins/libcxx/test

instead of using check-libcxx-aarch64-unknown-linux-gnu.

I don't remember the reason of it already, but initially it was changed to use the direct execution of the tests by python bin/llvm-lit.py ....

@vvereschaka
Copy link
Contributor

I see the failed ibc++ test of the aarch64 cross toolchain builder.
I'll test the current status of the check-<libcxx|libcxxabi|libunwind>-<target-triple> check targets for the toolchain builds and update the bots if it works/fixes the failures.

@ldionne
Copy link
Member Author

ldionne commented Aug 28, 2024

Ack. Please let me know. This definitely looks like the bot doing something slightly wrong, I don't think that's a problem with the patch. But let's discuss in case the fix isn't obvious.

@llvm-ci
Copy link
Collaborator

llvm-ci commented Aug 28, 2024

LLVM Buildbot has detected a new failure on builder llvm-clang-win-x-armv7l running on as-builder-1 while building libcxx at step 15 "test-libc++".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/38/builds/339

Here is the relevant piece of the build log for the reference
Step 15 (test-libc++) failure: Test just built components: libc++ completed (failure)
FAIL: llvm-libc++-static.cfg.in :: C:\buildbot\as-builder-1\x-armv7l\build\runtimes\runtimes-armv7-unknown-linux-gnueabihf-bins\test\libcxx\double_include.gen.py\barrier.sh.cpp


5c4lar pushed a commit to 5c4lar/llvm-project that referenced this pull request Aug 29, 2024
…ibrary (llvm#96910)

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.
qiaojbao pushed a commit to GPUOpen-Drivers/llvm-project that referenced this pull request Sep 30, 2024
…515f18a36

Local branch amd-gfx 4ce515f Merged main:69c43468d3f21df6232fda0530f03f18b0f40345 into amd-gfx:45ac84d24a03
Remote branch main 0e8208e [libc++] Run the Lit test suite against an installed version of the library (llvm#96910)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants