Skip to content

Update CARGO environment variable if it is set to a rustup proxy #4275

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

Merged
merged 2 commits into from
Apr 4, 2025
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
1 change: 1 addition & 0 deletions src/test/clitools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -684,6 +684,7 @@ async fn setup_test_state(test_dist_dir: tempfile::TempDir) -> (tempfile::TempDi
// **in this process** when the tests are still running.
unsafe {
// Unset env variables that will break our testing
env::remove_var("CARGO");
env::remove_var("RUSTUP_UPDATE_ROOT");
env::remove_var("RUSTUP_TOOLCHAIN");
env::remove_var("SHELL");
Expand Down
8 changes: 8 additions & 0 deletions src/test/mock_bin_src.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,14 @@ fn main() {
let mut out = io::stderr();
writeln!(out, "{}", std::env::var("PATH").unwrap()).unwrap();
}
Some("--echo-cargo-env") => {
let mut out = io::stderr();
if let Ok(cargo) = std::env::var("CARGO") {
writeln!(out, "{cargo}").unwrap();
} else {
panic!("CARGO environment variable not set");
}
}
arg => panic!("bad mock proxy commandline: {:?}", arg),
}
}
Expand Down
13 changes: 13 additions & 0 deletions src/toolchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,19 @@ impl<'a> Toolchain<'a> {
};
let mut cmd = Command::new(path);
self.set_env(&mut cmd);

// If we're running cargo and the `CARGO` environment variable is set
// to a rustup proxy then change `CARGO` to be the real cargo binary,
// but only if we know the absolute path to cargo.
// This works around an issue with old versions of cargo not updating
// the environment variable itself.
if Path::new(&binary).file_stem() == Some("cargo".as_ref()) && path.is_absolute() {
if let Some(cargo) = self.cfg.process.var_os("CARGO") {
if fs::read_link(&cargo).is_ok_and(|p| p.file_stem() == Some("rustup".as_ref())) {
cmd.env("CARGO", path);
}
}
}
Ok(cmd)
}

Expand Down
44 changes: 44 additions & 0 deletions tests/suite/cli_misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1233,3 +1233,47 @@ async fn toolchain_install_multi_components_comma() {
.await;
}
}

#[tokio::test]
async fn rustup_updates_cargo_env_if_proxy() {
let mut cx = CliTestContext::new(Scenario::SimpleV2).await;
cx.config.expect_ok(&["rustup", "default", "stable"]).await;

use std::env::consts::EXE_SUFFIX;
let proxy_path = cx
.config
.workdir
.borrow()
.join("bin")
.join(format!("cargo{EXE_SUFFIX}"));
let real_path = cx.config.run("rustup", &["which", "cargo"], &[]).await;
assert!(real_path.ok);
let real_path = real_path.stdout;

fs::create_dir_all(proxy_path.parent().unwrap()).unwrap();
#[cfg(windows)]
if std::os::windows::fs::symlink_file("rustup", &proxy_path).is_err() {
// skip this test on Windows if symlinking isn't enabled.
return;
}
#[cfg(unix)]
std::os::unix::fs::symlink("rustup", &proxy_path).unwrap();

// If CARGO isn't set then we should not set it.
cx.config
.expect_err(
&["cargo", "--echo-cargo-env"],
"CARGO environment variable not set",
)
.await;

// If CARGO is set to a proxy then change it to the real CARGO path
cx.config
.expect_ok_ex_env(
&["cargo", "--echo-cargo-env"],
&[("CARGO", proxy_path.to_str().unwrap())],
"",
&real_path,
)
.await;
}