Skip to content

Commit

Permalink
Recursive tool invocations should invoke the proxy, not the tool dire…
Browse files Browse the repository at this point in the history
…ctly

The way the proxy was setting up the PATH variable contained two bugs:
first, that it allowed the toolchain path to precede the value of CARGO_HOME/bin;
but second that it didn't add CARGO_HOME/bin to the path at all. The result
was that when e.g. cargo execs rustc, it was directly execing the toolchain
rustc.

Now it execs the proxy, assuming that CARGO_HOME/bin is set up correctly,
and guaranteeing not to run the toolchain tool directly.

Fixes #809
  • Loading branch information
brson committed Nov 17, 2016
1 parent e3aa6ec commit c8dec07
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 5 deletions.
6 changes: 6 additions & 0 deletions src/rustup-mock/src/mock_bin_src.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::process::Command;
use std::io::{self, BufWriter, Write};

fn main() {
Expand All @@ -13,6 +14,11 @@ fn main() {
for _ in 0 .. 10000 {
buf.write_all(b"error: a value named `fail` has already been defined in this module [E0428]\n").unwrap();
}
} else if args.get(1) == Some(&"--call-rustc".to_string()) {
// 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
Command::new("rustc").arg("--version").status().unwrap();
} else {
panic!("bad mock proxy commandline");
}
Expand Down
10 changes: 5 additions & 5 deletions src/rustup/toolchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,13 +321,13 @@ impl<'a> Toolchain<'a> {
}
env_var::prepend_path(sysenv::LOADER_PATH, &new_path, cmd);

// Prepend first cargo_home, then toolchain/bin to the PATH
let mut path_to_prepend = PathBuf::from("");
// Prepend CARGO_HOME/bin to the PATH variable so that we're sure to run
// cargo/rustc via the proxy bins. There is no fallback case for if the
// proxy bins don't exist. We'll just be running whatever happens to
// be on the PATH.
if let Ok(cargo_home) = utils::cargo_home() {
path_to_prepend.push(cargo_home.join("bin"));
env_var::prepend_path("PATH", &cargo_home.join("bin"), cmd);
}
path_to_prepend.push(self.path.join("bin"));
env_var::prepend_path("PATH", path_to_prepend.as_path(), cmd);
}

pub fn doc_path(&self, relative: &str) -> Result<PathBuf> {
Expand Down
35 changes: 35 additions & 0 deletions tests/cli-rustup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ extern crate rustup_utils;
extern crate rustup_mock;
extern crate tempdir;

use std::fs;
use std::env::consts::EXE_SUFFIX;
use rustup_mock::clitools::{self, Config, Scenario,
expect_ok, expect_ok_ex,
expect_stdout_ok,
Expand Down Expand Up @@ -269,6 +271,39 @@ fn link() {
});
}

// Issue #809. When we call the fallback cargo, when it in turn invokes
// "rustc", that rustc should actually be the rustup proxy, not the toolchain rustc.
// That way the proxy can pick the correct toolchain.
#[test]
fn fallback_cargo_calls_correct_rustc() {
setup(&|config| {
// Hm, this is the _only_ test that assumes that toolchain proxies
// exist in CARGO_HOME. Adding that proxy here.
let ref rustup_path = config.exedir.join(format!("rustup{}", EXE_SUFFIX));
let ref cargo_bin_path = config.cargodir.join("bin");
fs::create_dir_all(cargo_bin_path).unwrap();
let ref rustc_path = cargo_bin_path.join(format!("rustc{}", EXE_SUFFIX));
fs::hard_link(rustup_path, rustc_path).unwrap();

// Install a custom toolchain and a nightly toolchain for the cargo fallback
let path = config.customdir.join("custom-1");
let path = path.to_string_lossy();
expect_ok(config, &["rustup", "toolchain", "link", "custom",
&path]);
expect_ok(config, &["rustup", "default", "custom"]);
expect_ok(config, &["rustup", "update", "nightly"]);
expect_stdout_ok(config, &["rustc", "--version"],
"hash-c-1");

// Here --call-rustc tells the mock cargo bin to exec `rustc --version`.
// We should be ultimately calling the custom rustc, according to the
// RUSTUP_TOOLCHAIN variable set by the original "cargo" proxy, and
// interpreted by the nested "rustc" proxy.
expect_stdout_ok(config, &["cargo", "--call-rustc"],
"hash-c-1");
});
}

#[test]
fn show_toolchain_none() {
setup(&|config| {
Expand Down

0 comments on commit c8dec07

Please sign in to comment.