Merge pull request #705 from meqif/check-for-unexpected-cargo-rustc-before-install

Check for unexpected cargo/rustc before install

Author: brson

src/rustup-cli/self_update.rs

@@ -36,7 +36,7 @@ use rustup_dist::dist;
 use rustup_utils::utils;
 use std::env;
 use std::env::consts::EXE_SUFFIX;
-use std::path::{Path, PathBuf};
+use std::path::{Path, PathBuf, Component};
 use std::process::{self, Command};
 use std::fs;
 use tempdir::TempDir;
@@ -215,11 +215,12 @@ fn canonical_cargo_home() -> Result<String> {
 
 /// Installing is a simple matter of coping the running binary to
 /// `CARGO_HOME`/bin, hardlinking the various Rust tools to it,
-/// and and adding `CARGO_HOME`/bin to PATH.
+/// and adding `CARGO_HOME`/bin to PATH.
 pub fn install(no_prompt: bool, verbose: bool,
                mut opts: InstallOpts) -> Result<()> {
 
     try!(do_pre_install_sanity_checks());
+    try!(check_existence_of_rustc_or_cargo_in_path(no_prompt));
     try!(do_anti_sudo_check(no_prompt));
 
     if !try!(do_msvc_check(&opts)) {
@@ -331,8 +332,49 @@ pub fn install(no_prompt: bool, verbose: bool,
     Ok(())
 }
 
-fn do_pre_install_sanity_checks() -> Result<()> {
+fn rustc_or_cargo_exists_in_path() -> Result<()> {
+    // Ignore rustc and cargo if present in $HOME/.cargo/bin or a few other directories
+    fn ignore_paths(path: &PathBuf) -> bool {
+        !path.components().any(|c| c == Component::Normal(".cargo".as_ref())) &&
+            !path.components().any(|c| c == Component::Normal(".multirust".as_ref()))
+    }
+
+    if let Some(paths) = env::var_os("PATH") {
+        let paths = env::split_paths(&paths).filter(ignore_paths);
+
+        for path in paths {
+            let rustc = path.join(format!("rustc{}", EXE_SUFFIX));
+            let cargo = path.join(format!("cargo{}", EXE_SUFFIX));
+
+            if rustc.exists() || cargo.exists() {
+                return Err(path.to_str().unwrap().into());
+            }
+        }
+    }
+    Ok(())
+}
+
+fn check_existence_of_rustc_or_cargo_in_path(no_prompt: bool) -> Result<()> {
+    // Only the test runner should set this
+    let skip_check = env::var_os("RUSTUP_INIT_SKIP_PATH_CHECK");
 
+    // Ignore this check if called with no prompt (-y) or if the environment variable is set
+    if no_prompt || skip_check == Some("yes".into()) {
+        return Ok(());
+    }
+
+    if let Err(path) = rustc_or_cargo_exists_in_path() {
+        err!("it looks like you have an existing installation of Rust at:");
+        err!("{}", path);
+        err!("rustup cannot be installed alongside Rust. Please uninstall first");
+        err!("if this is what you want, restart the installation with `-y'");
+        Err("cannot install while Rust is installed".into())
+    } else {
+        Ok(())
+    }
+}
+
+fn do_pre_install_sanity_checks() -> Result<()> {
     let multirust_manifest_path
         = PathBuf::from("/usr/local/lib/rustlib/manifest-multirust");
     let rustc_manifest_path
@@ -652,7 +694,7 @@ pub fn uninstall(no_prompt: bool) -> Result<()> {
                 .chain_err(|| ErrorKind::WindowsUninstallMadness));
         process::exit(0);
     }
-    
+
     let ref cargo_home = try!(utils::cargo_home());
 
     if !cargo_home.join(&format!("bin/rustup{}", EXE_SUFFIX)).exists() {

src/rustup-mock/src/clitools.rs

@@ -313,6 +313,10 @@ pub fn env(config: &Config, cmd: &mut Command) {
 
     // Skip the MSVC warning check since it's environment dependent
     cmd.env("RUSTUP_INIT_SKIP_MSVC_CHECK", "yes");
+
+    // The test environment may interfere with checking the PATH for the existence of rustc or
+    // cargo, so we disable that check globally
+    cmd.env("RUSTUP_INIT_SKIP_PATH_CHECK", "yes");
 }
 
 pub fn run(config: &Config, name: &str, args: &[&str], env: &[(&str, &str)]) -> SanitizedOutput {

tests/cli-misc.rs

@@ -5,17 +5,22 @@ extern crate rustup_dist;
 extern crate rustup_utils;
 extern crate rustup_mock;
 extern crate time;
-use rustup_mock::clitools::{self, Config, Scenario,
+extern crate tempdir;
+
+use rustup_mock::clitools::{self, Config, Scenario, SanitizedOutput,
                                expect_stdout_ok, expect_stderr_ok,
                                expect_ok, expect_err, expect_timeout_ok,
                                run, this_host_triple};
 use rustup_utils::{raw, utils};
 
-use time::Duration;
-use std::ops::Sub;
+use std::io::Write;
 use std::ops::Add;
+use std::ops::Sub;
+use std::process::Stdio;
 use std::time::Duration as StdDuration;
 use std::env::consts::EXE_SUFFIX;
+use tempdir::TempDir;
+use time::Duration;
 
 macro_rules! for_host { ($s: expr) => (&format!($s, this_host_triple())) }
 
@@ -429,3 +434,52 @@ fn rls_does_not_exist_in_toolchain() {
                             this_host_triple(), EXE_SUFFIX));
     });
 }
+
+#[test]
+fn install_stops_if_rustc_exists() {
+    let temp_dir = TempDir::new("fakebin").unwrap();
+    // Create fake executable
+    let ref fake_exe = temp_dir.path().join(&format!("{}{}", "rustc", EXE_SUFFIX));
+    raw::append_file(fake_exe, "").unwrap();
+    let temp_dir_path = temp_dir.path().to_str().unwrap();
+
+    setup(&|config| {
+        let out = run(config, "rustup-init", &[],
+                      &[("RUSTUP_INIT_SKIP_PATH_CHECK", "no"), ("PATH", &temp_dir_path)]);
+        assert!(!out.ok);
+        assert!(out.stderr.contains("it looks like you have an existing installation of Rust at:"));
+        assert!(out.stderr.contains("if this is what you want, restart the installation with `-y'"));
+    });
+}
+
+#[test]
+fn install_stops_if_cargo_exists() {
+    let temp_dir = TempDir::new("fakebin").unwrap();
+    // Create fake executable
+    let ref fake_exe = temp_dir.path().join(&format!("{}{}", "cargo", EXE_SUFFIX));
+    raw::append_file(fake_exe, "").unwrap();
+    let temp_dir_path = temp_dir.path().to_str().unwrap();
+
+    setup(&|config| {
+        let out = run(config, "rustup-init", &[],
+                      &[("RUSTUP_INIT_SKIP_PATH_CHECK", "no"), ("PATH", &temp_dir_path)]);
+        assert!(!out.ok);
+        assert!(out.stderr.contains("it looks like you have an existing installation of Rust at:"));
+        assert!(out.stderr.contains("if this is what you want, restart the installation with `-y'"));
+    });
+}
+
+#[test]
+fn with_no_prompt_install_succeeds_if_rustc_exists() {
+    let temp_dir = TempDir::new("fakebin").unwrap();
+    // Create fake executable
+    let ref fake_exe = temp_dir.path().join(&format!("{}{}", "rustc", EXE_SUFFIX));
+    raw::append_file(fake_exe, "").unwrap();
+    let temp_dir_path = temp_dir.path().to_str().unwrap();
+
+    setup(&|config| {
+        let out = run(config, "rustup-init", &["-y"],
+                      &[("RUSTUP_INIT_SKIP_PATH_CHECK", "no"), ("PATH", &temp_dir_path)]);
+        assert!(out.ok);
+    });
+}