Page MenuHomePhabricator

[lldb] add check for libcxx runtime
ClosedPublic

Authored by rmaz on Feb 22 2021, 2:08 PM.

Details

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

Diff Detail

Event Timeline

rmaz created this revision.Feb 22 2021, 2:08 PM
rmaz requested review of this revision.Feb 22 2021, 2:08 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 22 2021, 2:08 PM
rmaz edited the summary of this revision. (Show Details)

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 edited the summary of this revision. (Show Details)Feb 22 2021, 2:41 PM

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 accepted this revision.Feb 22 2021, 3:09 PM

LGTM. Do you have commit access?

This revision is now accepted and ready to land.Feb 22 2021, 3:09 PM
JDevlieghere added inline comments.Feb 22 2021, 3:29 PM
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 updated this revision to Diff 325600.Feb 22 2021, 3:38 PM

update comment

rmaz marked an inline comment as done.Feb 22 2021, 3:39 PM
JDevlieghere accepted this revision.Feb 22 2021, 3:55 PM

Thanks! LGTM.

This revision was automatically updated to reflect the committed changes.