Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 27 additions & 7 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -625,17 +625,37 @@ impl Cfg {
targets: &[&str],
) -> Result<bool> {
let components_requested = !components.is_empty() || !targets.is_empty();

// If we're here, the toolchain exists on disk and is a dist toolchain
// so we should attempt to load its manifest
let manifest = if let Some(manifest) = distributable.get_manifest()? {
manifest
} else {
// If we can't read the manifest we'd best try and install
return Ok(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will this remove the corrupt dist?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it'll bubble the situation up a level which will then try to ensure the component set in question and report something along the lines of:

error: toolchain 'nightly-2020-11-24-x86_64-unknown-linux-gnu' does not support components

I think this is fair enough.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except that that should support components, and the reason it doesn't is that its corrupt, and we know its corrupt because we expected it to support components, and we returned Ok(true) rather than signalling the impossible situation...

I think this is good enough to do a release with; I don't think its good enough to consider done.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually no, I got that entirely wrong - argh. Let me faff.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think when I tested this, I must have accidentally reinstalled the test release rather than the version I was editing. I need to push a change to make that Ok(false) and then instead of being crap about it, it'll heal the installation.

};
match (distributable.list_components(), components_requested) {
// If the toolchain does not support components but there were components requested, bubble up the error
(Err(e), true) => Err(e),
(Ok(installed_components), _) => {
Ok(components.iter().chain(targets.iter()).all(|name| {
installed_components.iter().any(|status| {
status.component.short_name_in_manifest() == name && status.installed
// Otherwise check if all the components we want are installed
(Ok(installed_components), _) => Ok(components.iter().all(|name| {
installed_components.iter().any(|status| {
let cname = status.component.short_name(&manifest);
let cname = cname.as_str();
let cnameim = status.component.short_name_in_manifest();
let cnameim = cnameim.as_str();
(cname == *name || cnameim == *name) && status.installed
})
})
// And that all the targets we want are installed
&& targets.iter().all(|name| {
installed_components
.iter()
.filter(|c| c.component.short_name_in_manifest() == "rust-std")
.any(|status| {
let ctarg = status.component.target();
(ctarg == *name) && status.installed
})
}))
}
})),
_ => Ok(true),
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/toolchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,7 @@ impl<'a> DistributableToolchain<'a> {
}

// Installed only.
fn get_manifest(&self) -> Result<Option<Manifest>> {
pub fn get_manifest(&self) -> Result<Option<Manifest>> {
if !self.0.exists() {
return Err(ErrorKind::ToolchainNotInstalled(self.0.name().to_owned()).into());
}
Expand Down