Skip to content

Commit 352175f

Browse files
committed
Auto merge of #11285 - jonhoo:cargo-env, r=weihanglo
Make cargo forward pre-existing CARGO if set Currently, Cargo will always set `$CARGO` to point to what it detects its own path to be (using `std::env::current_exe`). Unfortunately, this runs into trouble when Cargo is used as a library, or when `current_exe` is not actually the binary itself (e.g., when invoked through Valgrind or `ld.so`), since `$CARGO` will not point at something that can be used as `cargo`. This, in turn, means that users can't currently rely on `$CARGO` to do the right thing, and will sometimes have to invoke `cargo` directly from `$PATH` instead, which may not reflect the `cargo` that's currently in use. This patch makes Cargo re-use the existing value of `$CARGO` if it's already set in the environment. For Cargo subcommands, this will mean that the initial invocation of `cargo` in `cargo foo` will set `$CARGO`, and then Cargo-as-a-library inside of `cargo-foo` will inherit that (correct) value instead of overwriting it with the incorrect value `cargo-foo`. For other execution environments that do not have `cargo` in their call stack, it gives them the opportunity to set a working value for `$CARGO`. One note about the implementation of this is that the test suite now needs to override `$CARGO` explicitly so that the _user's_ `$CARGO` does not interfere with the contents of the tests. It _could_ remove `$CARGO` instead, but overriding it seemed less error-prone. Fixes #10119. Fixes #10113.
2 parents 65b2149 + 724a197 commit 352175f

File tree

7 files changed

+78
-10
lines changed

7 files changed

+78
-10
lines changed

