[Lldb-commits] [lldb] r316800 - [CMake] Build clang as dependency when using in-tree clang for tests.

[Pavel Labath via lldb-commits]

The issue with the generator expressions is that they are expanded in
only in specific target properties, and here we need them to be
expanded in configure_lit_site_cfg. If you know the right incantation
to make that happen, then please let me know.

Deriving the c++ compiler name is a bit tricky, because the name is
very variable (sometimes it has a version suffix, sometimes it has a
triple prefix), but it is doable.

On 17 November 2017 at 21:44, Chris Bieneman <chris.bieneman at me.com> wrote:
> A few notes on this change.
>
>> On Oct 27, 2017, at 2:22 PM, Davide Italiano via lldb-commits <lldb-commits at lists.llvm.org> wrote:
>>
>> Author: davide
>> Date: Fri Oct 27 14:22:57 2017
>> New Revision: 316800
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=316800&view=rev
>> Log:
>> [CMake] Build clang as dependency when using in-tree clang for tests.
>>
>> Discussed with Zachary Turner and Pavel Labath on lldb-dev.
>> Let's hope this doesn't break anything :)
>>
>> Modified:
>>    lldb/trunk/lit/CMakeLists.txt
>>    lldb/trunk/test/CMakeLists.txt
>>
>> Modified: lldb/trunk/lit/CMakeLists.txt
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/CMakeLists.txt?rev=316800&r1=316799&r2=316800&view=diff
>> ==============================================================================
>> --- lldb/trunk/lit/CMakeLists.txt (original)
>> +++ lldb/trunk/lit/CMakeLists.txt Fri Oct 27 14:22:57 2017
>> @@ -54,6 +54,12 @@ add_lit_testsuite(check-lldb-lit "Runnin
>>
>> set_target_properties(check-lldb-lit PROPERTIES FOLDER "LLDB tests")
>>
>> +# If we're building with an in-tree clang, then list clang as a dependency
>> +# to run tests.
>> +if (TARGET clang)
>> +  add_dependencies(check-lldb-lit clang)
>> +endif()
>
> This is unnecessary. Higher up in the file clang is added to LLDB_TEST_DEPS, which gets setup correctly.
>
>> +
>> add_lit_testsuites(LLDB ${CMAKE_CURRENT_SOURCE_DIR}
>>   PARAMS lldb_site_config=${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg
>>          lldb_unit_site_config=${CMAKE_CURRENT_BINARY_DIR}/Unit/lit.site.cfg
>>
>> Modified: lldb/trunk/test/CMakeLists.txt
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/CMakeLists.txt?rev=316800&r1=316799&r2=316800&view=diff
>> ==============================================================================
>> --- lldb/trunk/test/CMakeLists.txt (original)
>> +++ lldb/trunk/test/CMakeLists.txt Fri Oct 27 14:22:57 2017
>> @@ -109,6 +109,12 @@ add_python_test_target(check-lldb
>>   "Testing LLDB (parallel execution, with a separate subprocess per test)"
>>   )
>>
>> +# If we're building with an in-tree clang, then list clang as a dependency
>> +# to run tests.
>> +if (TARGET clang)
>> +  add_dependencies(check-lldb clang)
>> +endif()
>
> This is only necessary because Pavel's change (r316728) broke it. Prior to Pavel's change we specified the in-tree compiler using a generator expression which is more robust and properly sets up a dependency.
>
> It would be much better to remove this and fix Pavel's patch to use generator expressions for finding the compiler. The fix for Pavel's patch would be to set `LLDB_DEFAULT_TEST_C_COMPILER` to `$<TARGET_FILE:clang>` instead of trying to hand munge the path. We also should derive the CXX compiler from the C compiler as it is pretty unnecessary to define them separately.
>
> -Chris
>
>> +
>> add_custom_target(lldb-test-depends DEPENDS ${LLDB_TEST_DEPENDS})
>> # This will add LLDB's test dependencies to the depenednecies for check-all and
>> # include them in the test-depends target.
>>
>>
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>