Add path-mapping test for env location expansion (#4120)
Adds regression testing for
https://github.com/bazelbuild/rules_rust/pull/4117 as well as identifies
some cases where there are further incompatibilities (namely uses of
`rust_test.crate`).
diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl
index 112a7b3..44d2576 100644
--- a/rust/private/rustc.bzl
+++ b/rust/private/rustc.bzl
@@ -890,10 +890,19 @@
other_flags.append(flag)
return other_flags
-def _has_location_expansion(flags):
- for flag in flags:
+def has_location_expansion(values):
+ """Return True if any string in `values` contains a Bazel location-expansion directive.
+
+ Args:
+ values: Iterable of strings (e.g. `rustc_flags`, or `rustc_env.values()`).
+
+ Returns:
+ bool: True if any value contains `$(location ...)`, `$(locations ...)`,
+ `$(execpath ...)`, or `$(execpaths ...)`.
+ """
+ for value in values:
for directive in ("$(location ", "$(locations ", "$(execpath ", "$(execpaths "):
- if directive in flag:
+ if directive in value:
return True
return False
@@ -1446,17 +1455,27 @@
all_args = [process_wrapper_flags, rustc_path, rustc_flags]
if rust_flags_args != None:
all_args.append(rust_flags_args)
- has_location_expansion = _has_location_expansion(authored_rustc_flags)
- rustc_env_attr = getattr(crate_info, "rustc_env", None)
- if not has_location_expansion and rustc_env_attr:
- has_location_expansion = _has_location_expansion(rustc_env_attr.values())
+
+ # Path mapping must be disabled whenever any input string carries a
+ # `$(location ...)` / `$(execpath ...)` macro: location expansion
+ # runs outside path mapping and would produce configuration-specific
+ # paths inside env values that the sandbox layout no longer matches.
+ # Check both `authored_rustc_flags` and `attr.rustc_env` (raw). We
+ # cannot rely on `crate_info.rustc_env`: `rust_test` pre-expands its
+ # own `rustc_env` inside the rule impl (see `rust.bzl`), so the
+ # markers are gone by the time they reach `crate_info`.
+ target_has_location_expansion = has_location_expansion(authored_rustc_flags)
+ if not target_has_location_expansion:
+ rustc_env_attr = getattr(attr, "rustc_env", None)
+ if rustc_env_attr:
+ target_has_location_expansion = has_location_expansion(rustc_env_attr.values())
args = struct(
process_wrapper_flags = process_wrapper_flags,
rustc_path = rustc_path,
rustc_flags = rustc_flags,
extra_rustc_flags = rust_flags_args,
- supports_path_mapping = not has_location_expansion,
+ supports_path_mapping = not target_has_location_expansion,
all = all_args,
)
diff --git a/test/unit/location_expansion/location_expansion_test.bzl b/test/unit/location_expansion/location_expansion_test.bzl
index 3c63355..7ca5d0b 100644
--- a/test/unit/location_expansion/location_expansion_test.bzl
+++ b/test/unit/location_expansion/location_expansion_test.bzl
@@ -3,7 +3,7 @@
load("@bazel_skylib//lib:unittest.bzl", "analysistest")
load("@bazel_skylib//rules:write_file.bzl", "write_file")
load("//cargo:defs.bzl", "cargo_build_script")
-load("//rust:defs.bzl", "rust_library")
+load("//rust:defs.bzl", "rust_library", "rust_test")
load("//test/unit:common.bzl", "assert_action_mnemonic", "assert_argv_contains", "assert_env_value")
def _find_action(tut, mnemonic):
@@ -35,6 +35,27 @@
location_expansion_rustc_flags_test = analysistest.make(_location_expansion_rustc_flags_test)
+def _location_expansion_rustc_env_test(ctx):
+ env = analysistest.begin(ctx)
+ tut = analysistest.target_under_test(env)
+ action = _find_action(tut, "Rustc")
+ if not action:
+ fail("No Rustc action found")
+
+ # Sanity-check: `$(execpaths ...)` in `rustc_env` is expanded at
+ # analysis time and surfaces as a configuration-specific path in
+ # the env value. The build-time guard against path-mapping
+ # mismatches lives in `mylibrary_env` itself: it pulls the env
+ # path in via `include_bytes!(env!("MY_DATA"))`, so any path under
+ # `--experimental_output_paths=strip` that doesn't match the
+ # sandbox layout fails the build — see the comment in
+ # `mylibrary_env.rs`.
+ expected = "${pwd}/" + ctx.bin_dir.path + "/test/unit/location_expansion/flag_execpaths.data"
+ assert_env_value(env, action, "MY_DATA", expected)
+ return analysistest.end(env)
+
+location_expansion_rustc_env_test = analysistest.make(_location_expansion_rustc_env_test)
+
def _location_expansion_build_script_env_test(ctx):
env = analysistest.begin(ctx)
tut = analysistest.target_under_test(env)
@@ -75,6 +96,36 @@
],
)
+ # A library whose location expansion lives ONLY in `rustc_env` (not
+ # in `rustc_flags`). Exercises the path-mapping interaction with
+ # rustc_env specifically.
+ rust_library(
+ name = "mylibrary_env",
+ srcs = ["mylibrary_env.rs"],
+ edition = "2018",
+ rustc_env = {
+ "MY_DATA": "$(execpaths :flag_generator_execpaths)",
+ },
+ compile_data = [
+ ":flag_generator_execpaths",
+ ],
+ )
+
+ # Same shape as `mylibrary_env` but built through `rust_test`, which
+ # takes a different `crate_info.rustc_env` code path — see
+ # `mytest_env.rs` for the details of why PR #4117 doesn't cover it.
+ rust_test(
+ name = "mytest_env",
+ srcs = ["mytest_env.rs"],
+ edition = "2018",
+ rustc_env = {
+ "MY_DATA": "$(execpaths :flag_generator_execpaths)",
+ },
+ compile_data = [
+ ":flag_generator_execpaths",
+ ],
+ )
+
cargo_build_script(
name = "mybuildscript",
srcs = ["build_script.rs"],
@@ -90,6 +141,11 @@
target_under_test = ":mylibrary",
)
+ location_expansion_rustc_env_test(
+ name = "location_expansion_rustc_env_test",
+ target_under_test = ":mylibrary_env",
+ )
+
location_expansion_build_script_env_test(
name = "location_expansion_build_script_env_test",
target_under_test = ":mybuildscript",
@@ -107,6 +163,7 @@
name = name,
tests = [
":location_expansion_rustc_flags_test",
+ ":location_expansion_rustc_env_test",
":location_expansion_build_script_env_test",
],
)
diff --git a/test/unit/location_expansion/mylibrary_env.rs b/test/unit/location_expansion/mylibrary_env.rs
new file mode 100644
index 0000000..7513641
--- /dev/null
+++ b/test/unit/location_expansion/mylibrary_env.rs
@@ -0,0 +1,16 @@
+// The byte content of `MY_DATA` is included at compile time. The
+// path is plumbed via a `$(execpaths ...)` location expansion in
+// `rustc_env`, so it points at a generated file under `bazel-out`.
+//
+// When Bazel path mapping (`--experimental_output_paths=strip`) is
+// active, the rustc action's sandbox sees files at
+// `bazel-out/cfg/bin/...` only. The expanded env value, however, is
+// a plain string set at analysis time and is NOT rewritten by path
+// mapping. So if the action wrongly advertises
+// `supports-path-mapping`, the env path stays at
+// `bazel-out/<config>/bin/...` while the file only exists at
+// `bazel-out/cfg/bin/...` inside the sandbox, and this
+// `include_bytes!` fails at compile time. PR #4117 disables path
+// mapping for actions whose `rustc_env` contains location
+// expansions; this `const` exercises that contract.
+pub const MY_DATA: &[u8] = include_bytes!(env!("MY_DATA"));
diff --git a/test/unit/location_expansion/mytest_env.rs b/test/unit/location_expansion/mytest_env.rs
new file mode 100644
index 0000000..8c7f508
--- /dev/null
+++ b/test/unit/location_expansion/mytest_env.rs
@@ -0,0 +1,18 @@
+// Same contract as `mylibrary_env.rs`, but exercised through
+// `rust_test` rather than `rust_library`. This surfaces a code path
+// PR #4117 missed: for `rust_test`, location expansion in `rustc_env`
+// is performed inside the rule implementation (see `rust.bzl`)
+// BEFORE the values reach `CrateInfo.rustc_env`. By the time
+// `construct_arguments` in `rustc.bzl` inspects
+// `crate_info.rustc_env.values()` looking for `$(location ...)`
+// markers, they've already been replaced with concrete paths — so
+// the check finds nothing and `supports-path-mapping` stays enabled.
+// Under `--experimental_output_paths=strip`, that leaves the env
+// value pointing at the un-mapped `bazel-out/<config>/bin/...` path
+// while the sandbox only has the file at `bazel-out/cfg/bin/...`.
+//
+// The bug is caught at compile time by `include_bytes!(env!(...))`:
+// rustc has to read the file at the env path inside the sandbox, so
+// a path-mapping mismatch fails the build outright. No runtime
+// assertion is needed — declaring the const is the whole test.
+const _MY_DATA: &[u8] = include_bytes!(env!("MY_DATA"));
