From 956daffe059733dce57dddd290603582c38c1a37 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sat, 12 Jan 2019 12:24:22 +0200 Subject: [PATCH] Replace Windows custom toolchain hack When using a custom toolchain on Windows, Rustup ensures that the fallback Cargo calls the correct rustc executable by hardlinking cargo into an empty directory and executing it there. The problem with this method is that if cargo is invoked twice at the same time, the second invocation fails because it can't replace the hardlink which it is in use by the first. This occurs when an integration test itself invokes cargo. This commit replaces the hack by specifying the correct version of rustc by setting the RUSTC environment variable when cargo is invoked. This is set to the absolute path of rustc in the correct toolchain rather than the rustup wrapper. This is consistent with the way rustc is invoked on a normal toolchain on Windows. --- src/rustup-mock/src/mock_bin_src.rs | 34 +++--------------------- src/rustup/toolchain.rs | 40 ++++++++++------------------- 2 files changed, 17 insertions(+), 57 deletions(-) diff --git a/src/rustup-mock/src/mock_bin_src.rs b/src/rustup-mock/src/mock_bin_src.rs index 993ad4b308..0fa58193cf 100644 --- a/src/rustup-mock/src/mock_bin_src.rs +++ b/src/rustup-mock/src/mock_bin_src.rs @@ -14,34 +14,6 @@ fn main() { let mut hash_file = PathBuf::from(format!("{}.version-hash", me.display())); let mut version = String::new(); let mut hash = String::new(); - if !version_file.exists() { - // There's a "MAJOR HACKS" statement in `toolchain.rs` right - // now where custom toolchains use a `cargo.exe` that's - // temporarily located elsewhere so they can execute the correct - // `rustc.exe`. This means that our dummy version files may not - // be just next to use. - // - // Detect this here and work around it. - assert!(cfg!(windows)); - assert!(env::var_os("RUSTUP_TOOLCHAIN").is_some()); - let mut alt = me.clone(); - alt.pop(); // remove our filename - assert!(alt.ends_with("fallback")); - alt.pop(); // pop 'fallback' - alt.push("toolchains"); - - let mut part = PathBuf::from("bin"); - part.push(me.file_name().unwrap()); - - let path = alt.read_dir().unwrap() - .map(|e| e.unwrap().path().join(&part)) - .filter(|p| p.exists()) - .find(|p| equivalent(&p, &me)) - .unwrap(); - - version_file = format!("{}.version", path.display()).into(); - hash_file = format!("{}.version-hash", path.display()).into(); - } File::open(&version_file).unwrap().read_to_string(&mut version).unwrap(); File::open(&hash_file).unwrap().read_to_string(&mut hash).unwrap(); println!("{} ({})", version, hash); @@ -57,9 +29,9 @@ fn main() { } Some("--call-rustc") => { // Used by the fallback_cargo_calls_correct_rustc test. Tests that - // the environment has been set up right such that invoking rustc - // will actually invoke the wrapper - let rustc = &format!("rustc{}", EXE_SUFFIX); + // the environment has been set up right such that invoking cargo + // will invoke the correct rustc executable. + let rustc = env::var_os("RUSTC").unwrap_or(format!("rustc{}", EXE_SUFFIX).into()); Command::new(rustc).arg("--version").status().unwrap(); } _ => panic!("bad mock proxy commandline"), diff --git a/src/rustup/toolchain.rs b/src/rustup/toolchain.rs index 3cb1e5610f..7ec5f05592 100644 --- a/src/rustup/toolchain.rs +++ b/src/rustup/toolchain.rs @@ -373,38 +373,26 @@ impl<'a> Toolchain<'a> { return Err(ErrorKind::ToolchainNotInstalled(primary_toolchain.name.to_owned()).into()); } - let src_file = self.path.join("bin").join(format!("cargo{}", EXE_SUFFIX)); - - // MAJOR HACKS: Copy cargo.exe to its own directory on windows before - // running it. This is so that the fallback cargo, when it in turn runs - // rustc.exe, will run the rustc.exe out of the PATH environment - // variable, _not_ the rustc.exe sitting in the same directory as the - // fallback. See the `fallback_cargo_calls_correct_rustc` testcase and - // PR 812. + let exe_path = self.path.join("bin").join(format!("cargo{}", EXE_SUFFIX)); + + let mut cmd = Command::new(exe_path); + self.set_env(&mut cmd); + cmd.env("RUSTUP_TOOLCHAIN", &primary_toolchain.name); + + // Set the RUSTC environment variable to the primary toolchain binary + // on Windows. This is so that the fallback cargo will run that executable + // and _not_ the rustc.exe sitting in the same directory. See the + // `fallback_cargo_calls_correct_rustc` testcase. // // On Windows, spawning a process will search the running application's // directory for the exe to spawn before searching PATH, and we don't want // it to do that, because cargo's directory contains the _wrong_ rustc. See // the documantation for the lpCommandLine argument of CreateProcess. - let exe_path = if cfg!(windows) { - use std::fs; - let fallback_dir = self.cfg.rustup_dir.join("fallback"); - fs::create_dir_all(&fallback_dir) - .chain_err(|| "unable to create dir to hold fallback exe")?; - let fallback_file = fallback_dir.join("cargo.exe"); - if fallback_file.exists() { - fs::remove_file(&fallback_file) - .chain_err(|| "unable to unlink old fallback exe")?; + if cfg!(windows) { + if env::var_os("RUSTC").is_none() { + cmd.env("RUSTC", primary_toolchain.binary_file("rustc")); } - fs::hard_link(&src_file, &fallback_file) - .chain_err(|| "unable to hard link fallback exe")?; - fallback_file - } else { - src_file - }; - let mut cmd = Command::new(exe_path); - self.set_env(&mut cmd); - cmd.env("RUSTUP_TOOLCHAIN", &primary_toolchain.name); + } Ok(cmd) }