-
Notifications
You must be signed in to change notification settings - Fork 1k
Allow installing alongside existing Rust (with warning) #2214
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
703b405
62f32f7
680b2ed
5bc1c8c
76164d2
8095f81
ac5105f
1e0a497
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,9 +235,16 @@ fn canonical_cargo_home() -> Result<String> { | |
| /// `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()?; | ||
| 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)?; | ||
|
|
||
|
|
@@ -378,25 +385,25 @@ 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()) { | ||
kinnison marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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 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."); | ||
| Err("cannot install while Rust is installed".into()) | ||
| } else { | ||
| Ok(()) | ||
| 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)?; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Errata: this is not ignorable if the location of the existing rust is in the CARGO_BIN directory that rustup's proxies will be written to. We're making this easier to run into, so perhaps we should check for that at this point.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rbtcollins Should it be possible to skip it in any way (e.g. environment variable) or not?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently the code will give a warning later about "tool X is already installed", and not touch those binaries. This would mean that rustup isn't actually installed. I don't think it makes sense to allow that to be bypassed, but would like @kinnison 's input. This might be something to do as a separate PR for instance.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The cases this PR covers are to do with
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The concern I have is that this is escalating it from a read-the-docs situation to a y/n prompt, so perhaps a lot easier to perform targeted foot removal.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Possibly so. I shall check the feel of that in my testing which I'm about to undertake.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay so the issue here is that if the |
||
| } | ||
| 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 +419,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 +429,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(()) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.