Skip to content

Commit

Permalink
Remove LCOV merger dependency of cc_test without coverage (#17004)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
fmeum authored Dec 13, 2022
1 parent 7e80d02 commit 3a13af4
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 4 deletions.
2 changes: 1 addition & 1 deletion src/main/starlark/builtins_bzl/common/cc/cc_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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),
},
Expand Down
6 changes: 3 additions & 3 deletions src/main/starlark/builtins_bzl/common/cc/semantics.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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",
),
}

Expand Down
42 changes: 42 additions & 0 deletions src/test/shell/bazel/cc_integration_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"

0 comments on commit 3a13af4

Please sign in to comment.