Remove LCOV merger dependency of cc_test without coverage (#17004)

fmeum · web-flow · commit 3a13af41034e · 2022-12-13T10:23:10.000+01:00
When coverage is disabled, `cc_test` should not depend on the Java LCOV merger tool. This is achieved by using `configuration_field`. Also depend on coverage tools in `exec` rather than `target` configuration, matching Java rules as well as the once-per-build coverage report generator. Fixes #16961 Closes #16994. PiperOrigin-RevId: 494941601 Change-Id: Ie614de713b87bb66d869515676698f1a71cb106e
diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_test.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_test.bzl
@@ -137,7 +137,7 @@ def make_cc_test(with_linkstatic = False, with_aspects = False):
             "stripped_binary": "%{name}.stripped",
             "dwp_file": "%{name}.dwp",
         },
-        fragments = ["google_cpp", "cpp"],
+        fragments = ["google_cpp", "cpp", "coverage"],
         exec_groups = {
             "cpp_link": exec_group(copy_from_rule = True),
         },
diff --git a/src/main/starlark/builtins_bzl/common/cc/semantics.bzl b/src/main/starlark/builtins_bzl/common/cc/semantics.bzl
@@ -80,14 +80,14 @@ def _get_test_malloc_attr():
 def _get_coverage_attrs():
     return {
         "_lcov_merger": attr.label(
-            default = "@bazel_tools//tools/test:lcov_merger",
+            default = configuration_field(fragment = "coverage", name = "output_generator"),
             executable = True,
-            cfg = "target",
+            cfg = "exec",
         ),
         "_collect_cc_coverage": attr.label(
             default = "@bazel_tools//tools/test:collect_cc_coverage",
             executable = True,
-            cfg = "target",
+            cfg = "exec",
         ),
     }
 
diff --git a/src/test/shell/bazel/cc_integration_test.sh b/src/test/shell/bazel/cc_integration_test.sh
@@ -1752,4 +1752,46 @@ EOF
   bazel build //:main --repo_env=CC=clang || fail "Expected compiler flag to have value 'clang'"
 }
 
+function test_cc_test_no_target_coverage_dep() {
+  # Regression test for https://github.com/bazelbuild/bazel/issues/16961
+  local package="${FUNCNAME[0]}"
+  mkdir -p "${package}"
+
+  cat > "${package}"/BUILD.bazel <<'EOF'
+cc_test(
+  name = "test",
+  srcs = ["test.cc"],
+)
+EOF
+  touch "${package}"/test.cc
+
+  out=$(bazel cquery --collect_code_coverage \
+   "deps(//${package}:test) intersect config(@remote_coverage_tools//:all, target)")
+  if [[ -n "$out" ]]; then
+    fail "Expected no dependency on lcov_merger in the target configuration, but got: $out"
+  fi
+}
+
+function test_cc_test_no_lcov_merger_dep_without_coverage() {
+  # Regression test for https://github.com/bazelbuild/bazel/issues/16961
+  local package="${FUNCNAME[0]}"
+  mkdir -p "${package}"
+
+  cat > "${package}"/BUILD.bazel <<'EOF'
+cc_test(
+  name = "test",
+  srcs = ["test.cc"],
+)
+EOF
+  touch "${package}"/test.cc
+
+  # FIXME: cc_test still unconditionally depends on the LCOV merger binary through
+  #  @remote_coverage_tools//:coverage_output_generator, which is also unnecessary:
+  #  https://github.com/bazelbuild/bazel/issues/15088
+  out=$(bazel cquery "somepath(//${package}:test,@remote_coverage_tools//:lcov_merger)")
+  if [[ -n "$out" ]]; then
+    fail "Expected no dependency on lcov_merger, but got: $out"
+  fi
+}
+
 run_suite "cc_integration_test"
