[lldb] add check for libcxx runtime

[rmaz]

Summary

When enabling LLDB tests with LLVM_ENABLE_RUNTIMES=libcxx CMake will fail with:

LLDB test suite requires libc++, but it is currently disabled.

The issue is that the targets in LLVM_ENABLE_RUNTIMES are configured after the targets in LLVM_ENABLE_PROJECTS, so at this point the check for the cxx target will fail. CMake will add a dependency for a target that does not exist yet however, so by first checking for libcxx in LLVM_ENABLE_RUNTIMES we ensure that the cxx target will be present at build time.

Tested with:

% cmake -G Ninja -C ~/local/llvm-project/lldb/cmake/caches/Apple-lldb-macOS.cmake -DLLVM_ENABLE_PROJECTS="clang;lldb" -DLLVM_ENABLE_RUNTIMES="libcxx" -DLIBCXX_INCLUDE_TESTS=NO ~/local/llvm-project/llvm
% ninja check-lldb

[smeenai]

My understanding of the issue is that, with the runtimes build, the cxx target will get created after we process LLDB's build files. The TARGET cxx check here will therefore fail, but the cxx target will end up existing later, and CMake is perfectly fine adding a dependency to a target that gets created later, so the failing check is spurious. The libcxx IN_LIST LLVM_ENABLE_RUNTIMES is basically a proxy for "the cxx target doesn't exist right now but will exist later". Could you add an explanation of the issue to the summary or the code comment?

(Note that if you change a commit message locally, arc diff won't update the summary of this review by default ... you'll need arc diff --verbatim to make it update the summary.)

[rmaz]

In D97227#2580049, @smeenai wrote:

My understanding of the issue is that, with the runtimes build, the cxx target will get created after we process LLDB's build files. The TARGET cxx check here will therefore fail, but the cxx target will end up existing later, and CMake is perfectly fine adding a dependency to a target that gets created later, so the failing check is spurious. The libcxx IN_LIST LLVM_ENABLE_RUNTIMES is basically a proxy for "the cxx target doesn't exist right now but will exist later". Could you add an explanation of the issue to the summary or the code comment?

I updated the summary, any clearer?

[smeenai]

LGTM. Do you have commit access?

[JDevlieghere]

lldb/test/CMakeLists.txt

120–121

Should we include LLVM_ENABLE_RUNTIMES in this message? E.g.

Please add libcxx to LLVM_ENABLE_PROJECTS or LLVM_ENABLE_RUNTIMES, or disable tests via ...

[rmaz]

update comment

[JDevlieghere]

Thanks! LGTM.

lldb/test/CMakeLists.txt

if(LLDB_BUILT_STANDALONE)
       # For now check that the include directory exists.
       set(cxx_dir "${LLVM_BINARY_DIR}/include/c++")
       if(NOT EXISTS ${cxx_dir})
         message(WARNING "LLDB test suite requires libc++ in llvm/projects/libcxx or an existing build symlinked to ${cxx_dir}")
       endif()
     else()
       # We require libcxx for the test suite, so if we aren't building it,
       # try to provide a helpful error about how to resolve the situation.
-      if(NOT TARGET cxx)
+      if(NOT TARGET cxx AND NOT libcxx IN_LIST LLVM_ENABLE_RUNTIMES)
         if(LLVM_ENABLE_PROJECTS STREQUAL "")
           # If `LLVM_ENABLE_PROJECTS` is not being used (implying that we are
           # using the old layout), suggest checking it out.
           message(FATAL_ERROR
             "LLDB test suite requires libc++, but it is currently disabled. "
             "Please checkout `libcxx` in `llvm/projects` or disable tests "
             "via `LLDB_INCLUDE_TESTS=OFF`.")
         else()
           # If `LLVM_ENABLE_PROJECTS` is being used, suggest adding it.
           message(FATAL_ERROR
             "LLDB test suite requires libc++, but it is currently disabled. "
-            "Please add `libcxx` to `LLVM_ENABLE_PROJECTS` or disable tests "
-            "via `LLDB_INCLUDE_TESTS=OFF`.")
+            "Please add `libcxx` to `LLVM_ENABLE_PROJECTS` or "
+            "`LLVM_ENABLE_RUNTIMES`, or disable tests via "
+            "`LLDB_INCLUDE_TESTS=OFF`.")
         endif()
       endif()
       add_lldb_test_dependency(cxx)
     endif()
   endif()
 endif()
 
 if (LLDB_BUILT_STANDALONE)