From a82794ec4a205b58ad7cfd4382f50db6690ac218 Mon Sep 17 00:00:00 2001
From: Eric Huss <eric@huss.org>
Date: Tue, 20 Feb 2024 16:27:04 -0800
Subject: [PATCH] Fix path issues for running rustup wrapper on Windows.

Cargo likes to modify PATH, which circumvents the ability to choose the
correct "cargo" executable to run on Windows (because Windows uses PATH
for both binary and shared library searching).
---
 crates/cargo-test-macro/src/lib.rs      | 21 ++++++++++++++++-----
 tests/testsuite/global_cache_tracker.rs | 23 +++++++++++++++--------
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/crates/cargo-test-macro/src/lib.rs b/crates/cargo-test-macro/src/lib.rs
index ef59733fcd3..f813b835a45 100644
--- a/crates/cargo-test-macro/src/lib.rs
+++ b/crates/cargo-test-macro/src/lib.rs
@@ -1,4 +1,5 @@
 use proc_macro::*;
+use std::path::Path;
 use std::process::Command;
 use std::sync::Once;
 
@@ -210,8 +211,9 @@ fn version() -> (u32, bool) {
     unsafe { VERSION }
 }
 
-fn check_command(command_name: &str, args: &[&str]) -> bool {
-    let mut command = Command::new(command_name);
+fn check_command(command_path: &Path, args: &[&str]) -> bool {
+    let mut command = Command::new(command_path);
+    let command_name = command.get_program().to_str().unwrap().to_owned();
     command.args(args);
     let output = match command.output() {
         Ok(output) => output,
@@ -220,7 +222,7 @@ fn check_command(command_name: &str, args: &[&str]) -> bool {
             //   environments like Docker. Consider installing it if Cargo
             //   gains more hg support, but otherwise it isn't critical.
             // * lldb is not pre-installed on Ubuntu and Windows, so skip.
-            if is_ci() && !matches!(command_name, "hg" | "lldb") {
+            if is_ci() && !matches!(command_name.as_str(), "hg" | "lldb") {
                 panic!("expected command `{command_name}` to be somewhere in PATH: {e}",);
             }
             return false;
@@ -240,7 +242,7 @@ fn check_command(command_name: &str, args: &[&str]) -> bool {
 }
 
 fn has_command(command: &str) -> bool {
-    check_command(command, &["--version"])
+    check_command(Path::new(command), &["--version"])
 }
 
 fn has_rustup_stable() -> bool {
@@ -255,7 +257,16 @@ fn has_rustup_stable() -> bool {
         // https://github.com/rust-lang/rustup/issues/3036 is resolved.
         return false;
     }
-    check_command("cargo", &["+stable", "--version"])
+    // Cargo mucks with PATH on Windows, adding sysroot host libdir, which is
+    // "bin", which circumvents the rustup wrapper. Use the path directly from
+    // CARGO_HOME.
+    let home = match option_env!("CARGO_HOME") {
+        Some(home) => home,
+        None if is_ci() => panic!("expected to run under rustup"),
+        None => return false,
+    };
+    let cargo = Path::new(home).join("bin/cargo");
+    check_command(&cargo, &["+stable", "--version"])
 }
 
 /// Whether or not this running in a Continuous Integration environment.
diff --git a/tests/testsuite/global_cache_tracker.rs b/tests/testsuite/global_cache_tracker.rs
index f55e01fb6d8..436013e4e3c 100644
--- a/tests/testsuite/global_cache_tracker.rs
+++ b/tests/testsuite/global_cache_tracker.rs
@@ -14,11 +14,12 @@ use cargo::GlobalContext;
 use cargo_test_support::paths::{self, CargoPathExt};
 use cargo_test_support::registry::{Package, RegistryBuilder};
 use cargo_test_support::{
-    basic_manifest, cargo_process, execs, git, project, retry, sleep_ms, thread_wait_timeout,
-    Project,
+    basic_manifest, cargo_process, execs, git, process, project, retry, sleep_ms,
+    thread_wait_timeout, Execs, Project,
 };
 use itertools::Itertools;
 use std::fmt::Write;
+use std::path::Path;
 use std::path::PathBuf;
 use std::process::Stdio;
 use std::time::{Duration, SystemTime};
@@ -153,6 +154,14 @@ fn populate_cache(
     (cache_dir, src_dir)
 }
 
+fn rustup_cargo() -> Execs {
+    // Get the path to the rustup cargo wrapper. This is necessary because
+    // cargo adds the "deps" directory into PATH on Windows, which points to
+    // the wrong cargo.
+    let rustup_cargo = Path::new(&std::env::var_os("CARGO_HOME").unwrap()).join("bin/cargo");
+    execs().with_process_builder(process(rustup_cargo))
+}
+
 #[cargo_test]
 fn auto_gc_gated() {
     // Requires -Zgc to both track last-use data and to run auto-gc.
@@ -1915,12 +1924,11 @@ fn compatible_with_older_cargo() {
             middle = "1.0"
         "#,
     );
-    p.process("cargo")
+    rustup_cargo()
         .args(&["+stable", "check", "-Zgc"])
+        .cwd(p.root())
         .masquerade_as_nightly_cargo(&["gc"])
         .env("__CARGO_TEST_LAST_USE_NOW", months_ago_unix(2))
-        // Necessary since `process` removes rustup.
-        .env("PATH", std::env::var_os("PATH").unwrap())
         .run();
     assert_eq!(get_registry_names("src"), ["middle-1.0.0", "new-1.0.0"]);
     assert_eq!(
@@ -1978,11 +1986,10 @@ fn forward_compatible() {
         .file("src/lib.rs", "")
         .build();
 
-    p.process("cargo")
+    rustup_cargo()
         .args(&["+stable", "check", "-Zgc"])
+        .cwd(p.root())
         .masquerade_as_nightly_cargo(&["gc"])
-        // Necessary since `process` removes rustup.
-        .env("PATH", std::env::var_os("PATH").unwrap())
         .run();
 
     let config = GlobalContextBuilder::new().unstable_flag("gc").build();