Skip to content

Commit

Permalink
Automatically install override toolchain when missing.
Browse files Browse the repository at this point in the history
A typical scenario is:

* I work on a repository that uses `rust-toolchain` to pin to a specific Nightly version
* I run `git pull`, `rust-toolchain` has been changed to update to a new Rust version
* I run `cargo build`

Result before this PR (typically): rustup fails with an error like:

```
error: override toolchain 'nightly-2017-08-31' is not installed
info: caused by: the toolchain file at '/home/simon/projects/servo/rust-toolchain' specifies an uninstalled toolchain
```

A better result would be to install toolchains as needed.

Closes #1218
  • Loading branch information
SimonSapin committed Sep 12, 2017
1 parent 861eac9 commit 6c4c5e1
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 21 deletions.
22 changes: 13 additions & 9 deletions src/rustup/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,16 +259,20 @@ impl Cfg {
}
};

match self.verify_toolchain(&name) {
Ok(t) => {
Ok(Some((t, reason)))
match self.get_toolchain(&name, false) {
Ok(toolchain) => {
if toolchain.exists() {
Ok(Some((toolchain, reason)))
} else if toolchain.is_custom() {
// Strip the confusing NotADirectory error and only mention that the override
// toolchain is not installed.
Err(Error::from(reason_err))
.chain_err(|| ErrorKind::OverrideToolchainNotInstalled(name.to_string()))
} else {
try!(toolchain.install_from_dist());
Ok(Some((toolchain, reason)))
}
}
Err(Error(ErrorKind::Utils(::rustup_utils::ErrorKind::NotADirectory { .. }), _)) => {
// Strip the confusing NotADirectory error and only mention that the override
// toolchain is not installed.
Err(Error::from(reason_err))
.chain_err(|| ErrorKind::OverrideToolchainNotInstalled(name.to_string()))
},
Err(e) => {
Err(e)
.chain_err(|| Error::from(reason_err))
Expand Down
14 changes: 6 additions & 8 deletions tests/cli-rustup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ extern crate rustup_dist;
extern crate rustup_utils;
extern crate rustup_mock;
extern crate tempdir;
extern crate regex;

use std::fs;
use std::env::consts::EXE_SUFFIX;
Expand All @@ -16,7 +15,6 @@ use rustup_mock::clitools::{self, Config, Scenario,
expect_err,
set_current_dist_date,
this_host_triple};
use regex::Regex;

macro_rules! for_host { ($s: expr) => (&format!($s, this_host_triple())) }

Expand Down Expand Up @@ -621,15 +619,15 @@ fn show_toolchain_override_not_installed() {
setup(&|config| {
expect_ok(config, &["rustup", "override", "add", "nightly"]);
expect_ok(config, &["rustup", "toolchain", "remove", "nightly"]);
// I'm not sure this should really be erroring when the toolchain
// is not installed; just capturing the behavior.
let mut cmd = clitools::cmd(config, "rustup", &["show"]);
clitools::env(config, &mut cmd);
let out = cmd.output().unwrap();
assert!(out.status.success());
let stdout = String::from_utf8(out.stdout).unwrap();
let stderr = String::from_utf8(out.stderr).unwrap();
assert!(!stdout.contains("not a directory"));
assert!(Regex::new(r"error: override toolchain 'nightly.*' is not installed, the directory override for '.*' specifies an uninstalled toolchain").unwrap().is_match(&stdout))
assert!(!stdout.contains("is not installed"));
assert!(stderr.contains("info: installing component 'rustc'"));
});
}

Expand Down Expand Up @@ -658,11 +656,11 @@ fn show_toolchain_env_not_installed() {
clitools::env(config, &mut cmd);
cmd.env("RUSTUP_TOOLCHAIN", "nightly");
let out = cmd.output().unwrap();
// I'm not sure this should really be erroring when the toolchain
// is not installed; just capturing the behavior.
assert!(out.status.success());
let stdout = String::from_utf8(out.stdout).unwrap();
assert!(stdout.contains("override toolchain 'nightly' is not installed, the RUSTUP_TOOLCHAIN environment variable specifies an uninstalled toolchain"));
let stderr = String::from_utf8(out.stderr).unwrap();
assert!(!stdout.contains("is not installed"));
assert!(stderr.contains("info: installing component 'rustc'"));
});
}

Expand Down
3 changes: 1 addition & 2 deletions tests/cli-v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,7 @@ fn remove_override_toolchain_err_handling() {
expect_ok(config, &["rustup", "default", "nightly"]);
expect_ok(config, &["rustup", "override", "add", "beta"]);
expect_ok(config, &["rustup", "toolchain", "remove", "beta"]);
expect_err(config, &["rustc"],
for_host!("toolchain 'beta-{0}' is not installed"));
expect_stderr_ok(config, &["rustc", "--version"], "info: installing component");
});
});
}
Expand Down
3 changes: 1 addition & 2 deletions tests/cli-v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,7 @@ fn remove_override_toolchain_err_handling() {
expect_ok(config, &["rustup", "default", "nightly"]);
expect_ok(config, &["rustup", "override", "add", "beta"]);
expect_ok(config, &["rustup", "toolchain", "remove", "beta"]);
expect_err(config, &["rustc"],
for_host!("toolchain 'beta-{0}' is not installed"));
expect_stderr_ok(config, &["rustc", "--version"], "info: installing component");
});
});
}
Expand Down

0 comments on commit 6c4c5e1

Please sign in to comment.