Skip to content

Commit

Permalink
UX: Report non-installable toolchains
Browse files Browse the repository at this point in the history
If you pick a default toolchain which is not installable, or if you
try to `rustup toolchain install` something non-installable then we
should report it as such and exit non-zero so scripting can fail.

Signed-off-by: Daniel Silverstone <dsilvers@digital-scurf.org>
  • Loading branch information
kinnison committed Jan 12, 2021
1 parent dc721d8 commit b4228c4
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 2 deletions.
4 changes: 4 additions & 0 deletions src/cli/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ error_chain! {
description("toolchain is not installed")
display("toolchain '{}' is not installed", t)
}
ToolchainNotInstallable(t: String) {
description("toolchain is not installable")
display("toolchain '{}' is not installable", t)
}
InvalidToolchainName(t: String) {
description("invalid toolchain name")
display("invalid toolchain name: '{}'{}", t, maybe_suggest_toolchain(t))
Expand Down
4 changes: 4 additions & 0 deletions src/cli/rustup_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -977,6 +977,10 @@ fn update(cfg: &mut Cfg, m: &ArgMatches<'_>) -> Result<utils::ExitCode> {
};

if let Some(status) = status.clone() {
let toolchainname = toolchain.name().to_string();
if !toolchain.exists() {
return Err(ErrorKind::ToolchainNotInstallable(toolchainname).into());
}
writeln!(process().stdout())?;
common::show_channel_update(cfg, toolchain.name(), Ok(status))?;
}
Expand Down
3 changes: 3 additions & 0 deletions src/cli/self_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -743,6 +743,9 @@ fn maybe_install_rust(
let distributable = DistributableToolchain::new(&toolchain)?;
let status = distributable.install_from_dist(true, false, components, targets, None)?;
let toolchain_str = toolchain.name().to_owned();
if !toolchain.exists() {
return Err(ErrorKind::ToolchainNotInstallable(toolchain.name().to_string()).into());
}
toolchain.cfg().set_default(&toolchain_str)?;
writeln!(process().stdout())?;
common::show_channel_update(&toolchain.cfg(), &toolchain_str, Ok(status))?;
Expand Down
4 changes: 4 additions & 0 deletions src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,10 @@ error_chain! {
description("toolchain is not installed")
display("toolchain '{}' is not installed", t)
}
ToolchainNotInstallable(t: String) {
description("toolchain is not installable")
display("toolchain '{}' is not installable", t)
}
ToolchainNotSelected {
description("toolchain is not selected")
display("no override and no default toolchain set")
Expand Down
9 changes: 7 additions & 2 deletions src/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::dist::dist;
use crate::dist::download::DownloadCfg;
use crate::dist::prefix::InstallPrefix;
use crate::dist::Notification;
use crate::errors::Result;
use crate::errors::{ErrorKind, Result};
use crate::notifications::Notification as RootNotification;
use crate::toolchain::{CustomToolchain, DistributableToolchain, Toolchain, UpdateStatus};
use crate::utils::utils;
Expand Down Expand Up @@ -76,7 +76,12 @@ impl<'a> InstallMethod<'a> {
(false, _) => UpdateStatus::Unchanged,
};

Ok(status)
// Final check, to ensure we're installed
if !toolchain.exists() {
Err(ErrorKind::ToolchainNotInstallable(toolchain.name().to_string()).into())
} else {
Ok(status)
}
}

pub fn run(self, path: &Path, notify_handler: &dyn Fn(Notification<'_>)) -> Result<bool> {
Expand Down
20 changes: 20 additions & 0 deletions tests/cli-misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,26 @@ fn update_unavailable_rustc() {
});
}

// issue 2562
#[test]
fn install_unavailable_platform() {
clitools::setup(Scenario::Unavailable, &|config| {
set_current_dist_date(config, "2015-01-02");
// explicit attempt to install should fail
expect_err(
config,
&["rustup", "toolchain", "install", "nightly"],
"is not installable",
);
// implicit attempt to install should fail
expect_err(
config,
&["rustup", "default", "nightly"],
"is not installable",
);
});
}

#[test]
fn update_nightly_even_with_incompat() {
clitools::setup(Scenario::MissingComponent, &|config| {
Expand Down

0 comments on commit b4228c4

Please sign in to comment.