From c3c5167d4a851f8c799f7012dba8d73470230ee5 Mon Sep 17 00:00:00 2001
From: Eric Huss <eric@huss.org>
Date: Sat, 4 Feb 2023 07:50:30 -0800
Subject: [PATCH] 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.
---
 src/toolchain.rs           | 19 +++++++++++++++++++
 tests/cli-rustup.rs        | 20 +++++++++++++++++---
 tests/mock/mock_bin_src.rs |  6 ++++--
 3 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/src/toolchain.rs b/src/toolchain.rs
index 550a4842da6..991053e2af7 100644
--- a/src/toolchain.rs
+++ b/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);
     }
 }
diff --git a/tests/cli-rustup.rs b/tests/cli-rustup.rs
index 47e388bfb86..938e93ec8b6 100644
--- a/tests/cli-rustup.rs
+++ b/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");
+        }
     });
 }
 
diff --git a/tests/mock/mock_bin_src.rs b/tests/mock/mock_bin_src.rs
index 24add3c43e6..4d4ba254320 100644
--- a/tests/mock/mock_bin_src.rs
+++ b/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();