Skip to content

Commit

Permalink
Fix path issues for running rustup wrapper on Windows.
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
ehuss committed Feb 21, 2024
1 parent ccaa118 commit a82794e
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 13 deletions.
21 changes: 16 additions & 5 deletions crates/cargo-test-macro/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use proc_macro::*;
use std::path::Path;
use std::process::Command;
use std::sync::Once;

Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand All @@ -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 {
Expand All @@ -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.
Expand Down
23 changes: 15 additions & 8 deletions tests/testsuite/global_cache_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit a82794e

Please sign in to comment.