From 4596cd230d34d44fce94c7af12e058a73ec892da Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Wed, 4 Oct 2023 18:30:41 -0700 Subject: [PATCH] Avoid building the non-test file group in :all. (#3191) This file group exists to allow a `genquery` rule and a Python test to verify our non-test dependency graph. We don't actually need to build the binaries in the file group as part of that. The `genquery` rule seems to do the right thing -- building it directly doesn't cause the binaries in the group to be built. But without a manual tag, the group itself is part of `:all` and thus part of `//...` and part of the rules that will be built even with PR #3106. A consequence is that any change to the toolchain causes several other binaries to be built as well because this file group is in the impacted set. Making it manual should avoid all of this, and without breaking the actual use from `genquery`. For example, before this change, in a fully cached build after a `bazel clean`: ``` > bazel test //bazel/check_deps:all INFO: Invocation ID: 2d83ebee-4c00-425d-be33-23f42b079614 INFO: Analyzed 3 targets (103 packages loaded, 7137 targets configured). INFO: Found 2 targets and 1 test target... INFO: Elapsed time: 4.081s, Critical Path: 2.61s INFO: 3111 processes: 2796 disk cache hit, 315 internal. INFO: Build completed successfully, 3111 total actions ``` After this change: ``` > bazel test //bazel/check_deps:al INFO: Invocation ID: c94089e8-a420-4d3c-9902-134e6b55b297 INFO: Analyzed 2 targets (92 packages loaded, 568 targets configured). INFO: Found 1 target and 1 test target... INFO: Elapsed time: 0.700s, Critical Path: 0.01s INFO: 7 processes: 2 disk cache hit, 5 internal. INFO: Build completed successfully, 7 total actions ``` While here, re-generate the file group, and fix several issues it uncovers: mark test utilities as `testonly` and update our LLVM package allowlist to include `clangd`'s package. --- bazel/check_deps/BUILD | 4 +++- bazel/check_deps/check_non_test_cc_deps.py | 2 +- common/BUILD | 1 + language_server/BUILD | 2 ++ utils/treesitter/BUILD | 6 ++++++ 5 files changed, 13 insertions(+), 2 deletions(-) diff --git a/bazel/check_deps/BUILD b/bazel/check_deps/BUILD index 54280aa07cd7..96d735a4488c 100644 --- a/bazel/check_deps/BUILD +++ b/bazel/check_deps/BUILD @@ -15,11 +15,13 @@ filegroup( data = [ "//explorer", "//installers/local:carbon", + "//language_server", "//migrate_cpp:rewriter", "//migrate_cpp/cpp_refactoring", - "//toolchain/diagnostics:null_diagnostics", "//toolchain/driver:carbon", + "//utils/treesitter", ], + tags = ["manual"], ) genquery( diff --git a/bazel/check_deps/check_non_test_cc_deps.py b/bazel/check_deps/check_non_test_cc_deps.py index 49aa7fa9ac42..a364d777ba71 100644 --- a/bazel/check_deps/check_non_test_cc_deps.py +++ b/bazel/check_deps/check_non_test_cc_deps.py @@ -42,7 +42,7 @@ for dep in deps: # Other packages in the LLVM project shouldn't be accidentally used # in Carbon. We can expand the above list if use cases emerge. - if package not in ("llvm", "lld", "clang"): + if package not in ("llvm", "lld", "clang", "clang-tools-extra/clangd"): sys.exit( "ERROR: unexpected dependency into the LLVM project: %s" % dep ) diff --git a/common/BUILD b/common/BUILD index 09dc556c04d3..20e84374bc6b 100644 --- a/common/BUILD +++ b/common/BUILD @@ -71,6 +71,7 @@ cc_library( cc_library( name = "enum_base_test_def", + testonly = 1, textual_hdrs = ["enum_base_test.def"], ) diff --git a/language_server/BUILD b/language_server/BUILD index 69e73fdfd515..6e9cf560e140 100644 --- a/language_server/BUILD +++ b/language_server/BUILD @@ -5,6 +5,8 @@ load("@rules_cc//cc:defs.bzl", "cc_binary") package(default_visibility = [ + "//bazel/check_deps:__pkg__", + "//installers:__subpackages__", "//language_server:__subpackages__", ]) diff --git a/utils/treesitter/BUILD b/utils/treesitter/BUILD index 61d540f192e0..a0f9e029af48 100644 --- a/utils/treesitter/BUILD +++ b/utils/treesitter/BUILD @@ -5,6 +5,11 @@ load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_test") load("@rules_tree_sitter//tree_sitter:tree_sitter.bzl", "tree_sitter_cc_library") +package(default_visibility = [ + "//bazel/check_deps:__pkg__", + "//installers:__subpackages__", +]) + tree_sitter_cc_library( name = "treesitter", srcs = ["src/scanner.c"], @@ -13,6 +18,7 @@ tree_sitter_cc_library( cc_binary( name = "test_runner", + testonly = 1, srcs = ["test_runner.cpp"], deps = [ ":treesitter",