Skip to content

Commit

Permalink
Auto-install toolchains more consistently
Browse files Browse the repository at this point in the history
This combines all auto-installation logic to the root of the codepaths
leading to running a binary, which should lead to auto-installation
happening for all cases, as long as a toolchain has been selected.

Config.find_default no longer verifies the toolchain in order to permit
this auto-installation to take place in one location; the callers of
find_default do not generally need verified toolchains, so this should
be fine.
  • Loading branch information
rbtcollins committed Mar 4, 2020
1 parent 6de2f76 commit af1f031
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 30 deletions.
4 changes: 2 additions & 2 deletions src/cli/rustup_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -928,7 +928,7 @@ fn show(cfg: &Cfg) -> Result<()> {

let cwd = utils::current_dir()?;
let installed_toolchains = cfg.list_toolchains()?;
let active_toolchain = cfg.find_override_toolchain_or_default(&cwd);
let active_toolchain = cfg.find_or_install_override_toolchain_or_default(&cwd);

// active_toolchain will carry the reason we don't have one in its detail.
let active_targets = if let Ok(ref at) = active_toolchain {
Expand Down Expand Up @@ -1053,7 +1053,7 @@ fn show(cfg: &Cfg) -> Result<()> {

fn show_active_toolchain(cfg: &Cfg) -> Result<()> {
let cwd = utils::current_dir()?;
if let Some((toolchain, reason)) = cfg.find_override_toolchain_or_default(&cwd)? {
if let Some((toolchain, reason)) = cfg.find_or_install_override_toolchain_or_default(&cwd)? {
if let Some(reason) = reason {
println!("{} ({})", toolchain.name(), reason);
} else {
Expand Down
60 changes: 32 additions & 28 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ impl Cfg {
}

pub fn which_binary(&self, path: &Path, binary: &str) -> Result<Option<PathBuf>> {
if let Some((toolchain, _)) = self.find_override_toolchain_or_default(path)? {
if let Some((toolchain, _)) = self.find_or_install_override_toolchain_or_default(path)? {
Ok(Some(toolchain.binary_file(binary)))
} else {
Ok(None)
Expand Down Expand Up @@ -370,10 +370,7 @@ impl Cfg {
.with(|s| Ok(s.default_toolchain.clone()))?;

if let Some(name) = opt_name {
let toolchain = self
.verify_toolchain(&name)
.chain_err(|| ErrorKind::ToolchainNotInstalled(name.to_string()))?;

let toolchain = Toolchain::from(self, &name)?;
Ok(Some(toolchain))
} else {
Ok(None)
Expand Down Expand Up @@ -427,24 +424,18 @@ impl Cfg {
),
};

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 {
toolchain.install_from_dist(true, false, &[], &[])?;
Ok(Some((toolchain, reason)))
}
}
Err(e) => Err(e)
.chain_err(|| Error::from(reason_err))
.chain_err(|| ErrorKind::OverrideToolchainNotInstalled(name.to_string())),
let toolchain = Toolchain::from(self, &name)?;
// Overridden toolchains can be literally any string, but only
// distributable toolchains will be auto-installed by the wrapping
// code; provide a nice error for this common case. (default could
// be set badly too, but that is much less common).
if !toolchain.exists() && 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 {
Ok(Some((toolchain, reason)))
}
} else {
Ok(None)
Expand Down Expand Up @@ -495,17 +486,30 @@ impl Cfg {
Ok(None)
}

pub fn find_override_toolchain_or_default(
pub fn find_or_install_override_toolchain_or_default(
&self,
path: &Path,
) -> Result<Option<(Toolchain<'_>, Option<OverrideReason>)>> {
Ok(
if let Some((toolchain, reason)) =
if let Some((toolchain, reason)) = self.find_override(path)? {
Some((toolchain, Some(reason)))
} else {
self.find_default()?.map(|toolchain| (toolchain, None))
},
)
}
{
if !toolchain.exists() {
if toolchain.is_custom() {
return Err(
ErrorKind::ToolchainNotInstalled(toolchain.name().to_string()).into(),
);
}
toolchain.install_from_dist(true, false, &[], &[])?;
}
Ok(Some((toolchain, reason)))
} else {
// No override and no default set
Err(ErrorKind::ToolchainNotSelected.into())
}
}

pub fn get_default(&self) -> Result<Option<String>> {
Expand Down Expand Up @@ -581,7 +585,7 @@ impl Cfg {
&self,
path: &Path,
) -> Result<(Toolchain<'_>, Option<OverrideReason>)> {
self.find_override_toolchain_or_default(path)
self.find_or_install_override_toolchain_or_default(path)
.and_then(|r| r.ok_or_else(|| "no default toolchain configured".into()))
}

Expand Down
4 changes: 4 additions & 0 deletions src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,10 @@ error_chain! {
description("toolchain is not installed")
display("toolchain '{}' is not installed", t)
}
ToolchainNotSelected {
description("toolchain is not selected")
display("no override and no default toolchain set")
}
OverrideToolchainNotInstalled(t: String) {
description("override toolchain is not installed")
display("override toolchain '{}' is not installed", t)
Expand Down

0 comments on commit af1f031

Please sign in to comment.