From 44970a825d11b42ac8a2028e66a8e8553e1d667d Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Thu, 20 Feb 2025 06:23:08 -0500 Subject: [PATCH] Fix #15099 Overwrite `$CARGO` when the current exe is detected to be a cargo binary. See: https://github.com/rust-lang/cargo/issues/15099#issuecomment-2666737150 --- src/cargo/util/context/mod.rs | 20 +++++++- tests/testsuite/cargo_command.rs | 5 +- tests/testsuite/freshness.rs | 83 ++++++++++++++++++++++++++++++++ tests/testsuite/test.rs | 22 ++++----- 4 files changed, 116 insertions(+), 14 deletions(-) diff --git a/src/cargo/util/context/mod.rs b/src/cargo/util/context/mod.rs index 0b388978d..f55bc1bfa 100644 --- a/src/cargo/util/context/mod.rs +++ b/src/cargo/util/context/mod.rs @@ -493,9 +493,25 @@ impl GlobalContext { paths::resolve_executable(&argv0) } + // Determines whether `path` is a cargo binary. + // See: https://github.com/rust-lang/cargo/issues/15099#issuecomment-2666737150 + fn is_cargo(path: &Path) -> bool { + path.file_stem() == Some(OsStr::new("cargo")) + } + + let from_current_exe = from_current_exe(); + if from_current_exe.as_deref().is_ok_and(is_cargo) { + return from_current_exe; + } + + let from_argv = from_argv(); + if from_argv.as_deref().is_ok_and(is_cargo) { + return from_argv; + } + let exe = from_env() - .or_else(|_| from_current_exe()) - .or_else(|_| from_argv()) + .or(from_current_exe) + .or(from_argv) .context("couldn't get the path to cargo executable")?; Ok(exe) }) diff --git a/tests/testsuite/cargo_command.rs b/tests/testsuite/cargo_command.rs index 785b7d9c1..5e8356447 100644 --- a/tests/testsuite/cargo_command.rs +++ b/tests/testsuite/cargo_command.rs @@ -391,10 +391,13 @@ fn cargo_subcommand_env() { .canonicalize() .unwrap(); let envtest_bin = envtest_bin.to_str().unwrap(); + // Previously, `$CARGO` would be left at `envtest_bin`. However, with the + // fix for #15099, `$CARGO` is now overwritten with the path to the current + // exe when it is detected to be a cargo binary. cargo_process("envtest") .env("PATH", &path) .env(cargo::CARGO_ENV, &envtest_bin) - .with_stdout_data(format!("{}\n", envtest_bin).raw().raw()) + .with_stdout_data(format!("{}\n", cargo.display()).raw()) .run(); } diff --git a/tests/testsuite/freshness.rs b/tests/testsuite/freshness.rs index 7ad417e19..8429f9911 100644 --- a/tests/testsuite/freshness.rs +++ b/tests/testsuite/freshness.rs @@ -1,5 +1,6 @@ //! Tests for fingerprinting (rebuild detection). +use std::env::consts::EXE_SUFFIX; use std::fs::{self, OpenOptions}; use std::io; use std::io::prelude::*; @@ -3182,3 +3183,85 @@ fn use_mtime_cache_in_cargo_home() { "#]]) .run(); } + +#[cargo_test] +fn overwrite_cargo_environment_variable() { + // If passed arguments `arg1 arg2 ...`, this program runs them as a command. + // If passed no arguments, this program simply prints `$CARGO`. + let p = project() + .file("Cargo.toml", &basic_manifest("foo", "1.0.0")) + .file( + "src/main.rs", + r#" + fn main() { + let mut args = std::env::args().skip(1); + if let Some(arg1) = args.next() { + let status = std::process::Command::new(arg1) + .args(args) + .status() + .unwrap(); + assert!(status.success()); + } else { + eprintln!("{}", std::env::var("CARGO").unwrap()); + } + } + "#, + ) + .build(); + + // Create two other cargo binaries in the project root, one with the wrong + // name and one with the right name. + let cargo_exe = cargo_test_support::cargo_exe(); + let wrong_name_path = p.root().join(format!("wrong_name{EXE_SUFFIX}")); + let other_cargo_path = p.root().join(cargo_exe.file_name().unwrap()); + std::fs::hard_link(&cargo_exe, &wrong_name_path).unwrap(); + std::fs::hard_link(&cargo_exe, &other_cargo_path).unwrap(); + + // The output of each of the following commands should be `path-to-cargo`: + // ``` + // cargo run + // cargo run -- cargo run + // cargo run -- wrong_name run + // ``` + + let cargo = cargo_exe.display().to_string(); + let wrong_name = wrong_name_path.display().to_string(); + let stderr_cargo = format!( + "{}[EXE]\n", + cargo_exe + .canonicalize() + .unwrap() + .with_extension("") + .to_str() + .unwrap() + ); + + for cmd in [ + "run", + &format!("run -- {cargo} run"), + &format!("run -- {wrong_name} run"), + ] { + p.cargo(cmd).with_stderr_contains(&stderr_cargo).run(); + } + + // The output of the following command should be `path-to-other-cargo`: + // ``` + // cargo run -- other_cargo run + // ``` + + let other_cargo = other_cargo_path.display().to_string(); + let stderr_other_cargo = format!( + "{}[EXE]\n", + other_cargo_path + .canonicalize() + .unwrap() + .with_extension("") + .to_str() + .unwrap() + .replace(p.root().parent().unwrap().to_str().unwrap(), "[ROOT]") + ); + + p.cargo(&format!("run -- {other_cargo} run")) + .with_stderr_contains(stderr_other_cargo) + .run(); +} diff --git a/tests/testsuite/test.rs b/tests/testsuite/test.rs index e0ec17e43..d2eb46307 100644 --- a/tests/testsuite/test.rs +++ b/tests/testsuite/test.rs @@ -3909,22 +3909,22 @@ test env_test ... ok .run(); // Check that `cargo test` propagates the environment's $CARGO - let rustc = cargo_util::paths::resolve_executable("rustc".as_ref()) - .unwrap() - .canonicalize() - .unwrap(); - let stderr_rustc = format!( + let cargo_exe = cargo_test_support::cargo_exe(); + let other_cargo_path = p.root().join(cargo_exe.file_name().unwrap()); + std::fs::hard_link(&cargo_exe, &other_cargo_path).unwrap(); + let stderr_other_cargo = format!( "{}[EXE]", - rustc + other_cargo_path + .canonicalize() + .unwrap() .with_extension("") .to_str() .unwrap() - .replace(rustc_host, "[HOST_TARGET]") + .replace(p.root().parent().unwrap().to_str().unwrap(), "[ROOT]") ); - p.cargo("test --lib -- --nocapture") - // we use rustc since $CARGO is only used if it points to a path that exists - .env(cargo::CARGO_ENV, rustc) - .with_stderr_contains(stderr_rustc) + p.process(other_cargo_path) + .args(&["test", "--lib", "--", "--nocapture"]) + .with_stderr_contains(stderr_other_cargo) .with_stdout_data(str![[r#" ... test env_test ... ok