From 990c282ad44db5046e4dd50494e854bd2eb23115 Mon Sep 17 00:00:00 2001
From: Fabian Meumertzheim <fabian@meumertzhe.im>
Date: Tue, 13 Dec 2022 00:25:09 -0800
Subject: [PATCH] Remove LCOV merger dependency of `cc_test` without coverage

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
---
 .../builtins_bzl/common/cc/cc_test.bzl        |  2 +-
 .../builtins_bzl/common/cc/semantics.bzl      |  6 +--
 src/test/shell/bazel/cc_integration_test.sh   | 42 +++++++++++++++++++
 3 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_test.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_test.bzl
index 6540a4ff5643c9..c83582aa78f7b6 100644
--- 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
index fef1fe0559c244..58da8f90bb39ee 100644
--- a/src/main/starlark/builtins_bzl/common/cc/semantics.bzl
+++ b/src/main/starlark/builtins_bzl/common/cc/semantics.bzl
@@ -87,14 +87,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
index a317e18b2e61a5..982104158d1388 100755
--- a/src/test/shell/bazel/cc_integration_test.sh
+++ b/src/test/shell/bazel/cc_integration_test.sh
@@ -1816,4 +1816,46 @@ EOF
     --repo_env=BAZEL_CONLYOPTS=-DEXIT_CODE=0 || fail "Expected C compilation to pass"
 }
 
+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"