[clang-tidy] Enable plugin tests with LLVM_INSTALL_TOOLCHAIN_ONLY

[igorkudrin]

The only reason for the removed condition was that there was a dependency for CTTestTidyModule on the clang-tidy-headers target, which was only created under the same NOT LLVM_INSTALL_TOOLCHAIN_ONLY condition. It looks like this target is not needed for CTTestTidyModule to build and run, so the dependency can be removed along with the condition.

See also https://reviews.llvm.org/D111100 for earlier discussions.

[llvmbot]

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: Igor Kudrin (igorkudrin)

Changes

The only reason for the removed condition was that there was a dependency for CTTestTidyModule on the clang-tidy-headers target, which was only created under the same NOT LLVM_INSTALL_TOOLCHAIN_ONLY condition. It looks like this target is not needed for CTTestTidyModule to build and run, so the dependency can be removed along with the condition.

See also https://reviews.llvm.org/D111100 for earlier discussions.

---

Full diff: https://github.com/llvm/llvm-project/pull/90370.diff

2 Files Affected:

• (modified) clang-tools-extra/test/CMakeLists.txt (+21-24)
• (modified) clang-tools-extra/test/lit.site.cfg.py.in (+1-1)

diff --git a/clang-tools-extra/test/CMakeLists.txt b/clang-tools-extra/test/CMakeLists.txt
index f4c529ee8af207..7a1c168e22f97c 100644
--- a/clang-tools-extra/test/CMakeLists.txt
+++ b/clang-tools-extra/test/CMakeLists.txt
@@ -67,33 +67,30 @@ foreach(dep ${LLVM_UTILS_DEPS})
   endif()
 endforeach()
 
-if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
-  if (NOT WIN32 OR NOT LLVM_LINK_LLVM_DYLIB)
-    llvm_add_library(
-        CTTestTidyModule
-        MODULE clang-tidy/CTTestTidyModule.cpp
-        PLUGIN_TOOL clang-tidy
-        DEPENDS clang-tidy-headers)
-  endif()
+if (NOT WIN32 OR NOT LLVM_LINK_LLVM_DYLIB)
+  llvm_add_library(
+      CTTestTidyModule
+      MODULE clang-tidy/CTTestTidyModule.cpp
+      PLUGIN_TOOL clang-tidy)
+endif()
 
-  if(CLANG_BUILT_STANDALONE)
-    # LLVMHello library is needed below
-    if (EXISTS ${LLVM_MAIN_SRC_DIR}/lib/Transforms/Hello
-       AND NOT TARGET LLVMHello)
-      add_subdirectory(${LLVM_MAIN_SRC_DIR}/lib/Transforms/Hello
-        lib/Transforms/Hello)
-    endif()
+if(CLANG_BUILT_STANDALONE)
+  # LLVMHello library is needed below
+  if (EXISTS ${LLVM_MAIN_SRC_DIR}/lib/Transforms/Hello
+      AND NOT TARGET LLVMHello)
+    add_subdirectory(${LLVM_MAIN_SRC_DIR}/lib/Transforms/Hello
+      lib/Transforms/Hello)
   endif()
+endif()
 
-  if(TARGET CTTestTidyModule)
-      list(APPEND CLANG_TOOLS_TEST_DEPS CTTestTidyModule LLVMHello)
-      target_include_directories(CTTestTidyModule PUBLIC BEFORE "${CLANG_TOOLS_SOURCE_DIR}")
-      if(CLANG_PLUGIN_SUPPORT AND (WIN32 OR CYGWIN))
-        set(LLVM_LINK_COMPONENTS
-          Support
-        )
-      endif()
-  endif()
+if(TARGET CTTestTidyModule)
+    list(APPEND CLANG_TOOLS_TEST_DEPS CTTestTidyModule LLVMHello)
+    target_include_directories(CTTestTidyModule PUBLIC BEFORE "${CLANG_TOOLS_SOURCE_DIR}")
+    if(CLANG_PLUGIN_SUPPORT AND (WIN32 OR CYGWIN))
+      set(LLVM_LINK_COMPONENTS
+        Support
+      )
+    endif()
 endif()
 
 add_lit_testsuite(check-clang-extra "Running clang-tools-extra/test"
diff --git a/clang-tools-extra/test/lit.site.cfg.py.in b/clang-tools-extra/test/lit.site.cfg.py.in
index 4eb830a1baf1f1..e6503a4c097cac 100644
--- a/clang-tools-extra/test/lit.site.cfg.py.in
+++ b/clang-tools-extra/test/lit.site.cfg.py.in
@@ -10,7 +10,7 @@ config.python_executable = "@Python3_EXECUTABLE@"
 config.target_triple = "@LLVM_TARGET_TRIPLE@"
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.clang_tidy_staticanalyzer = @CLANG_TIDY_ENABLE_STATIC_ANALYZER@
-config.has_plugins = @CLANG_PLUGIN_SUPPORT@ & ~@LLVM_INSTALL_TOOLCHAIN_ONLY@
+config.has_plugins = @CLANG_PLUGIN_SUPPORT@
 # Support substitution of the tools and libs dirs with user parameters. This is
 # used when we can't determine the tool dir at configuration time.
 config.llvm_tools_dir = lit_config.substitute("@LLVM_TOOLS_DIR@")

[vtjnash]

I think out of tree builds of clang-tidy (back in the svn days, when people did partial checkouts of individual projects) probably needed this to be able to correctly correctly find the right headers. Maybe standalone builds aren't permitted anymore? I think I mentioned this a bit in earlier revisions of that PR https://reviews.llvm.org/D111100#3284178

[igorkudrin]

I think out of tree builds of clang-tidy (back in the svn days, when people did partial checkouts of individual projects) probably needed this to be able to correctly correctly find the right headers. Maybe standalone builds aren't permitted anymore? I think I mentioned this a bit in earlier revisions of that PR https://reviews.llvm.org/D111100#3284178

I don't have much to say about standalone builds. I hope to hear about them from the people who mentioned them in D111100. But as far as I can see, clang-tidy-headers is an empty target, with no dependencies or actions; at least it looks like that when building with ninja on Linux or Windows. It seems unlikely that this is different for standalone builds. Removing the dependency on an empty target should be safe.

[MaskRay]

This simplification seems desired. https://reviews.llvm.org/D111100#3302749 had a LLVM_INSTALL_TOOLCHAIN_ONLY condition, which seems unneeded.