From 8abe469de696dea59a9d67ca59e4b7abc3babf31 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 19 Dec 2025 13:43:58 -0600 Subject: [PATCH 1/7] refactor(compile): Only generate the name if needed --- src/cargo/core/compiler/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index cffa9a69cf6..93b7edc0850 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -1461,7 +1461,7 @@ fn build_base_args( let name = bin_target .binary_filename() - .unwrap_or(bin_target.name().to_string()); + .unwrap_or_else(|| bin_target.name().to_string()); let key = format!("CARGO_BIN_EXE_{}", name); cmd.env(&key, exe_path); } From 6423bfe62e83692919d9cddbaddee16ce8ef0f72 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 19 Dec 2025 13:44:43 -0600 Subject: [PATCH 2/7] refactor(compile): Use the binary name for the placeholder --- src/cargo/core/compiler/mod.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 93b7edc0850..27adbc4e4c4 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -1450,6 +1450,10 @@ fn build_base_args( .iter() .filter(|target| target.is_bin()) { + let name = bin_target + .binary_filename() + .unwrap_or_else(|| bin_target.name().to_string()); + // For `cargo check` builds we do not uplift the CARGO_BIN_EXE_ artifacts to the // artifact-dir. We do not want to provide a path to a non-existent binary but we still // need to provide *something* so `env!("CARGO_BIN_EXE_...")` macros will compile. @@ -1457,12 +1461,9 @@ fn build_base_args( .files() .bin_link_for_target(bin_target, unit.kind, build_runner.bcx)? .map(|path| path.as_os_str().to_os_string()) - .unwrap_or_else(|| OsString::from(format!("placeholder:{}", bin_target.name()))); + .unwrap_or_else(|| OsString::from(format!("placeholder:{name}"))); - let name = bin_target - .binary_filename() - .unwrap_or_else(|| bin_target.name().to_string()); - let key = format!("CARGO_BIN_EXE_{}", name); + let key = format!("CARGO_BIN_EXE_{name}"); cmd.env(&key, exe_path); } } From 98c382fd3099449afe31cca94b015ef24802187f Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 19 Dec 2025 14:01:40 -0600 Subject: [PATCH 3/7] refactor(compile): Move CARGO_BIN_EXE_ next to artifact deps Seeing as this is an implicit artifact At this new location, it will now run as part of `cargo rustdoc --test ` which technically fixes a bug but `--test` is not really supported (#13427) so I didn't mark this as a `fix` nor did I add tests. --- src/cargo/core/compiler/mod.rs | 53 +++++++++++++++++----------------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 27adbc4e4c4..84153185d09 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -1441,32 +1441,6 @@ fn build_base_args( .env("RUSTC_BOOTSTRAP", "1"); } - // Add `CARGO_BIN_EXE_` environment variables for building tests. - if unit.target.is_test() || unit.target.is_bench() { - for bin_target in unit - .pkg - .manifest() - .targets() - .iter() - .filter(|target| target.is_bin()) - { - let name = bin_target - .binary_filename() - .unwrap_or_else(|| bin_target.name().to_string()); - - // For `cargo check` builds we do not uplift the CARGO_BIN_EXE_ artifacts to the - // artifact-dir. We do not want to provide a path to a non-existent binary but we still - // need to provide *something* so `env!("CARGO_BIN_EXE_...")` macros will compile. - let exe_path = build_runner - .files() - .bin_link_for_target(bin_target, unit.kind, build_runner.bcx)? - .map(|path| path.as_os_str().to_os_string()) - .unwrap_or_else(|| OsString::from(format!("placeholder:{name}"))); - - let key = format!("CARGO_BIN_EXE_{name}"); - cmd.env(&key, exe_path); - } - } Ok(()) } @@ -1770,6 +1744,33 @@ fn build_deps_args( cmd.arg(arg); } + // Add `CARGO_BIN_EXE_` environment variables for building tests. + if unit.target.is_test() || unit.target.is_bench() { + for bin_target in unit + .pkg + .manifest() + .targets() + .iter() + .filter(|target| target.is_bin()) + { + let name = bin_target + .binary_filename() + .unwrap_or_else(|| bin_target.name().to_string()); + + // For `cargo check` builds we do not uplift the CARGO_BIN_EXE_ artifacts to the + // artifact-dir. We do not want to provide a path to a non-existent binary but we still + // need to provide *something* so `env!("CARGO_BIN_EXE_...")` macros will compile. + let exe_path = build_runner + .files() + .bin_link_for_target(bin_target, unit.kind, build_runner.bcx)? + .map(|path| path.as_os_str().to_os_string()) + .unwrap_or_else(|| OsString::from(format!("placeholder:{name}"))); + + let key = format!("CARGO_BIN_EXE_{name}"); + cmd.env(&key, exe_path); + } + } + for (var, env) in artifact::get_env(build_runner, deps)? { cmd.env(&var, env); } From 1e56263a42ee3dd860ac22018f69bfe7303dc248 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 19 Dec 2025 14:39:25 -0600 Subject: [PATCH 4/7] refactor(compile): Move implicit artfacts into explicit artifacts This now runs in build scripts and doctests *but* the unit check should prevent any side effects from happening. --- src/cargo/core/compiler/artifact.rs | 31 +++++++++++++++++++++ src/cargo/core/compiler/build_runner/mod.rs | 2 +- src/cargo/core/compiler/custom_build.rs | 2 +- src/cargo/core/compiler/mod.rs | 29 +------------------ 4 files changed, 34 insertions(+), 30 deletions(-) diff --git a/src/cargo/core/compiler/artifact.rs b/src/cargo/core/compiler/artifact.rs index 42d979eed1b..d6b9fc105b3 100644 --- a/src/cargo/core/compiler/artifact.rs +++ b/src/cargo/core/compiler/artifact.rs @@ -12,9 +12,40 @@ use std::ffi::OsString; /// if artifacts are present. pub fn get_env( build_runner: &BuildRunner<'_, '_>, + unit: &Unit, dependencies: &[UnitDep], ) -> CargoResult> { let mut env = HashMap::new(); + + // Add `CARGO_BIN_EXE_` environment variables for building tests. + // + // These aren't built for `cargo check`, so can't use `dependencies` + if unit.target.is_test() || unit.target.is_bench() { + for bin_target in unit + .pkg + .manifest() + .targets() + .iter() + .filter(|target| target.is_bin()) + { + let name = bin_target + .binary_filename() + .unwrap_or_else(|| bin_target.name().to_string()); + + // For `cargo check` builds we do not uplift the CARGO_BIN_EXE_ artifacts to the + // artifact-dir. We do not want to provide a path to a non-existent binary but we still + // need to provide *something* so `env!("CARGO_BIN_EXE_...")` macros will compile. + let exe_path = build_runner + .files() + .bin_link_for_target(bin_target, unit.kind, build_runner.bcx)? + .map(|path| path.as_os_str().to_os_string()) + .unwrap_or_else(|| OsString::from(format!("placeholder:{name}"))); + + let key = format!("CARGO_BIN_EXE_{name}"); + env.insert(key, exe_path); + } + } + for unit_dep in dependencies.iter().filter(|d| d.unit.artifact.is_true()) { for artifact_path in build_runner .outputs(&unit_dep.unit)? diff --git a/src/cargo/core/compiler/build_runner/mod.rs b/src/cargo/core/compiler/build_runner/mod.rs index 02f3c62267a..d55208d667e 100644 --- a/src/cargo/core/compiler/build_runner/mod.rs +++ b/src/cargo/core/compiler/build_runner/mod.rs @@ -279,7 +279,7 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> { unstable_opts, linker: self.compilation.target_linker(unit.kind).clone(), script_metas, - env: artifact::get_env(&self, self.unit_deps(unit))?, + env: artifact::get_env(&self, unit, self.unit_deps(unit))?, }); } diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index db9bc9fce07..70a07c8c512 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -377,7 +377,7 @@ fn build_work(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResul .inherit_jobserver(&build_runner.jobserver); // Find all artifact dependencies and make their file and containing directory discoverable using environment variables. - for (var, value) in artifact::get_env(build_runner, dependencies)? { + for (var, value) in artifact::get_env(build_runner, unit, dependencies)? { cmd.env(&var, value); } diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 84153185d09..6aa74cfba84 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -1744,34 +1744,7 @@ fn build_deps_args( cmd.arg(arg); } - // Add `CARGO_BIN_EXE_` environment variables for building tests. - if unit.target.is_test() || unit.target.is_bench() { - for bin_target in unit - .pkg - .manifest() - .targets() - .iter() - .filter(|target| target.is_bin()) - { - let name = bin_target - .binary_filename() - .unwrap_or_else(|| bin_target.name().to_string()); - - // For `cargo check` builds we do not uplift the CARGO_BIN_EXE_ artifacts to the - // artifact-dir. We do not want to provide a path to a non-existent binary but we still - // need to provide *something* so `env!("CARGO_BIN_EXE_...")` macros will compile. - let exe_path = build_runner - .files() - .bin_link_for_target(bin_target, unit.kind, build_runner.bcx)? - .map(|path| path.as_os_str().to_os_string()) - .unwrap_or_else(|| OsString::from(format!("placeholder:{name}"))); - - let key = format!("CARGO_BIN_EXE_{name}"); - cmd.env(&key, exe_path); - } - } - - for (var, env) in artifact::get_env(build_runner, deps)? { + for (var, env) in artifact::get_env(build_runner, unit, deps)? { cmd.env(&var, env); } From 4fb51f3b557f0ee1156242a6003d7115ddf113b6 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 19 Dec 2025 14:45:31 -0600 Subject: [PATCH 5/7] refactor(compile): Relax traits for UnitOutput --- src/cargo/core/compiler/compilation.rs | 1 - src/cargo/ops/cargo_test.rs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index dff2d00ba80..9c4febabd91 100644 --- a/src/cargo/core/compiler/compilation.rs +++ b/src/cargo/core/compiler/compilation.rs @@ -53,7 +53,6 @@ pub struct Doctest { } /// Information about the output of a unit. -#[derive(Ord, PartialOrd, Eq, PartialEq)] pub struct UnitOutput { /// The unit that generated this output. pub unit: Unit, diff --git a/src/cargo/ops/cargo_test.rs b/src/cargo/ops/cargo_test.rs index 0e3e309c3b6..7ddf662464b 100644 --- a/src/cargo/ops/cargo_test.rs +++ b/src/cargo/ops/cargo_test.rs @@ -103,7 +103,7 @@ pub fn run_benches(ws: &Workspace<'_>, options: &TestOptions, args: &[&str]) -> fn compile_tests<'a>(ws: &Workspace<'a>, options: &TestOptions) -> CargoResult> { let mut compilation = ops::compile(ws, &options.compile_opts)?; - compilation.tests.sort(); + compilation.tests.sort_by_key(|u| u.unit.clone()); Ok(compilation) } From 89b59e5b8804a321ff1e006eb5e5cf25f974f395 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 19 Dec 2025 14:54:10 -0600 Subject: [PATCH 6/7] test(test): Show runtime behavior of CARGO_BIN_EXE --- tests/testsuite/test.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/testsuite/test.rs b/tests/testsuite/test.rs index dacff7631b7..66db48ce8f3 100644 --- a/tests/testsuite/test.rs +++ b/tests/testsuite/test.rs @@ -5216,6 +5216,9 @@ fn bin_env_for_test() { assert_eq!(env!("CARGO_BIN_EXE_foo"), ""); assert_eq!(env!("CARGO_BIN_EXE_with-dash"), ""); assert_eq!(env!("CARGO_BIN_EXE_grüßen"), ""); + assert_eq!(std::env::var("CARGO_BIN_EXE_foo").ok(), None); + assert_eq!(std::env::var("CARGO_BIN_EXE_with-dash").ok(), None); + assert_eq!(std::env::var("CARGO_BIN_EXE_grüßen").ok(), None); } "# .replace("", &bin_path("foo")) From 94156a965bb243120b6ce2226706898d2ee82887 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 19 Dec 2025 15:01:56 -0600 Subject: [PATCH 7/7] feat(test): Make CARGO_BIN_EXE_ available at runtime This also makes artifact deps available for consistency purposes. I originally proposed this for when moving Cargo off of Cargo's own internals but went with a different solution. Re-visiting this because `assert_cmd` has 2300+ dependents and it seems like making `assert_cmd::cargo_bin` work would be a better use of time than migrating all of those users to `assert_cmd::cargo_bin!`. For example, within the top 130 dependents (100,000 downloads or more), I found 66 that used `assert_cmd::cargo_bin` (rust-lang/rust#149852). > The reason to make `CARGO_BIN_EXE` set only at build-time was to > address the concern of manually running the test executable. > It's not > too common for tests to look at any runtime environment variables > (that I know of), so it usually just works (like running > `gdb target/debug/.../test-xxx`). > The concern was that if it were set at > runtime it would move more down the path where directly running the test > executable doesn't "just work". See https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/cargo_bin_exe.20and.20tests/near/513638220 However, - This is user opt-in - Users can run with `-vv` to see the env variables that are set - Users can run `CARGO_TARGET_..._RUNNER=gdb` - We can notify the `cargo nextest`, the main third-party test runner, about this. It should be easy to support as they have their own version of this, see https://nexte.st/docs/configuration/env-vars/?h=nextest_bin_exe#environment-variables-nextest-sets --- src/cargo/core/compiler/build_runner/mod.rs | 14 ++++++++------ src/cargo/core/compiler/compilation.rs | 3 +++ src/cargo/ops/cargo_run.rs | 1 + src/cargo/ops/cargo_test.rs | 9 +++++++++ tests/testsuite/test.rs | 6 +++--- 5 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/cargo/core/compiler/build_runner/mod.rs b/src/cargo/core/compiler/build_runner/mod.rs index d55208d667e..ca223cf8395 100644 --- a/src/cargo/core/compiler/build_runner/mod.rs +++ b/src/cargo/core/compiler/build_runner/mod.rs @@ -316,17 +316,17 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> { if unit.mode == CompileMode::Test { self.compilation .tests - .push(self.unit_output(unit, &output.path)); + .push(self.unit_output(unit, &output.path)?); } else if unit.target.is_executable() { self.compilation .binaries - .push(self.unit_output(unit, bindst)); + .push(self.unit_output(unit, bindst)?); } else if unit.target.is_cdylib() && !self.compilation.cdylibs.iter().any(|uo| uo.unit == *unit) { self.compilation .cdylibs - .push(self.unit_output(unit, bindst)); + .push(self.unit_output(unit, bindst)?); } } Ok(()) @@ -554,13 +554,15 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> { /// Returns a [`UnitOutput`] which represents some information about the /// output of a unit. - pub fn unit_output(&self, unit: &Unit, path: &Path) -> UnitOutput { + pub fn unit_output(&self, unit: &Unit, path: &Path) -> CargoResult { let script_metas = self.find_build_script_metadatas(unit); - UnitOutput { + let env = artifact::get_env(&self, unit, self.unit_deps(unit))?; + Ok(UnitOutput { unit: unit.clone(), path: path.to_path_buf(), script_metas, - } + env, + }) } /// Check if any output file name collision happens. diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index 9c4febabd91..24431fe979b 100644 --- a/src/cargo/core/compiler/compilation.rs +++ b/src/cargo/core/compiler/compilation.rs @@ -62,6 +62,9 @@ pub struct UnitOutput { /// /// This is used for indexing [`Compilation::extra_env`]. pub script_metas: Option>, + + /// Environment variables to set in the unit's process. + pub env: HashMap, } /// A structure returning the result of a compilation. diff --git a/src/cargo/ops/cargo_run.rs b/src/cargo/ops/cargo_run.rs index b7a56a8a0b0..240865ec5e5 100644 --- a/src/cargo/ops/cargo_run.rs +++ b/src/cargo/ops/cargo_run.rs @@ -97,6 +97,7 @@ pub fn run( unit, path, script_metas, + env: _env, } = &compile.binaries[0]; let exe = match path.strip_prefix(gctx.cwd()) { Ok(path) if path.file_name() == Some(path.as_os_str()) => Path::new(".").join(path), diff --git a/src/cargo/ops/cargo_test.rs b/src/cargo/ops/cargo_test.rs index 7ddf662464b..9ea6231ef56 100644 --- a/src/cargo/ops/cargo_test.rs +++ b/src/cargo/ops/cargo_test.rs @@ -8,6 +8,7 @@ use crate::util::errors::CargoResult; use crate::util::{CliError, CliResult, GlobalContext, add_path_args}; use anyhow::format_err; use cargo_util::{ProcessBuilder, ProcessError}; +use std::collections::HashMap; use std::ffi::OsString; use std::fmt::Write; use std::path::{Path, PathBuf}; @@ -126,6 +127,7 @@ fn run_unit_tests( unit, path, script_metas, + env, } in compilation.tests.iter() { let (exe_display, mut cmd) = cmd_builds( @@ -134,6 +136,7 @@ fn run_unit_tests( unit, path, script_metas.as_ref(), + env, test_args, compilation, "unittests", @@ -287,6 +290,7 @@ fn display_no_run_information( unit, path, script_metas, + env, } in compilation.tests.iter() { let (exe_display, cmd) = cmd_builds( @@ -295,6 +299,7 @@ fn display_no_run_information( unit, path, script_metas.as_ref(), + env, test_args, compilation, exec_type, @@ -319,6 +324,7 @@ fn cmd_builds( unit: &Unit, path: &PathBuf, script_metas: Option<&Vec>, + env: &HashMap, test_args: &[&str], compilation: &Compilation<'_>, exec_type: &str, @@ -348,6 +354,9 @@ fn cmd_builds( if unit.target.harness() && gctx.shell().verbosity() == Verbosity::Quiet { cmd.arg("--quiet"); } + for (key, val) in env.iter() { + cmd.env(key, val); + } Ok((exe_display, cmd)) } diff --git a/tests/testsuite/test.rs b/tests/testsuite/test.rs index 66db48ce8f3..ef4c985f7c6 100644 --- a/tests/testsuite/test.rs +++ b/tests/testsuite/test.rs @@ -5216,9 +5216,9 @@ fn bin_env_for_test() { assert_eq!(env!("CARGO_BIN_EXE_foo"), ""); assert_eq!(env!("CARGO_BIN_EXE_with-dash"), ""); assert_eq!(env!("CARGO_BIN_EXE_grüßen"), ""); - assert_eq!(std::env::var("CARGO_BIN_EXE_foo").ok(), None); - assert_eq!(std::env::var("CARGO_BIN_EXE_with-dash").ok(), None); - assert_eq!(std::env::var("CARGO_BIN_EXE_grüßen").ok(), None); + assert_eq!(std::env::var("CARGO_BIN_EXE_foo").ok().as_deref(), Some("")); + assert_eq!(std::env::var("CARGO_BIN_EXE_with-dash").ok().as_deref(), Some("")); + assert_eq!(std::env::var("CARGO_BIN_EXE_grüßen").ok().as_deref(), Some("")); } "# .replace("", &bin_path("foo"))