Recursive tool invocations should invoke the proxy, not the tool directly

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

Author: brson

src/rustup-mock/src/mock_bin_src.rs

@@ -1,3 +1,4 @@
+use std::process::Command;
 use std::io::{self, BufWriter, Write};
 
 fn main() {
@@ -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");
     }

src/rustup/toolchain.rs

@@ -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> {

tests/cli-rustup.rs

@@ -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,
@@ -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| {