Fix coverage for crates with mixed sources (#4106)
An expansion of https://github.com/bazelbuild/rules_rust/pull/4079 with
some added testing.
closes https://github.com/bazelbuild/rules_rust/pull/4079
diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml
index 7b5b0bc..0902d01 100644
--- a/.bazelci/presubmit.yml
+++ b/.bazelci/presubmit.yml
@@ -71,6 +71,18 @@
; 1>&2 head -50 bazel-out/_coverage/_coverage_report.dat \
; exit 1 \
; }
+ # Regression check: mixed in-tree/generated-source crates must produce
+ # real coverage, not just the zero-LF baseline record.
+ - |
+ awk '/^SF:test\/generated_inputs\/lib\.rs$/,/^end_of_record$/' \
+ bazel-out/_coverage/_coverage_report.dat \
+ | grep -qE '^LF:[1-9]' \
+ || { 1>&2 echo "No coverage for mixed-source crate (test/generated_inputs/lib.rs)" \
+ ; 1>&2 echo "Coverage was silently dropped for crates with generated sources." \
+ ; 1>&2 awk '/^SF:test\/generated_inputs\/lib\.rs$/,/^end_of_record$/' \
+ bazel-out/_coverage/_coverage_report.dat \
+ ; exit 1 \
+ ; }
split_coverage_postprocessing_shell_commands: &split_coverage_postprocessing_shell_commands
- echo "coverage --experimental_fetch_all_coverage_outputs" >> user.bazelrc
- echo "coverage --experimental_split_coverage_postprocessing" >> user.bazelrc
diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl
index c278b5f..69e0d05 100644
--- a/rust/private/rustc.bzl
+++ b/rust/private/rustc.bzl
@@ -897,6 +897,14 @@
return True
return False
+def _args_map_bin_dir(file):
+ """Extract `bazel-out/<config>/bin` from a File whose path lives in the configuration's bin directory.
+
+ Evaluated at action-execution time so that Bazel's path mapping (`--experimental_output_paths=strip`) can
+ rewrite the `<config>` segment to `cfg` before we slice it off.
+ """
+ return "/".join(file.path.split("/", 3)[:3])
+
def construct_arguments(
*,
ctx,
@@ -1326,6 +1334,25 @@
# https://doc.rust-lang.org/rustc/instrument-coverage.html
rustc_flags.add("--codegen=instrument-coverage")
+ # Crates with generated sources are compiled from the output tree
+ # (see `transform_sources`), so the coverage mapping records their
+ # files with a `bazel-out/<config>/bin/` prefix. Bazel's lcov
+ # merger silently drops all coverage for such crates, so we remap
+ # the prefix away. The prefix is derived from the crate's own
+ # output File (rather than `ctx.bin_dir`) so that Bazel's path
+ # mapping (`--experimental_output_paths=strip`) can rewrite the
+ # `<config>` segment to `cfg` before the value reaches rustc —
+ # `ctx.bin_dir` is a `root`, not a `File`, and is not subject to
+ # path-mapping rewriting. Skipped for rustdoc (which passes
+ # `remap_path_prefix=None`), since rustdoc only supports
+ # `--remap-path-prefix` behind `-Zunstable-options`.
+ if remap_path_prefix != None:
+ rustc_flags.add_all(
+ [crate_info.output],
+ format_each = "--remap-path-prefix=%s/=",
+ map_each = _args_map_bin_dir,
+ )
+
if toolchain._experimental_link_std_dylib:
rustc_flags.add("--codegen=prefer-dynamic")
diff --git a/test/unit/common.bzl b/test/unit/common.bzl
index cedf39e..26509c3 100644
--- a/test/unit/common.bzl
+++ b/test/unit/common.bzl
@@ -149,3 +149,25 @@
real_value = action.env[key],
),
)
+
+def get_bin_dir_from_action(action):
+ """Extract `bazel-out/<config>/bin` from an action.
+
+ Inspects argv first so the path-mapping (`--experimental_output_paths=strip`)
+ form `bazel-out/cfg/bin` is returned when active, otherwise falls back to
+ the first output's `.dirname` (handles `-ST-<hash>` config-transition
+ suffixes).
+
+ Args:
+ action: The action to extract the bin directory from.
+
+ Returns:
+ The bin directory path as a string.
+ """
+ for arg in action.argv:
+ if arg.startswith("bazel-out/cfg/bin/"):
+ return "bazel-out/cfg/bin"
+ bin_dir = action.outputs.to_list()[0].dirname
+ if "/bin/" in bin_dir:
+ bin_dir = bin_dir.split("/bin/")[0] + "/bin"
+ return bin_dir
diff --git a/test/unit/native_deps/native_deps_test.bzl b/test/unit/native_deps/native_deps_test.bzl
index dd9858c..2042320 100644
--- a/test/unit/native_deps/native_deps_test.bzl
+++ b/test/unit/native_deps/native_deps_test.bzl
@@ -11,35 +11,12 @@
"assert_argv_contains_prefix_not",
"assert_argv_contains_prefix_suffix",
"assert_list_contains_adjacent_elements",
+ "get_bin_dir_from_action",
)
def _get_toolchain(ctx):
return ctx.attr._toolchain[platform_common.ToolchainInfo]
-def _get_bin_dir_from_action(action):
- """Extract the bin directory from an action's outputs.
-
- This handles config transitions that add suffixes like -ST-<hash>, as
- well as Bazel path mapping (`--experimental_output_paths=strip` plus a
- `supports-path-mapping` requirement on the action), which rewrites
- argv entries to live under `bazel-out/cfg/bin/...` even though the
- File's `.dirname` still returns the un-mapped, configuration-specific
- path.
-
- Args:
- action: The action to extract the bin directory from.
-
- Returns:
- The bin directory path as a string.
- """
- for arg in action.argv:
- if arg.startswith("bazel-out/cfg/bin/"):
- return "bazel-out/cfg/bin"
- bin_dir = action.outputs.to_list()[0].dirname
- if "/bin/" in bin_dir:
- bin_dir = bin_dir.split("/bin/")[0] + "/bin"
- return bin_dir
-
def _get_darwin_component(arg):
"""Extract darwin component from a path.
@@ -213,7 +190,7 @@
toolchain = _get_toolchain(ctx)
link_args = _extract_linker_args(action.argv)
- bin_dir = _get_bin_dir_from_action(action)
+ bin_dir = get_bin_dir_from_action(action)
# Validate bin_dir structure (ignoring ST-{hash} suffix from config transitions)
_assert_bin_dir_structure(env, ctx, bin_dir, toolchain)
@@ -296,7 +273,7 @@
action = tut.actions[0]
linker_args = _extract_linker_args(action.argv)
- bin_dir = _get_bin_dir_from_action(action)
+ bin_dir = get_bin_dir_from_action(action)
# Validate bin_dir structure (ignoring ST-{hash} suffix from config transitions)
_assert_bin_dir_structure(env, ctx, bin_dir, toolchain)
diff --git a/test/unit/remap_path_prefix/remap_path_prefix_test.bzl b/test/unit/remap_path_prefix/remap_path_prefix_test.bzl
index a5e4d67..0dbdce9 100644
--- a/test/unit/remap_path_prefix/remap_path_prefix_test.bzl
+++ b/test/unit/remap_path_prefix/remap_path_prefix_test.bzl
@@ -6,7 +6,10 @@
load(
"//test/unit:common.bzl",
"assert_action_mnemonic",
+ "assert_argv_contains",
+ "assert_argv_contains_not",
"assert_list_contains_adjacent_elements",
+ "get_bin_dir_from_action",
)
def _remap_path_prefix_test_impl(ctx):
@@ -42,6 +45,68 @@
_subst_flags_test = analysistest.make(_subst_flags_test_impl)
+def _coverage_remap_path_prefix_test_impl(ctx):
+ """Verify a single `--remap-path-prefix` flag covers the bin directory.
+
+ The flag is derived from a `File` via `map_each` so Bazel's path
+ mapping (`--experimental_output_paths=strip`) rewrites the
+ `<config>` segment to `cfg` before it reaches rustc — one flag
+ works for both the path-mapped and un-mapped forms.
+ """
+ env = analysistest.begin(ctx)
+ target = analysistest.target_under_test(env)
+
+ action = target.actions[0]
+ assert_action_mnemonic(env, action, "Rustc")
+
+ # If the host toolchain does not support coverage (no `llvm_cov` or
+ # missing `profiler_builtins`) the instrument-coverage flag will be
+ # absent and the remap flag is not expected either.
+ if "--codegen=instrument-coverage" not in action.argv:
+ return analysistest.end(env)
+
+ bin_dir = get_bin_dir_from_action(action)
+ assert_argv_contains(env, action, "--remap-path-prefix={}/=".format(bin_dir))
+
+ return analysistest.end(env)
+
+_coverage_remap_path_prefix_test = analysistest.make(
+ _coverage_remap_path_prefix_test_impl,
+ config_settings = {
+ "//command_line_option:collect_code_coverage": True,
+ },
+)
+
+_coverage_remap_path_prefix_path_mapping_test = analysistest.make(
+ _coverage_remap_path_prefix_test_impl,
+ config_settings = {
+ "//command_line_option:collect_code_coverage": True,
+ "//command_line_option:experimental_output_paths": "strip",
+ },
+)
+
+def _no_coverage_remap_path_prefix_test_impl(ctx):
+ """Verify the coverage-specific remap flag is absent without coverage."""
+ env = analysistest.begin(ctx)
+ target = analysistest.target_under_test(env)
+
+ action = target.actions[0]
+ assert_action_mnemonic(env, action, "Rustc")
+
+ assert_argv_contains_not(env, action, "--codegen=instrument-coverage")
+
+ bin_dir = get_bin_dir_from_action(action)
+ assert_argv_contains_not(env, action, "--remap-path-prefix={}/=".format(bin_dir))
+
+ return analysistest.end(env)
+
+_no_coverage_remap_path_prefix_test = analysistest.make(
+ _no_coverage_remap_path_prefix_test_impl,
+ config_settings = {
+ "//command_line_option:collect_code_coverage": False,
+ },
+)
+
def remap_path_prefix_test_suite(name):
"""Entry-point macro called from the BUILD file.
@@ -78,6 +143,38 @@
edition = "2021",
)
+ # A library whose source set contains a generated file — this is the
+ # case `transform_sources` is designed for, and the one that
+ # previously lost all coverage data because the records pointed at
+ # `bazel-out/.../bin/...`.
+ write_file(
+ name = "mixed_inline_src",
+ out = "mixed_inline.rs",
+ content = [
+ "pub fn inline() {}",
+ "",
+ ],
+ )
+
+ write_file(
+ name = "mixed_generated_src",
+ out = "mixed_generated.rs",
+ content = [
+ "pub fn generated() {}",
+ "",
+ ],
+ )
+
+ rust_library(
+ name = "remap_mixed_lib",
+ srcs = [
+ ":mixed_inline.rs",
+ ":mixed_generated_src",
+ ],
+ crate_root = ":mixed_inline.rs",
+ edition = "2021",
+ )
+
_remap_path_prefix_test(
name = "remap_path_prefix_lib_test",
target_under_test = ":remap_lib",
@@ -98,11 +195,29 @@
target_under_test = ":remap_bin",
)
+ _coverage_remap_path_prefix_test(
+ name = "coverage_remap_path_prefix_mixed_lib_test",
+ target_under_test = ":remap_mixed_lib",
+ )
+
+ _coverage_remap_path_prefix_path_mapping_test(
+ name = "coverage_remap_path_prefix_path_mapping_mixed_lib_test",
+ target_under_test = ":remap_mixed_lib",
+ )
+
+ _no_coverage_remap_path_prefix_test(
+ name = "no_coverage_remap_path_prefix_lib_test",
+ target_under_test = ":remap_lib",
+ )
+
tests = [
":remap_path_prefix_lib_test",
":remap_path_prefix_bin_test",
":subst_flags_lib_test",
":subst_flags_bin_test",
+ ":coverage_remap_path_prefix_mixed_lib_test",
+ ":coverage_remap_path_prefix_path_mapping_mixed_lib_test",
+ ":no_coverage_remap_path_prefix_lib_test",
]
native.test_suite(
