Add RUSTUP_WINDOWS_PATH_ADD_BIN

Due to the uncertainty around whether or not this change will
cause problems, this introduces an opt-in for removing the PATH
change on Windows.

Author: ehuss

src/toolchain.rs

@@ -452,6 +452,25 @@ impl<'a> InstalledCommonToolchain<'a> {
             path_entries.push(cargo_home.join("bin"));
         }
 
+        if cfg!(target_os = "windows") {
+            // Historically rustup has included the bin directory in PATH to
+            // work around some bugs (see
+            // https://github.com/rust-lang/rustup/pull/3178 for more
+            // information). This shouldn't be needed anymore, and it causes
+            // problems because calling tools recursively (like `cargo
+            // +nightly metadata` from within a cargo subcommand). The
+            // recursive call won't work because it is not executing the
+            // proxy, so the `+` toolchain override doesn't work.
+            //
+            // This is opt-in to allow us to get more real-world testing.
+            if process()
+                .var_os("RUSTUP_WINDOWS_PATH_ADD_BIN")
+                .map_or(true, |s| s == "1")
+            {
+                path_entries.push(self.0.path.join("bin"));
+            }
+        }
+
         env_var::prepend_path("PATH", path_entries, cmd);
     }
 }

tests/cli-rustup.rs

@@ -586,11 +586,25 @@ fn recursive_cargo() {
         fs::create_dir_all(&cargo_bin_path).unwrap();
         fs::copy(&real_mock_cargo, &cargo_subcommand).unwrap();
 
-        expect_stdout_ok(
+        let args = &["cargo", "--recursive-cargo-subcommand"];
+        if cfg!(windows) {
+            expect_err(
+                config,
+                &["cargo", "--recursive-cargo-subcommand"],
+                "bad mock proxy commandline",
+            );
+        }
+
+        let out = run(
             config,
-            &["cargo", "--recursive-cargo-subcommand"],
-            "hash-nightly-2",
+            args[0],
+            &args[1..],
+            &[("RUSTUP_WINDOWS_PATH_ADD_BIN", "0")],
         );
+        if !out.ok || !out.stdout.contains("hash-nightly-2") {
+            clitools::print_command(args, &out);
+            panic!("expected hash-nightly-2 in output");
+        }
     });
 }
 

tests/mock/mock_bin_src.rs

@@ -62,16 +62,18 @@ fn main() {
             Command::new(rustc).arg("--version").status().unwrap();
         }
         Some("--recursive-cargo-subcommand") => {
-            Command::new("cargo-foo")
+            let status = Command::new("cargo-foo")
                 .arg("--recursive-cargo")
                 .status()
                 .unwrap();
+            assert!(status.success());
         }
         Some("--recursive-cargo") => {
-            Command::new("cargo")
+            let status = Command::new("cargo")
                 .args(&["+nightly", "--version"])
                 .status()
                 .unwrap();
+            assert!(status.success());
         }
         Some("--echo-args") => {
             let mut out = io::stderr();