From 3a13af41034e1f80cc0fbc1634cf8f724a85b78f Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Tue, 13 Dec 2022 10:23:10 +0100 Subject: [PATCH] Remove LCOV merger dependency of `cc_test` without coverage (#17004) 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 aaed989b5e01c9..cedeed5b282b5c 100644 --- 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 index 038fe3ce939151..6964d20eb4e83b 100755 --- 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"