diff --git a/src/rustup-cli/log.rs b/src/rustup-cli/log.rs index ebfbd17636..016f9c8ebe 100644 --- a/src/rustup-cli/log.rs +++ b/src/rustup-cli/log.rs @@ -16,6 +16,10 @@ macro_rules! verbose { ( $ ( $ arg : tt ) * ) => ( $crate::log::verbose_fmt ( format_args ! ( $ ( $ arg ) * ) ) ) } +macro_rules! debug { + ( $ ( $ arg : tt ) * ) => ( $crate::log::debug_fmt ( format_args ! ( $ ( $ arg ) * ) ) ) +} + pub fn warn_fmt(args: fmt::Arguments) { let mut t = term2::stderr(); let _ = t.fg(term2::color::BRIGHT_YELLOW); @@ -54,3 +58,15 @@ pub fn verbose_fmt(args: fmt::Arguments) { let _ = t.write_fmt(args); let _ = write!(t, "\n"); } + +pub fn debug_fmt(args: fmt::Arguments) { + if std::env::var("RUSTUP_DEBUG").is_ok() { + let mut t = term2::stderr(); + let _ = t.fg(term2::color::BRIGHT_BLUE); + let _ = t.attr(term2::Attr::Bold); + let _ = write!(t, "verbose: "); + let _ = t.reset(); + let _ = t.write_fmt(args); + let _ = write!(t, "\n"); + } +} diff --git a/src/rustup-cli/self_update.rs b/src/rustup-cli/self_update.rs index 9b45da7b2e..a630b7a248 100644 --- a/src/rustup-cli/self_update.rs +++ b/src/rustup-cli/self_update.rs @@ -223,6 +223,7 @@ fn canonical_cargo_home() -> Result { /// and adding `CARGO_HOME`/bin to PATH. pub fn install(no_prompt: bool, verbose: bool, mut opts: InstallOpts) -> Result<()> { do_pre_install_sanity_checks()?; + do_pre_install_options_sanity_checks(&opts)?; check_existence_of_rustc_or_cargo_in_path(no_prompt)?; do_anti_sudo_check(no_prompt)?; @@ -454,7 +455,7 @@ fn do_pre_install_sanity_checks() -> Result<()> { "delete `{}` to remove rustup.sh", rustup_sh_path.expect("").display() ); - warn!("or, if you already rustup installed, you can run"); + 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()); @@ -463,6 +464,35 @@ fn do_pre_install_sanity_checks() -> Result<()> { Ok(()) } +fn do_pre_install_options_sanity_checks(opts: &InstallOpts) -> Result<()> { + // Verify that the installation options are vaguely sane + (|| { + let host_triple = dist::TargetTriple::from_str(&opts.default_host_triple); + let toolchain_to_use = if opts.default_toolchain == "none" { + "stable" + } else { + &opts.default_toolchain + }; + let partial_channel = dist::PartialToolchainDesc::from_str(toolchain_to_use)?; + let resolved = partial_channel.resolve(&host_triple)?.to_string(); + debug!( + "Successfully resolved installation toolchain as: {}", + resolved + ); + Ok(()) + })() + .map_err(|e: Box| { + format!( + "Pre-checks for host and toolchain failed: {}\n\ + If you are unsure of suitable values, the 'stable' toolchain is the default.\n\ + Valid host triples look something like: {}", + e, + dist::TargetTriple::from_host_or_build() + ) + })?; + Ok(()) +} + // If the user is trying to install with sudo, on some systems this will // result in writing root-owned files to the user's home directory, because // sudo is configured not to change $HOME. Don't let that bogosity happen. diff --git a/src/rustup-dist/src/dist.rs b/src/rustup-dist/src/dist.rs index a4415b37eb..5f1c860401 100644 --- a/src/rustup-dist/src/dist.rs +++ b/src/rustup-dist/src/dist.rs @@ -291,11 +291,25 @@ impl PartialToolchainDesc { } } - pub fn resolve(self, host: &TargetTriple) -> ToolchainDesc { - let host = PartialTargetTriple::from_str(&host.0) - .expect("host triple couldn't be converted to partial triple"); - let host_arch = host.arch.expect(""); - let host_os = host.os.expect(""); + pub fn resolve(self, input_host: &TargetTriple) -> Result { + let host = PartialTargetTriple::from_str(&input_host.0).ok_or_else(|| { + format!( + "Provided host '{}' couldn't be converted to partial triple", + input_host.0 + ) + })?; + let host_arch = host.arch.ok_or_else(|| { + format!( + "Provided host '{}' did not specify a CPU architecture", + input_host.0 + ) + })?; + let host_os = host.os.ok_or_else(|| { + format!( + "Provided host '{}' did not specify an operating system", + input_host.0 + ) + })?; let host_env = host.env; // If OS was specified, don't default to host environment, even if the OS matches @@ -314,11 +328,11 @@ impl PartialToolchainDesc { format!("{}-{}", arch, os) }; - ToolchainDesc { + Ok(ToolchainDesc { channel: self.channel, date: self.date, target: TargetTriple(trip), - } + }) } pub fn has_triple(&self) -> bool { diff --git a/src/rustup/config.rs b/src/rustup/config.rs index d92b2259ea..414a4881d7 100644 --- a/src/rustup/config.rs +++ b/src/rustup/config.rs @@ -99,7 +99,7 @@ impl Cfg { ); let dist_root = dist_root_server.clone() + "/dist"; - Ok(Cfg { + let cfg = Cfg { rustup_dir: rustup_dir, settings_file: settings_file, toolchains_dir: toolchains_dir, @@ -111,7 +111,15 @@ impl Cfg { env_override: env_override, dist_root_url: dist_root, dist_root_server: dist_root_server, - }) + }; + + // Run some basic checks against the constructed configuration + // For now, that means simply checking that 'stable' can resolve + // for the current configuration. + cfg.resolve_toolchain("stable") + .map_err(|e| format!("Unable parse configuration: {}", e))?; + + Ok(cfg) } pub fn set_default(&self, toolchain: &str) -> Result<()> { @@ -470,9 +478,11 @@ impl Cfg { } pub fn set_default_host_triple(&self, host_triple: &str) -> Result<()> { - if dist::PartialTargetTriple::from_str(host_triple).is_none() { - return Err("Invalid host triple".into()); - } + // Ensure that the provided host_triple is capable of resolving + // against the 'stable' toolchain. This provides early errors + // if the supplied triple is insufficient / bad. + dist::PartialToolchainDesc::from_str("stable")? + .resolve(&dist::TargetTriple::from_str(host_triple))?; self.settings_file.with_mut(|s| { s.default_host_triple = Some(host_triple.to_owned()); Ok(()) @@ -493,7 +503,7 @@ impl Cfg { pub fn resolve_toolchain(&self, name: &str) -> Result { if let Ok(desc) = dist::PartialToolchainDesc::from_str(name) { let host = self.get_default_host_triple()?; - Ok(desc.resolve(&host).to_string()) + Ok(desc.resolve(&host)?.to_string()) } else { Ok(name.to_owned()) } diff --git a/tests/cli-rustup.rs b/tests/cli-rustup.rs index 266baae328..8a4aafd762 100644 --- a/tests/cli-rustup.rs +++ b/tests/cli-rustup.rs @@ -900,7 +900,19 @@ fn set_default_host_invalid_triple() { expect_err( config, &["rustup", "set", "default-host", "foo"], - "Invalid host triple", + "error: Provided host 'foo' couldn't be converted to partial triple", + ); + }); +} + +// #745 +#[test] +fn set_default_host_invalid_triple_valid_partial() { + setup(&|config| { + expect_err( + config, + &["rustup", "set", "default-host", "x86_64-msvc"], + "error: Provided host 'x86_64-msvc' did not specify an operating system", ); }); }