Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make cargo forward pre-existing CARGO if set #11285

Merged
merged 1 commit into from
Nov 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions crates/cargo-test-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,8 +410,10 @@ impl Project {
/// Example:
/// p.cargo("build --bin foo").run();
pub fn cargo(&self, cmd: &str) -> Execs {
let mut execs = self.process(&cargo_exe());
let cargo = cargo_exe();
let mut execs = self.process(&cargo);
if let Some(ref mut p) = execs.process_builder {
p.env("CARGO", cargo);
p.arg_line(cmd);
}
execs
Expand Down Expand Up @@ -1328,7 +1330,9 @@ impl ArgLine for snapbox::cmd::Command {
}

pub fn cargo_process(s: &str) -> Execs {
let mut p = process(&cargo_exe());
let cargo = cargo_exe();
let mut p = process(&cargo);
p.env("CARGO", cargo);
p.arg_line(s);
execs().with_process_builder(p)
}
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/registry/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ fn run_command(

let mut cmd = Command::new(&exe);
cmd.args(args)
.env("CARGO", config.cargo_exe()?)
.env(crate::CARGO_ENV, config.cargo_exe()?)
.env("CARGO_REGISTRY_NAME", name)
.env("CARGO_REGISTRY_API_URL", api_url);
match action {
Expand Down
15 changes: 14 additions & 1 deletion src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,18 @@ impl Config {
pub fn cargo_exe(&self) -> CargoResult<&Path> {
self.cargo_exe
.try_borrow_with(|| {
fn from_env() -> CargoResult<PathBuf> {
// Try re-using the `cargo` set in the environment already. This allows
// commands that use Cargo as a library to inherit (via `cargo <subcommand>`)
// or set (by setting `$CARGO`) a correct path to `cargo` when the current exe
// is not actually cargo (e.g., `cargo-*` binaries, Valgrind, `ld.so`, etc.).
let exe = env::var_os(crate::CARGO_ENV)
.map(PathBuf::from)
.ok_or_else(|| anyhow!("$CARGO not set"))?
.canonicalize()?;
Ok(exe)
}

fn from_current_exe() -> CargoResult<PathBuf> {
// Try fetching the path to `cargo` using `env::current_exe()`.
// The method varies per operating system and might fail; in particular,
Expand All @@ -431,7 +443,8 @@ impl Config {
paths::resolve_executable(&argv0)
}

let exe = from_current_exe()
let exe = from_env()
.or_else(|_| from_current_exe())
.or_else(|_| from_argv())
.with_context(|| "couldn't get the path to cargo executable")?;
Ok(exe)
Expand Down
6 changes: 6 additions & 0 deletions src/doc/src/reference/environment-variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ system:
* `CARGO_TARGET_DIR` — Location of where to place all generated artifacts,
relative to the current working directory. See [`build.target-dir`] to set
via config.
* `CARGO` - If set, Cargo will forward this value instead of setting it
to its own auto-detected path when it builds crates and when it
executes build scripts and external subcommands. This value is not
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we include that the value, if set, should be the path to the cargo binary performing the build?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that's necessary. It may also be confusing phrasing for tools that use Cargo as a library but do not otherwise have a cargo binary to point to (e.g., because they're not cargo subcommands).

directly executed by Cargo, and should always point at a command that
behaves exactly like `cargo`, as that's what users of the variable
will be expecting.
* `RUSTC` — Instead of running `rustc`, Cargo will execute this specified
compiler instead. See [`build.rustc`] to set via config.
* `RUSTC_WRAPPER` — Instead of simply running `rustc`, Cargo will execute this
Expand Down
29 changes: 24 additions & 5 deletions tests/testsuite/build_script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ use cargo_test_support::compare::assert_match_exact;
use cargo_test_support::paths::CargoPathExt;
use cargo_test_support::registry::Package;
use cargo_test_support::tools;
use cargo_test_support::{basic_manifest, cross_compile, is_coarse_mtime, project, project_in};
use cargo_test_support::{
basic_manifest, cargo_exe, cross_compile, is_coarse_mtime, project, project_in,
};
use cargo_test_support::{rustc_host, sleep_ms, slow_cpu_multiplier, symlink_supported};
use cargo_util::paths::remove_dir_all;
use cargo_util::paths::{self, remove_dir_all};
use std::env;
use std::fs;
use std::io;
Expand Down Expand Up @@ -80,8 +82,15 @@ fn custom_build_env_vars() {
)
.file("bar/src/lib.rs", "pub fn hello() {}");

let cargo = cargo_exe().canonicalize().unwrap();
let cargo = cargo.to_str().unwrap();
let rustc = paths::resolve_executable("rustc".as_ref())
.unwrap()
.canonicalize()
.unwrap();
let rustc = rustc.to_str().unwrap();
let file_content = format!(
r#"
r##"
use std::env;
use std::path::Path;

Expand All @@ -107,7 +116,12 @@ fn custom_build_env_vars() {

let _feat = env::var("CARGO_FEATURE_FOO").unwrap();

let _cargo = env::var("CARGO").unwrap();
let cargo = env::var("CARGO").unwrap();
if env::var_os("CHECK_CARGO_IS_RUSTC").is_some() {{
assert_eq!(cargo, r#"{rustc}"#);
}} else {{
assert_eq!(cargo, r#"{cargo}"#);
}}

let rustc = env::var("RUSTC").unwrap();
assert_eq!(rustc, "rustc");
Expand All @@ -124,7 +138,7 @@ fn custom_build_env_vars() {
let rustflags = env::var("CARGO_ENCODED_RUSTFLAGS").unwrap();
assert_eq!(rustflags, "");
}}
"#,
"##,
p.root()
.join("target")
.join("debug")
Expand All @@ -135,6 +149,11 @@ fn custom_build_env_vars() {
let p = p.file("bar/build.rs", &file_content).build();

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

#[cargo_test]
Expand Down
15 changes: 14 additions & 1 deletion tests/testsuite/cargo_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,13 +335,26 @@ fn cargo_subcommand_env() {

let cargo = cargo_exe().canonicalize().unwrap();
let mut path = path();
path.push(target_dir);
path.push(target_dir.clone());
let path = env::join_paths(path.iter()).unwrap();

cargo_process("envtest")
.env("PATH", &path)
.with_stdout(cargo.to_str().unwrap())
.run();

// Check that subcommands inherit an overriden $CARGO
let envtest_bin = target_dir
.join("cargo-envtest")
.with_extension(std::env::consts::EXE_EXTENSION)
.canonicalize()
.unwrap();
let envtest_bin = envtest_bin.to_str().unwrap();
cargo_process("envtest")
.env("PATH", &path)
.env(cargo::CARGO_ENV, &envtest_bin)
.with_stdout(envtest_bin)
.run();
}

#[cargo_test]
Expand Down
13 changes: 13 additions & 0 deletions tests/testsuite/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3485,6 +3485,19 @@ fn cargo_test_env() {
.with_stderr_contains(cargo.to_str().unwrap())
.with_stdout_contains("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 rustc = rustc.to_str().unwrap();
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(rustc)
.with_stdout_contains("test env_test ... ok")
.run();
}

#[cargo_test]
Expand Down