From 703b405c85877e719ff48132435147dfb9fb2dfc Mon Sep 17 00:00:00 2001 From: Solomon Ucko Date: Thu, 23 Jan 2020 11:00:09 -0500 Subject: [PATCH 1/8] Allow installing alongside existing Rust (with warning) See https://github.com/rust-lang/rustup/issues/953 --- src/cli/self_update.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index 609cfdbd8a..ed60df8fa4 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -412,7 +412,7 @@ fn do_pre_install_sanity_checks() -> Result<()> { "run `{}` as root to uninstall Rust", uninstaller_path.display() ); - return Err("cannot install while Rust is installed".into()); + // return Err("cannot install while Rust is installed".into()); } if rustup_sh_exists { From 62f32f7440acaf86e43154670248f2fbc4b22d0b Mon Sep 17 00:00:00 2001 From: Solly Date: Mon, 27 Jan 2020 22:09:05 -0500 Subject: [PATCH 2/8] Made some errors non-fatal. --- src/cli/common.rs | 14 ++++++++++++++ src/cli/self_update.rs | 22 ++++++++++++---------- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/cli/common.rs b/src/cli/common.rs index 355d76d12d..bc93025a57 100644 --- a/src/cli/common.rs +++ b/src/cli/common.rs @@ -522,3 +522,17 @@ pub fn report_error(e: &Error) { } } } + +pub fn ignorable_error(error: crate::errors::Error, no_prompt: bool) -> Result<()> { + report_error(&error); + if no_prompt { + warn!("continuing (because the -y flag is set and the error is ignorable)"); + Ok(()) + } else { + if confirm("\nContinue? (y/N)", false).unwrap_or(false) { + Ok(()) + } else { + Err(error) + } + } +} diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index ed60df8fa4..cc800c0d5e 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -30,7 +30,7 @@ //! Deleting the running binary during uninstall is tricky //! and racy on Windows. -use crate::common::{self, Confirm}; +use crate::common::{self, ignorable_error, Confirm}; use crate::errors::*; use crate::markdown::md; use crate::term2; @@ -235,7 +235,7 @@ fn canonical_cargo_home() -> Result { /// `CARGO_HOME`/bin, hard-linking the various Rust tools to it, /// and adding `CARGO_HOME`/bin to PATH. pub fn install(no_prompt: bool, verbose: bool, quiet: bool, mut opts: InstallOpts) -> Result<()> { - do_pre_install_sanity_checks()?; + do_pre_install_sanity_checks(no_prompt)?; do_pre_install_options_sanity_checks(&opts)?; check_existence_of_rustc_or_cargo_in_path(no_prompt)?; #[cfg(unix)] @@ -378,8 +378,8 @@ 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()) { + // Skip this if the environment variable is set + if skip_check == Some("yes".into()) { return Ok(()); } @@ -390,13 +390,12 @@ fn check_existence_of_rustc_or_cargo_in_path(no_prompt: bool) -> Result<()> { err!("Otherwise you may have confusion unless you are careful with your PATH"); err!("If you are sure that you want both rustup and your already installed Rust"); err!("then please restart the installation and pass `-y' to bypass this check."); - Err("cannot install while Rust is installed".into()) - } else { - Ok(()) + ignorable_error("cannot install while Rust is installed".into(), no_prompt)?; } + Ok(()) } -fn do_pre_install_sanity_checks() -> Result<()> { +fn do_pre_install_sanity_checks(no_prompt: bool) -> Result<()> { let rustc_manifest_path = PathBuf::from("/usr/local/lib/rustlib/manifest-rustc"); let uninstaller_path = PathBuf::from("/usr/local/lib/rustlib/uninstall.sh"); let rustup_sh_path = utils::home_dir().unwrap().join(".rustup"); @@ -412,7 +411,7 @@ fn do_pre_install_sanity_checks() -> Result<()> { "run `{}` as root to uninstall Rust", uninstaller_path.display() ); - // return Err("cannot install while Rust is installed".into()); + ignorable_error("cannot install while Rust is installed".into(), no_prompt)?; } if rustup_sh_exists { @@ -422,7 +421,10 @@ fn do_pre_install_sanity_checks() -> Result<()> { warn!("or, if you already have rustup installed, you can run"); warn!("`rustup self update` and `rustup toolchain list` to upgrade"); warn!("your directory structure"); - return Err("cannot install while rustup.sh is installed".into()); + ignorable_error( + "cannot install while rustup.sh is installed".into(), + no_prompt, + )?; } Ok(()) From 680b2ed90a6f7f4e4323d503c436aff344a1dfef Mon Sep 17 00:00:00 2001 From: Solly Date: Tue, 28 Jan 2020 20:34:31 -0500 Subject: [PATCH 3/8] Fixed test case. --- tests/cli-self-upd.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cli-self-upd.rs b/tests/cli-self-upd.rs index 10f4079d8e..4eb96e9c01 100644 --- a/tests/cli-self-upd.rs +++ b/tests/cli-self-upd.rs @@ -1166,7 +1166,7 @@ fn install_but_rustup_sh_is_installed() { fs::create_dir_all(&rustup_dir).unwrap(); let version_file = rustup_dir.join("rustup-version"); raw::write_file(&version_file, "").unwrap(); - expect_err( + expect_stderr_ok( config, &["rustup-init", "-y"], "cannot install while rustup.sh is installed", From 5bc1c8cffe6db0111493db61d15e9e8acdbdcf95 Mon Sep 17 00:00:00 2001 From: Solly Date: Tue, 28 Jan 2020 20:58:16 -0500 Subject: [PATCH 4/8] Added environment variable `RUSTUP_INIT_SKIP_CHECKING_EXISTENCE`. --- src/cli/self_update.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index cc800c0d5e..8516b3db7b 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -235,9 +235,16 @@ fn canonical_cargo_home() -> Result { /// `CARGO_HOME`/bin, hard-linking the various Rust tools to it, /// and adding `CARGO_HOME`/bin to PATH. pub fn install(no_prompt: bool, verbose: bool, quiet: bool, mut opts: InstallOpts) -> Result<()> { - do_pre_install_sanity_checks(no_prompt)?; + if !env::var_os("RUSTUP_INIT_SKIP_EXISTENCE_CHECKS").map_or(false, |s| s == "yes") { + do_pre_install_sanity_checks(no_prompt)?; + } + do_pre_install_options_sanity_checks(&opts)?; - check_existence_of_rustc_or_cargo_in_path(no_prompt)?; + + if !env::var_os("RUSTUP_INIT_SKIP_EXISTENCE_CHECKS").map_or(false, |s| s == "yes") { + check_existence_of_rustc_or_cargo_in_path(no_prompt)?; + } + #[cfg(unix)] do_anti_sudo_check(no_prompt)?; From 76164d21b013d612d213491f6a07e02591adcc73 Mon Sep 17 00:00:00 2001 From: Solly Date: Wed, 29 Jan 2020 21:51:58 -0500 Subject: [PATCH 5/8] Added tests for ignorable errors. --- tests/cli-inst-interactive.rs | 83 +++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/tests/cli-inst-interactive.rs b/tests/cli-inst-interactive.rs index 93c8501943..5a1eb54297 100644 --- a/tests/cli-inst-interactive.rs +++ b/tests/cli-inst-interactive.rs @@ -8,6 +8,8 @@ use crate::mock::clitools::{ }; use crate::mock::{get_path, restore_path}; use lazy_static::lazy_static; +use rustup::utils::raw; +use std::fs; use std::io::Write; use std::process::Stdio; use std::sync::Mutex; @@ -266,3 +268,84 @@ fn test_warn_if_complete_profile_is_used() { ); }); } + +fn create_rustup_sh_metadata(config: &Config) { + let rustup_dir = config.homedir.join(".rustup"); + fs::create_dir_all(&rustup_dir).unwrap(); + let version_file = rustup_dir.join("rustup-version"); + raw::write_file(&version_file, "").unwrap(); +} + +#[test] +fn test_prompt_fail_if_rustup_sh_already_installed_reply_nothing() { + setup(&|config| { + create_rustup_sh_metadata(&config); + let out = run_input(config, &["rustup-init"], "\n"); + assert!(!out.ok); + assert!(out + .stderr + .contains("warning: it looks like you have existing rustup.sh metadata")); + assert!(out + .stderr + .contains("error: cannot install while rustup.sh is installed")); + assert!(out.stdout.contains("Continue? (y/N)")); + }) +} + +#[test] +fn test_prompt_fail_if_rustup_sh_already_installed_reply_no() { + setup(&|config| { + create_rustup_sh_metadata(&config); + let out = run_input(config, &["rustup-init"], "no\n"); + assert!(!out.ok); + assert!(out + .stderr + .contains("warning: it looks like you have existing rustup.sh metadata")); + assert!(out + .stderr + .contains("error: cannot install while rustup.sh is installed")); + assert!(out.stdout.contains("Continue? (y/N)")); + }) +} + +#[test] +fn test_prompt_succeed_if_rustup_sh_already_installed_reply_yes() { + setup(&|config| { + create_rustup_sh_metadata(&config); + let out = run_input(config, &["rustup-init"], "yes\n\n\n"); + assert!(out.ok); + assert!(out + .stderr + .contains("warning: it looks like you have existing rustup.sh metadata")); + assert!(out + .stderr + .contains("error: cannot install while rustup.sh is installed")); + assert!(out.stdout.contains("Continue? (y/N)")); + assert!(!out.stdout.contains( + "warning: continuing (because the -y flag is set and the error is ignorable)" + )) + }) +} + +#[test] +fn test_warn_succeed_if_rustup_sh_already_installed_y_flag() { + setup(&|config| { + create_rustup_sh_metadata(&config); + let out = run_input(config, &["rustup-init", "-y"], ""); + assert!(out.ok); + assert!(out + .stderr + .contains("warning: it looks like you have existing rustup.sh metadata")); + assert!(out + .stderr + .contains("error: cannot install while rustup.sh is installed")); + assert!(out.stderr.contains( + "warning: continuing (because the -y flag is set and the error is ignorable)" + )); + assert!(!out.stdout.contains("Continue? (y/N)")); + }) +} + +#[ignore] // Can't test environment variable. +#[test] +fn test_succeed_if_rustup_sh_already_installed_env_var_set() {} From 8095f817f960b30a05956ff78fd8e87fcd853e6a Mon Sep 17 00:00:00 2001 From: Solly Date: Sat, 1 Feb 2020 19:42:46 -0500 Subject: [PATCH 6/8] Created `run_input_with_env`. --- tests/cli-inst-interactive.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/cli-inst-interactive.rs b/tests/cli-inst-interactive.rs index 5a1eb54297..9e8bbaeaf0 100644 --- a/tests/cli-inst-interactive.rs +++ b/tests/cli-inst-interactive.rs @@ -41,9 +41,22 @@ pub fn setup(f: &dyn Fn(&Config)) { } fn run_input(config: &Config, args: &[&str], input: &str) -> SanitizedOutput { + run_input_with_env(config, args, input, &[]) +} + +fn run_input_with_env( + config: &Config, + args: &[&str], + input: &str, + env: &[(&str, &str)], +) -> SanitizedOutput { let mut cmd = clitools::cmd(config, args[0], &args[1..]); clitools::env(config, &mut cmd); + for (key, value) in env.iter() { + cmd.env(key, value); + } + cmd.stdin(Stdio::piped()); cmd.stdout(Stdio::piped()); cmd.stderr(Stdio::piped()); @@ -345,7 +358,3 @@ fn test_warn_succeed_if_rustup_sh_already_installed_y_flag() { assert!(!out.stdout.contains("Continue? (y/N)")); }) } - -#[ignore] // Can't test environment variable. -#[test] -fn test_succeed_if_rustup_sh_already_installed_env_var_set() {} From ac5105f88ddda6c177e29c76eba47f7a9f2187c1 Mon Sep 17 00:00:00 2001 From: Solly Date: Thu, 30 Jan 2020 20:47:50 -0500 Subject: [PATCH 7/8] Added test for `RUSTUP_INIT_SKIP_CHECKING_EXISTENCE`. --- tests/cli-inst-interactive.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/cli-inst-interactive.rs b/tests/cli-inst-interactive.rs index 9e8bbaeaf0..53841283c1 100644 --- a/tests/cli-inst-interactive.rs +++ b/tests/cli-inst-interactive.rs @@ -358,3 +358,27 @@ fn test_warn_succeed_if_rustup_sh_already_installed_y_flag() { assert!(!out.stdout.contains("Continue? (y/N)")); }) } + +#[test] +fn test_succeed_if_rustup_sh_already_installed_env_var_set() { + setup(&|config| { + create_rustup_sh_metadata(&config); + let out = run_input_with_env( + config, + &["rustup-init", "-y"], + "", + &[("RUSTUP_INIT_SKIP_EXISTENCE_CHECKS", "yes")], + ); + assert!(out.ok); + assert!(!out + .stderr + .contains("warning: it looks like you have existing rustup.sh metadata")); + assert!(!out + .stderr + .contains("error: cannot install while rustup.sh is installed")); + assert!(!out.stderr.contains( + "warning: continuing (because the -y flag is set and the error is ignorable)" + )); + assert!(!out.stdout.contains("Continue? (y/N)")); + }) +} From 1e0a4974b0dd64f3cae4856cfd71449a91809039 Mon Sep 17 00:00:00 2001 From: Solly Date: Tue, 18 Feb 2020 21:59:38 -0500 Subject: [PATCH 8/8] Improved warning/error message for `check_existence_of_rustc_or_cargo_in_path`. --- src/cli/self_update.rs | 13 +++++++------ tests/cli-misc.rs | 4 ++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index 8516b3db7b..3974f3376b 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -391,12 +391,13 @@ fn check_existence_of_rustc_or_cargo_in_path(no_prompt: bool) -> Result<()> { } 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 should not be installed alongside Rust. Please uninstall your existing Rust first."); - err!("Otherwise you may have confusion unless you are careful with your PATH"); - err!("If you are sure that you want both rustup and your already installed Rust"); - err!("then please restart the installation and pass `-y' to bypass this check."); + warn!("it looks like you have an existing installation of Rust at:"); + warn!("{}", path); + warn!("rustup should not be installed alongside Rust. Please uninstall your existing Rust first."); + warn!("Otherwise you may have confusion unless you are careful with your PATH"); + warn!("If you are sure that you want both rustup and your already installed Rust"); + warn!("then please reply `y' or `yes' or set RUSTUP_INIT_SKIP_PATH_CHECK to yes"); + warn!("or pass `-y' to ignore all ignorable checks."); ignorable_error("cannot install while Rust is installed".into(), no_prompt)?; } Ok(()) diff --git a/tests/cli-misc.rs b/tests/cli-misc.rs index ba00332f22..eb53771f52 100644 --- a/tests/cli-misc.rs +++ b/tests/cli-misc.rs @@ -620,7 +620,7 @@ fn install_stops_if_rustc_exists() { .contains("it looks like you have an existing installation of Rust at:")); assert!(out .stderr - .contains("restart the installation and pass `-y'")); + .contains("If you are sure that you want both rustup and your already installed Rust")); }); } @@ -652,7 +652,7 @@ fn install_stops_if_cargo_exists() { .contains("it looks like you have an existing installation of Rust at:")); assert!(out .stderr - .contains("restart the installation and pass `-y'")); + .contains("If you are sure that you want both rustup and your already installed Rust")); }); }