compnerd added inline comments.
================ Comment at: lit/CMakeLists.txt:64 ) +if(NOT LLDB_BUILT_STANDALONE) + list(APPEND LLDB_TEST_DEPS llvm-strip) ---------------- JDevlieghere wrote: > xiaobai wrote: > > why not `if(TARGET llvm-strip)`? I think that expresses the intent more > > cleanly (and conforms to the existing pattern). > I actually ran into an issue with that today, we had the following code in > the top-level CMake list: > > ``` > if(TARGET dsymutil) > add_lldb_test_dependency(dsymutil) > endif() > ``` > Nevertheless, `ninja lldb-test-deps` didn't build dsymutil. > > ``` > $ /V/J/l/build-ra> ninja lldb-test-deps > [1/1] Python script sym-linking LLDB Python API > $ /V/J/l/build-ra> ninja dsymutil > [1/1] Linking CXX executable bin/dsymutil > ``` My slight preference for this is to actually have a good way to identify that this can be simplified once unified builds are the only supported build style. I suppose a comment along with the `if(TARGET)` check would work too. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68614/new/ https://reviews.llvm.org/D68614 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