crates/cargo-test-support/src/lib.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -410,8 +410,10 @@ impl Project {
410410
/// Example:
411411
/// p.cargo("build --bin foo").run();
412412
pub fn cargo(&self, cmd: &str) -> Execs {
413-
let mut execs = self.process(&cargo_exe());
413+
let cargo = cargo_exe();
414+
let mut execs = self.process(&cargo);
414415
if let Some(ref mut p) = execs.process_builder {
416+
p.env("CARGO", cargo);
415417
p.arg_line(cmd);
416418
}
417419
execs
@@ -1328,7 +1330,9 @@ impl ArgLine for snapbox::cmd::Command {
13281330
}
13291331

13301332
pub fn cargo_process(s: &str) -> Execs {
1331-
let mut p = process(&cargo_exe());
1333+
let cargo = cargo_exe();
1334+
let mut p = process(&cargo);
1335+
p.env("CARGO", cargo);
13321336
p.arg_line(s);
13331337
execs().with_process_builder(p)
13341338
}

src/cargo/ops/registry/auth.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ fn run_command(
123123

124124
let mut cmd = Command::new(&exe);
125125
cmd.args(args)
126-
.env("CARGO", config.cargo_exe()?)
126+
.env(crate::CARGO_ENV, config.cargo_exe()?)
127127
.env("CARGO_REGISTRY_NAME", name)
128128
.env("CARGO_REGISTRY_API_URL", api_url);
129129
match action {

src/cargo/util/config/mod.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,18 @@ impl Config {
406406
pub fn cargo_exe(&self) -> CargoResult<&Path> {
407407
self.cargo_exe
408408
.try_borrow_with(|| {
409+
fn from_env() -> CargoResult<PathBuf> {
410+
// Try re-using the `cargo` set in the environment already. This allows
411+
// commands that use Cargo as a library to inherit (via `cargo <subcommand>`)
412+
// or set (by setting `$CARGO`) a correct path to `cargo` when the current exe
413+
// is not actually cargo (e.g., `cargo-*` binaries, Valgrind, `ld.so`, etc.).
414+
let exe = env::var_os(crate::CARGO_ENV)
415+
.map(PathBuf::from)
416+
.ok_or_else(|| anyhow!("$CARGO not set"))?
417+
.canonicalize()?;
418+
Ok(exe)
419+
}
420+
409421
fn from_current_exe() -> CargoResult<PathBuf> {
410422
// Try fetching the path to `cargo` using `env::current_exe()`.
411423
// The method varies per operating system and might fail; in particular,
@@ -431,7 +443,8 @@ impl Config {
431443
paths::resolve_executable(&argv0)
432444
}
433445

434-
let exe = from_current_exe()
446+
let exe = from_env()
447+
.or_else(|_| from_current_exe())
435448
.or_else(|_| from_argv())
436449
.with_context(|| "couldn't get the path to cargo executable")?;
437450
Ok(exe)

src/doc/src/reference/environment-variables.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ system:
2323
* `CARGO_TARGET_DIR` — Location of where to place all generated artifacts,
2424
relative to the current working directory. See [`build.target-dir`] to set
2525
via config.
26+
* `CARGO` - If set, Cargo will forward this value instead of setting it
27+
to its own auto-detected path when it builds crates and when it
28+
executes build scripts and external subcommands. This value is not
29+
directly executed by Cargo, and should always point at a command that
30+
behaves exactly like `cargo`, as that's what users of the variable
31+
will be expecting.
2632
* `RUSTC` — Instead of running `rustc`, Cargo will execute this specified
2733
compiler instead. See [`build.rustc`] to set via config.
2834
* `RUSTC_WRAPPER` — Instead of simply running `rustc`, Cargo will execute this

tests/testsuite/build_script.rs

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@ use cargo_test_support::compare::assert_match_exact;
44
use cargo_test_support::paths::CargoPathExt;
55
use cargo_test_support::registry::Package;
66
use cargo_test_support::tools;
7-
use cargo_test_support::{basic_manifest, cross_compile, is_coarse_mtime, project, project_in};
7+
use cargo_test_support::{
8+
basic_manifest, cargo_exe, cross_compile, is_coarse_mtime, project, project_in,
9+
};
810
use cargo_test_support::{rustc_host, sleep_ms, slow_cpu_multiplier, symlink_supported};
9-
use cargo_util::paths::remove_dir_all;
11+
use cargo_util::paths::{self, remove_dir_all};
1012
use std::env;
1113
use std::fs;
1214
use std::io;
@@ -80,8 +82,15 @@ fn custom_build_env_vars() {
8082
)
8183
.file("bar/src/lib.rs", "pub fn hello() {}");
8284

85+
let cargo = cargo_exe().canonicalize().unwrap();
86+
let cargo = cargo.to_str().unwrap();
87+
let rustc = paths::resolve_executable("rustc".as_ref())
88+
.unwrap()
89+
.canonicalize()
90+
.unwrap();
91+
let rustc = rustc.to_str().unwrap();
8392
let file_content = format!(
84-
r#"
93+
r##"
8594
use std::env;
8695
use std::path::Path;
8796
@@ -107,7 +116,12 @@ fn custom_build_env_vars() {
107116
108117
let _feat = env::var("CARGO_FEATURE_FOO").unwrap();
109118
110-
let _cargo = env::var("CARGO").unwrap();
119+
let cargo = env::var("CARGO").unwrap();
120+
if env::var_os("CHECK_CARGO_IS_RUSTC").is_some() {{
121+
assert_eq!(cargo, r#"{rustc}"#);
122+
}} else {{
123+
assert_eq!(cargo, r#"{cargo}"#);
124+
}}
111125
112126
let rustc = env::var("RUSTC").unwrap();
113127
assert_eq!(rustc, "rustc");
@@ -124,7 +138,7 @@ fn custom_build_env_vars() {
124138
let rustflags = env::var("CARGO_ENCODED_RUSTFLAGS").unwrap();
125139
assert_eq!(rustflags, "");
126140
}}
127-
"#,
141+
"##,
128142
p.root()
129143
.join("target")
130144
.join("debug")
@@ -135,6 +149,11 @@ fn custom_build_env_vars() {
135149
let p = p.file("bar/build.rs", &file_content).build();
136150

137151
p.cargo("build --features bar_feat").run();
152+
p.cargo("build --features bar_feat")
153+
// we use rustc since $CARGO is only used if it points to a path that exists
154+
.env("CHECK_CARGO_IS_RUSTC", "1")
155+
.env(cargo::CARGO_ENV, rustc)
156+
.run();
138157
}
139158

140159
#[cargo_test]

tests/testsuite/cargo_command.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,13 +335,26 @@ fn cargo_subcommand_env() {
335335

336336
let cargo = cargo_exe().canonicalize().unwrap();
337337
let mut path = path();
338-
path.push(target_dir);
338+
path.push(target_dir.clone());
339339
let path = env::join_paths(path.iter()).unwrap();
340340

341341
cargo_process("envtest")
342342
.env("PATH", &path)
343343
.with_stdout(cargo.to_str().unwrap())
344344
.run();
345+
346+
// Check that subcommands inherit an overriden $CARGO
347+
let envtest_bin = target_dir
348+
.join("cargo-envtest")
349+
.with_extension(std::env::consts::EXE_EXTENSION)
350+
.canonicalize()
351+
.unwrap();
352+
let envtest_bin = envtest_bin.to_str().unwrap();
353+
cargo_process("envtest")
354+
.env("PATH", &path)
355+
.env(cargo::CARGO_ENV, &envtest_bin)
356+
.with_stdout(envtest_bin)
357+
.run();
345358
}
346359

347360
#[cargo_test]

tests/testsuite/test.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3485,6 +3485,19 @@ fn cargo_test_env() {
34853485
.with_stderr_contains(cargo.to_str().unwrap())
34863486
.with_stdout_contains("test env_test ... ok")
34873487
.run();
3488+
3489+
// Check that `cargo test` propagates the environment's $CARGO
3490+
let rustc = cargo_util::paths::resolve_executable("rustc".as_ref())
3491+
.unwrap()
3492+
.canonicalize()
3493+
.unwrap();
3494+
let rustc = rustc.to_str().unwrap();
3495+
p.cargo("test --lib -- --nocapture")
3496+
// we use rustc since $CARGO is only used if it points to a path that exists
3497+
.env(cargo::CARGO_ENV, rustc)
3498+
.with_stderr_contains(rustc)
3499+
.with_stdout_contains("test env_test ... ok")
3500+
.run();
34883501
}
34893502

34903503
#[cargo_test]

0 commit comments

Comments
 (0)