Don't add toolchain bin to PATH on Windows

Author: ehuss

src/toolchain.rs

@@ -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);
     }
 }

tests/cli-rustup.rs

@@ -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| {

tests/mock/mock_bin_src.rs

@@ -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 {
@@ -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),
     }
 }