Skip to content

Commit

Permalink
Don't add toolchain bin to PATH on Windows
Browse files Browse the repository at this point in the history
  • Loading branch information
ehuss committed Jan 31, 2023
1 parent a942c34 commit 4d7311f
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 5 deletions.
4 changes: 0 additions & 4 deletions src/toolchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -452,10 +452,6 @@ impl<'a> InstalledCommonToolchain<'a> {
path_entries.push(cargo_home.join("bin"));
}

if cfg!(target_os = "windows") {
path_entries.push(self.0.path.join("bin"));
}

env_var::prepend_path("PATH", path_entries, cmd);
}
}
Expand Down
45 changes: 45 additions & 0 deletions tests/cli-rustup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,51 @@ fn fallback_cargo_calls_correct_rustc() {
});
}

// Checks that cargo can recursively invoke itself with rustup shorthand (via
// the proxy).
//
// This involves a series of chained commands:
//
// 1. Calls `cargo --recursive-cargo-subcommand`
// 2. The rustup `cargo` proxy launches, and launches the "mock" nightly cargo exe.
// 3. The nightly "mock" cargo sees --recursive-cargo-subcommand, and launches
// `cargo-foo --recursive-cargo`
// 4. `cargo-foo` sees `--recursive-cargo` and launches `cargo +nightly --version`
// 5. The rustup `cargo` proxy launches, and launches the "mock" nightly cargo exe.
// 6. The nightly "mock" cargo sees `--version` and prints the version.
//
// Previously, rustup would place the toolchain's `bin` directory in PATH for
// Windows due to some DLL issues. However, those aren't necessary anymore.
// If the toolchain `bin` directory is in PATH, then this test would fail in
// step 5 because the `cargo` executable would be the "mock" nightly cargo,
// and the first argument would be `+nightly` which would be an error.
#[test]
fn recursive_cargo() {
setup(&|config| {
expect_ok(config, &["rustup", "default", "nightly"]);

// We need an intermediary to run cargo itself.
// The "mock" cargo can't do that because on Windows it will check
// for a `cargo.exe` in the current directory before checking PATH.
//
// The solution here is to copy from the "mock" `cargo.exe` into
// `~/.cargo/bin/cargo-foo`. This is just for convenience to avoid
// needing to build another executable just for this test.
let output = clitools::run(config, "rustup", &["which", "cargo"], &[]);
let real_mock_cargo = output.stdout.trim();
let cargo_bin_path = config.cargodir.join("bin");
let cargo_subcommand = cargo_bin_path.join(format!("cargo-foo{}", EXE_SUFFIX));
fs::create_dir_all(&cargo_bin_path).unwrap();
fs::copy(&real_mock_cargo, &cargo_subcommand).unwrap();

expect_stdout_ok(
config,
&["cargo", "--recursive-cargo-subcommand"],
"hash-nightly-2",
);
});
}

#[test]
fn show_home() {
setup(&|config| {
Expand Down
14 changes: 13 additions & 1 deletion tests/mock/mock_bin_src.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,18 @@ fn main() {
let rustc = &format!("rustc{}", EXE_SUFFIX);
Command::new(rustc).arg("--version").status().unwrap();
}
Some("--recursive-cargo-subcommand") => {
Command::new("cargo-foo")
.arg("--recursive-cargo")
.status()
.unwrap();
}
Some("--recursive-cargo") => {
Command::new("cargo")
.args(&["+nightly", "--version"])
.status()
.unwrap();
}
Some("--echo-args") => {
let mut out = io::stderr();
for arg in args {
Expand All @@ -71,7 +83,7 @@ fn main() {
let mut out = io::stderr();
writeln!(out, "{}", std::env::var("PATH").unwrap()).unwrap();
}
_ => panic!("bad mock proxy commandline"),
arg => panic!("bad mock proxy commandline: {:?}", arg),
}
}

Expand Down

0 comments on commit 4d7311f

Please sign in to comment.