From 7f56d40e610a0c5ed71111d7a3f3d4182d18c3f7 Mon Sep 17 00:00:00 2001 From: Jack Marsh Date: Mon, 24 Aug 2026 14:52:58 +0100 Subject: [PATCH 1/2] Rebase a generated path handed over through an env var A build script can give the compiler a path through a custom env var rather than through OUT_DIR, and it records that path where it ran. The build script's directory is gone by the time the crate compiles, so the crate does not build at all: error: couldn't read .../_mime_guess_build_script._build/mime_guess_out/mime_types_generated.rs --> src/impl_bin_search.rs:4:1 | 4 | include!(env!("MIME_TYPES_GENERATED_PATH")); mime_guess is the crate that found it, and it sits under a good deal of the web ecosystem. Nothing in this repo declared it, so nothing here failed. The machinery already existed: rebase_build_path rewrites a recorded build-script path to where the output actually landed, and was applied to link search paths and not to env values. It only rewrites a value that starts with the recorded out dir, so an env var that is not a path passes through untouched. test/buildscript_env is a crate doing the same thing deliberately: its build script writes into OUT_DIR, hands the path over as an env var, and its lib includes it. Red before this with the same error. --- BUILD | 1 + test/buildscript_env/BUILD | 37 ++++++++++++++ test/buildscript_env/buildscript_env_test.rs | 12 +++++ test/buildscript_env/genpath/Cargo.toml | 5 ++ test/buildscript_env/genpath/build.rs | 15 ++++++ test/buildscript_env/genpath/src/lib.rs | 7 +++ tools/please_rust/src/compile.rs | 52 +++++++++++++++++++- 7 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 test/buildscript_env/BUILD create mode 100644 test/buildscript_env/buildscript_env_test.rs create mode 100644 test/buildscript_env/genpath/Cargo.toml create mode 100644 test/buildscript_env/genpath/build.rs create mode 100644 test/buildscript_env/genpath/src/lib.rs diff --git a/BUILD b/BUILD index 1ac7393..03d72b7 100644 --- a/BUILD +++ b/BUILD @@ -24,6 +24,7 @@ rust_project( # of those resolves in the editor only if its lock is here too. lock = [ "//third_party/crates:rust_lock", + "//test/buildscript_env:buildscript_env_lock", "//test/firstparty:firstparty_lock", "//test/links:links_lock", "//test/patch:patch_lock", diff --git a/test/buildscript_env/BUILD b/test/buildscript_env/BUILD new file mode 100644 index 0000000..e469f4a --- /dev/null +++ b/test/buildscript_env/BUILD @@ -0,0 +1,37 @@ +subinclude("//build_defs:rust", "//build_defs:rust_repo") + +# A build script can hand a path to the compiler through a custom env var +# rather than through OUT_DIR, and it records that path where it ran. That +# directory is gone by the time the crate compiles, so the crate does not +# build at all: mime_guess, which does +# include!(env!("MIME_TYPES_GENERATED_PATH")), could not be built here. + +genrule( + name = "genpath_src", + srcs = glob(["genpath/**"]), + outs = ["genpath-0.1.0"], + cmd = "mv $PKG_DIR/genpath $OUT", +) + +rust_repo( + name = "genpath", + crate = "genpath", + download = ":genpath_src", + lock = ":buildscript_env_lock", + third_party_path = "test/buildscript_env", + version = "0.1.0", +) + +rust_resolve( + name = "buildscript_env_lock", + entries = [ + "genpath|genpath|0.1.0||true|true", + ], +) + +rust_test( + name = "buildscript_env_test", + root = "buildscript_env_test.rs", + edition = "2021", + deps = [":genpath"], +) diff --git a/test/buildscript_env/buildscript_env_test.rs b/test/buildscript_env/buildscript_env_test.rs new file mode 100644 index 0000000..04779ac --- /dev/null +++ b/test/buildscript_env/buildscript_env_test.rs @@ -0,0 +1,12 @@ +extern crate genpath; + +#[test] +fn a_generated_path_passed_through_an_env_var_resolves() { + // Reaching this at all means the include! found the generated file. + assert_eq!(genpath::ANSWER, 42); +} + +#[test] +fn an_env_var_that_is_not_a_path_is_untouched() { + assert_eq!(genpath::plain(), "not-a-path"); +} diff --git a/test/buildscript_env/genpath/Cargo.toml b/test/buildscript_env/genpath/Cargo.toml new file mode 100644 index 0000000..1933141 --- /dev/null +++ b/test/buildscript_env/genpath/Cargo.toml @@ -0,0 +1,5 @@ +[package] +name = "genpath" +version = "0.1.0" +edition = "2021" +build = "build.rs" diff --git a/test/buildscript_env/genpath/build.rs b/test/buildscript_env/genpath/build.rs new file mode 100644 index 0000000..aba608b --- /dev/null +++ b/test/buildscript_env/genpath/build.rs @@ -0,0 +1,15 @@ +// Writes a source file into OUT_DIR and hands its path to the compiler +// through a custom env var rather than through OUT_DIR itself. This is what +// mime_guess does, and the path recorded here points inside the build +// script's own sandbox, which no longer exists when the crate is compiled. +use std::io::Write; + +fn main() { + let out = std::env::var("OUT_DIR").expect("OUT_DIR is set for a build script"); + let path = std::path::Path::new(&out).join("generated_answer.rs"); + let mut f = std::fs::File::create(&path).expect("write into OUT_DIR"); + writeln!(f, "pub const ANSWER: u32 = 42;").unwrap(); + println!("cargo:rustc-env=GENERATED_ANSWER_PATH={}", path.display()); + // A value that is not a path must survive untouched. + println!("cargo:rustc-env=PLAIN_VALUE=not-a-path"); +} diff --git a/test/buildscript_env/genpath/src/lib.rs b/test/buildscript_env/genpath/src/lib.rs new file mode 100644 index 0000000..aa09b76 --- /dev/null +++ b/test/buildscript_env/genpath/src/lib.rs @@ -0,0 +1,7 @@ +// The include! is the point: it resolves the env var at compile time, so a +// stale path fails the build rather than producing a wrong answer. +include!(env!("GENERATED_ANSWER_PATH")); + +pub fn plain() -> &'static str { + env!("PLAIN_VALUE") +} diff --git a/tools/please_rust/src/compile.rs b/tools/please_rust/src/compile.rs index d143435..c15aa42 100644 --- a/tools/please_rust/src/compile.rs +++ b/tools/please_rust/src/compile.rs @@ -610,6 +610,12 @@ fn build_command(args: &CompileArgs) -> Result { // Link libraries from build script if let Some(ref directives) = buildscript_directives { + // The same resolution as the search paths above: where the out dir + // ended up, rather than where the build script left it. + let resolved_out = directives + .out_dir + .as_ref() + .and_then(|d| resolve_out_dir(d, args.buildscript.as_deref())); for lib in &directives.rustc_link_libs { // Handle KIND=NAME format (e.g., "static=foo", "dylib=bar") if let Some((kind, name)) = lib.split_once('=') { @@ -624,9 +630,23 @@ fn build_command(args: &CompileArgs) -> Result { cmd.arg("-C").arg(format!("link-arg={}", arg)); } - // Set environment variables from build script + // Set environment variables from build script. + // + // A value can be a path into the out dir, and that path was recorded + // where the build script ran, which is gone by now. mime_guess sets + // MIME_TYPES_GENERATED_PATH and then does + // include!(env!("MIME_TYPES_GENERATED_PATH")), so without rebasing it + // the crate does not compile at all. Anything that is not a path + // under the out dir is passed through untouched. for (key, value) in &directives.rustc_envs { - cmd.env(key, value); + cmd.env( + key, + rebase_build_path( + value, + directives.built_out_dir.as_deref(), + resolved_out.as_deref(), + ), + ); } } @@ -728,6 +748,34 @@ mod tests { assert_eq!(rebase_build_path("/some/dir", None, Some(now)), "/some/dir"); } + /// A build script can hand a path to the compiler through a custom env + /// var rather than through OUT_DIR, and that path was recorded where the + /// build script ran. mime_guess does + /// include!(env!("MIME_TYPES_GENERATED_PATH")) and could not compile at + /// all: the recorded path pointed into the build script's sandbox, which + /// is gone by then. + #[test] + fn an_env_var_holding_a_generated_path_is_rebased() { + let built = Path::new("/plz-out/tmp/x/_mime_guess_build_script._build/mime_guess_out"); + let now = Path::new("/plz-out/gen/x/mime_guess_out"); + + assert_eq!( + rebase_build_path( + "/plz-out/tmp/x/_mime_guess_build_script._build/mime_guess_out/mime_types_generated.rs", + Some(built), + Some(now) + ), + "/plz-out/gen/x/mime_guess_out/mime_types_generated.rs" + ); + + // An env var that is not a path is not a path, and passes through. + assert_eq!( + rebase_build_path("something=else", Some(built), Some(now)), + "something=else" + ); + assert_eq!(rebase_build_path("1", Some(built), Some(now)), "1"); + } + #[test] fn out_dir_resolves_relative_to_buildscript() { let dir = From 031d737641c32fc051d576124fbd3a5f1c55348d Mon Sep 17 00:00:00 2001 From: Jack Marsh Date: Mon, 24 Aug 2026 15:04:02 +0100 Subject: [PATCH 2/2] Skip a test the pinned tool cannot pass test/buildscript_env proves a fix in the tool, so a release made before that fix fails it. That is what the test is for, and not a regression in the released tool. The pinned-tool job asks whether a released tool still works with these rules. A change it predates is outside that question, so the run skips tests labelled requires_unreleased_tool, on both linux and macos. rust_test grows a labels argument to make that sayable. It had none, so a test could not be selected or skipped at all, which is what labels are for everywhere else in plz. --- .github/workflows/rust.yaml | 8 ++++++++ build_defs/rust.build_defs | 8 ++++++-- test/buildscript_env/BUILD | 4 ++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/.github/workflows/rust.yaml b/.github/workflows/rust.yaml index ab24b92..013066d 100644 --- a/.github/workflows/rust.yaml +++ b/.github/workflows/rust.yaml @@ -159,8 +159,13 @@ jobs: # disambiguated declaration - which is where a stale tool shows up. - name: Test against the pinned release tool run: | + # A test proving a fix in the tool cannot pass against a release + # made before that fix. Skipping it is the honest reading of what + # this job asks: whether the released tool still works with these + # rules, not whether it has changes it predates. ./pleasew test -p -v notice \ -o plugin.rust.pleaserusttool://tools/please_rust:please_rust_release \ + --exclude requires_unreleased_tool \ //test/... //examples/... \ --log_file plz-out/log/pinned.log - name: Archive logs @@ -206,8 +211,11 @@ jobs: # pins, so its hash was taken on trust. - name: Test against the pinned release tool run: | + # See the note on the linux job: a test proving a fix in the tool + # cannot pass against a release made before that fix. ./pleasew test -p -v notice \ -o plugin.rust.pleaserusttool://tools/please_rust:please_rust_release \ + --exclude requires_unreleased_tool \ //test/... //examples/... \ --log_file plz-out/log/pinned.log - name: Archive logs diff --git a/build_defs/rust.build_defs b/build_defs/rust.build_defs index 066e759..4b7f367 100644 --- a/build_defs/rust.build_defs +++ b/build_defs/rust.build_defs @@ -908,7 +908,7 @@ def rust_binary(name:str, main:str, modules:list=None, deps:list=[], cc_deps:lis return bin_rule -def rust_test(name:str, root:str, modules:list=None, deps:list=[], cc_deps:list=[], edition:str="2018", features:list=None, manifest:str=None, visibility:list=None): +def rust_test(name:str, root:str, modules:list=None, deps:list=[], cc_deps:list=[], edition:str="2018", features:list=None, manifest:str=None, labels:list=[], visibility:list=None): """Defines a test rule for a Rust library. Note that the Rust test runner has no ability to output in a format plz @@ -922,6 +922,10 @@ def rust_test(name:str, root:str, modules:list=None, deps:list=[], cc_deps:list= deps (list): Dependencies of the test harness. edition (str): Rust edition to compile with (2015, 2018, 2021, 2024). features (list): Feature cfgs to enable. + labels (list): Extra labels, on top of the `rust` one every rule + carries. Labels are how a run selects or skips tests, + so a test that only passes under some condition can say + so and be excluded with `plz test --exclude