vtjnash added inline comments.

================
Comment at: clang-tools-extra/test/CMakeLists.txt:91
+  if(TARGET CTTestTidyModule)
+      list(APPEND CLANG_TOOLS_TEST_DEPS CTTestTidyModule LLVMHello)
+      target_include_directories(CTTestTidyModule PUBLIC BEFORE 
"${CLANG_TOOLS_SOURCE_DIR}")
----------------
tstellar wrote:
> aaron.ballman wrote:
> > tstellar wrote:
> > > Our stand-alone builds for Fedora are still not working after this patch, 
> > > even with D119199.  Why is it necesary to add LLVMHello as a test 
> > > dependency?  Which test is using it?
> > CTestTidyModule.cpp is using it:
> > ```
> > // RUN: clang-tidy -checks='-*,mytest*' --list-checks -load 
> > %llvmshlibdir/CTTestTidyModule%pluginext -load 
> > %llvmshlibdir/LLVMHello%pluginext | FileCheck --check-prefix=CHECK-LIST %s
> > ```
> > I *think* this is testing that we can load a clang-tidy plugin and an llvm 
> > plugin at the same time and not hit conflicting symbols or other issues. If 
> > I'm correct, then I think that's useful test functionality, but I wouldn't 
> > describe it as critical, so I'd be fine if we dropped it for now to get 
> > this patch in, and then added the extra testing in a subsequent patch if we 
> > think it's necessary.
> OK, that does sound like a useful test.  I will spend some time investigating 
> how to make this work with stand-alone builds.
Yes, that is the purpose. It is only a test dependency, so we could prevent it 
from running these tests instead. However, in theory, it should have figured 
out the relative path to the llvm/ source directory in the repo and re-built 
this target it if required for the test and not already available (with 
D119199):
https://github.com/llvm/llvm-project/blob/27f72eb25e366cf6fd79ea7495fec5d926a5b895/clang-tools-extra/test/CMakeLists.txt#L92

As this seemed to be the way other subprojects were implementing it already:
https://github.com/llvm/llvm-project/blob/27f72eb25e366cf6fd79ea7495fec5d926a5b895/clang-tools-extra/clangd/unittests/CMakeLists.txt#L9


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111100/new/

https://reviews.llvm.org/D111100

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to