Move list_components to DistributableToolchain

Author: rbtcollins

src/cli/common.rs

@@ -6,6 +6,7 @@ use crate::term2;
 use git_testament::{git_testament, render_testament};
 use lazy_static::lazy_static;
 use rustup::dist::notifications as dist_notifications;
+use rustup::toolchain::DistributableToolchain;
 use rustup::utils::notifications as util_notifications;
 use rustup::utils::notify::NotificationLevel;
 use rustup::utils::utils;
@@ -346,16 +347,9 @@ where
 
 pub fn list_targets(toolchain: &Toolchain<'_>) -> Result<()> {
     let mut t = term2::stdout();
-    let components = match toolchain.list_components()? {
-        // XXX: long term move this error to cli ? the normal .into doesn't work
-        // because Result here is the wrong sort and expression type ascription
-        // isn't a feature yet.
-        None => Err(rustup::Error(
-            rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()),
-            error_chain::State::default(),
-        )),
-        Some(components) => Ok(components),
-    }?;
+    let distributable = DistributableToolchain::new(&toolchain)
+        .chain_err(|| rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()))?;
+    let components = distributable.list_components()?;
     for component in components {
         if component.component.short_name_in_manifest() == "rust-std" {
             let target = component
@@ -378,16 +372,9 @@ pub fn list_targets(toolchain: &Toolchain<'_>) -> Result<()> {
 
 pub fn list_installed_targets(toolchain: &Toolchain<'_>) -> Result<()> {
     let mut t = term2::stdout();
-    let components = match toolchain.list_components()? {
-        // XXX: long term move this error to cli ? the normal .into doesn't work
-        // because Result here is the wrong sort and expression type ascription
-        // isn't a feature yet.
-        None => Err(rustup::Error(
-            rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()),
-            error_chain::State::default(),
-        )),
-        Some(components) => Ok(components),
-    }?;
+    let distributable = DistributableToolchain::new(&toolchain)
+        .chain_err(|| rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()))?;
+    let components = distributable.list_components()?;
     for component in components {
         if component.component.short_name_in_manifest() == "rust-std" {
             let target = component
@@ -405,16 +392,9 @@ pub fn list_installed_targets(toolchain: &Toolchain<'_>) -> Result<()> {
 
 pub fn list_components(toolchain: &Toolchain<'_>) -> Result<()> {
     let mut t = term2::stdout();
-    let components = match toolchain.list_components()? {
-        // XXX: long term move this error to cli ? the normal .into doesn't work
-        // because Result here is the wrong sort and expression type ascription
-        // isn't a feature yet.
-        None => Err(rustup::Error(
-            rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()),
-            error_chain::State::default(),
-        )),
-        Some(components) => Ok(components),
-    }?;
+    let distributable = DistributableToolchain::new(&toolchain)
+        .chain_err(|| rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()))?;
+    let components = distributable.list_components()?;
     for component in components {
         let name = component.name;
         if component.installed {
@@ -431,16 +411,9 @@ pub fn list_components(toolchain: &Toolchain<'_>) -> Result<()> {
 
 pub fn list_installed_components(toolchain: &Toolchain<'_>) -> Result<()> {
     let mut t = term2::stdout();
-    let components = match toolchain.list_components()? {
-        // XXX: long term move this error to cli ? the normal .into doesn't work
-        // because Result here is the wrong sort and expression type ascription
-        // isn't a feature yet.
-        None => Err(rustup::Error(
-            rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()),
-            error_chain::State::default(),
-        )),
-        Some(components) => Ok(components),
-    }?;
+    let distributable = DistributableToolchain::new(&toolchain)
+        .chain_err(|| rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()))?;
+    let components = distributable.list_components()?;
     for component in components {
         if component.installed {
             writeln!(t, "{}", component.name)?;

src/cli/rustup_mode.rs

@@ -936,14 +936,18 @@ fn show(cfg: &Cfg) -> Result<()> {
 
     // active_toolchain will carry the reason we don't have one in its detail.
     let active_targets = if let Ok(ref at) = active_toolchain {
-        match at.0.list_components() {
-            Ok(None) => vec![],
-            Ok(Some(cs_vec)) => cs_vec
-                .into_iter()
-                .filter(|c| c.component.short_name_in_manifest() == "rust-std")
-                .filter(|c| c.installed)
-                .collect(),
-            Err(_) => vec![],
+        if let Ok(distributable) = DistributableToolchain::new(&at.0) {
+            match distributable.list_components() {
+                Ok(cs_vec) => cs_vec
+                    .into_iter()
+                    .filter(|c| c.component.short_name_in_manifest() == "rust-std")
+                    .filter(|c| c.installed)
+                    .collect(),
+                Err(_) => vec![],
+            }
+        } else {
+            // These three vec![] could perhaps be reduced with and_then on active_toolchain.
+            vec![]
         }
     } else {
         vec![]
@@ -1109,7 +1113,8 @@ fn target_add(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
         }
 
         targets.clear();
-        for component in toolchain.list_components()?.unwrap() {
+        let distributable = DistributableToolchain::new(&toolchain)?;
+        for component in distributable.list_components()? {
             if component.component.short_name_in_manifest() == "rust-std"
                 && component.available
                 && !component.installed
@@ -1333,28 +1338,26 @@ const DOCS_DATA: &[(&str, &str, &str,)] = &[
 
 fn doc(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
     let toolchain = explicit_or_dir_toolchain(cfg, m)?;
-    match toolchain.list_components()? {
-        None => { /* custom - no validation */ }
-        Some(components) => {
-            if let [_] = components
-                .into_iter()
-                .filter(|cstatus| {
-                    cstatus.component.short_name_in_manifest() == "rust-docs" && !cstatus.installed
-                })
-                .take(1)
-                .collect::<Vec<ComponentStatus>>()
-                .as_slice()
-            {
-                info!(
-                    "`rust-docs` not installed in toolchain `{}`",
-                    toolchain.name()
-                );
-                info!(
-                    "To install, try `rustup component add --toolchain {} rust-docs`",
-                    toolchain.name()
-                );
-                return Err("unable to view documentation which is not installed".into());
-            }
+    if let Ok(distributable) = DistributableToolchain::new(&toolchain) {
+        let components = distributable.list_components()?;
+        if let [_] = components
+            .into_iter()
+            .filter(|cstatus| {
+                cstatus.component.short_name_in_manifest() == "rust-docs" && !cstatus.installed
+            })
+            .take(1)
+            .collect::<Vec<ComponentStatus>>()
+            .as_slice()
+        {
+            info!(
+                "`rust-docs` not installed in toolchain `{}`",
+                toolchain.name()
+            );
+            info!(
+                "To install, try `rustup component add --toolchain {} rust-docs`",
+                toolchain.name()
+            );
+            return Err("unable to view documentation which is not installed".into());
         }
     }
     let topical_path: PathBuf;

src/toolchain.rs

@@ -496,74 +496,6 @@ impl<'a> Toolchain<'a> {
         })
     }
     // Distributable only. Installed only.
-    pub fn list_components(&self) -> Result<Option<Vec<ComponentStatus>>> {
-        if !self.exists() {
-            return Err(ErrorKind::ToolchainNotInstalled(self.name.to_owned()).into());
-        }
-
-        let toolchain = &self.name;
-        let toolchain = match ToolchainDesc::from_str(toolchain).ok() {
-            None => return Ok(None),
-            Some(toolchain) => toolchain,
-        };
-
-        let prefix = InstallPrefix::from(self.path.to_owned());
-        let manifestation = Manifestation::open(prefix, toolchain.target.clone())?;
-
-        if let Some(manifest) = manifestation.load_manifest()? {
-            let config = manifestation.read_config()?;
-
-            // Return all optional components of the "rust" package for the
-            // toolchain's target triple.
-            let mut res = Vec::new();
-
-            let rust_pkg = manifest
-                .packages
-                .get("rust")
-                .expect("manifest should contain a rust package");
-            let targ_pkg = rust_pkg
-                .targets
-                .get(&toolchain.target)
-                .expect("installed manifest should have a known target");
-
-            for component in &targ_pkg.components {
-                let installed = config
-                    .as_ref()
-                    .map(|c| component.contained_within(&c.components))
-                    .unwrap_or(false);
-
-                let component_target = TargetTriple::new(&component.target());
-
-                // Get the component so we can check if it is available
-                let component_pkg = manifest
-                    .get_package(&component.short_name_in_manifest())
-                    .unwrap_or_else(|_| {
-                        panic!(
-                            "manifest should contain component {}",
-                            &component.short_name(&manifest)
-                        )
-                    });
-                let component_target_pkg = component_pkg
-                    .targets
-                    .get(&component_target)
-                    .expect("component should have target toolchain");
-
-                res.push(ComponentStatus {
-                    component: component.clone(),
-                    name: component.name(&manifest),
-                    installed,
-                    available: component_target_pkg.available(),
-                });
-            }
-
-            res.sort_by(|a, b| a.component.cmp(&b.component));
-
-            Ok(Some(res))
-        } else {
-            Err(ErrorKind::ComponentsUnsupported(self.name.to_string()).into())
-        }
-    }
-    // Distributable only. Installed only.
     fn get_component_suggestion(
         &self,
         component: &Component,
@@ -576,8 +508,9 @@ impl<'a> Toolchain<'a> {
         // High number can result in inaccurate suggestions for short queries e.g. `rls`
         const MAX_DISTANCE: usize = 3;
 
-        let components = self.list_components();
-        if let Ok(Some(components)) = components {
+        let distributable = DistributableToolchain::new(&self);
+        let components = distributable.ok().map(|d| d.list_components());
+        if let Some(Ok(components)) = components {
             let short_name_distance = components
                 .iter()
                 .filter(|c| !only_installed || c.installed)
@@ -825,6 +758,73 @@ impl<'a> DistributableToolchain<'a> {
         ))
     }
 
+    // Installed only.
+    pub fn list_components(&self) -> Result<Vec<ComponentStatus>> {
+        if !self.0.exists() {
+            return Err(ErrorKind::ToolchainNotInstalled(self.0.name.to_owned()).into());
+        }
+
+        let toolchain = &self.0.name;
+        let toolchain = ToolchainDesc::from_str(toolchain)
+            .chain_err(|| ErrorKind::ComponentsUnsupported(self.0.name.to_string()))?;
+
+        let prefix = InstallPrefix::from(self.0.path.to_owned());
+        let manifestation = Manifestation::open(prefix, toolchain.target.clone())?;
+
+        if let Some(manifest) = manifestation.load_manifest()? {
+            let config = manifestation.read_config()?;
+
+            // Return all optional components of the "rust" package for the
+            // toolchain's target triple.
+            let mut res = Vec::new();
+
+            let rust_pkg = manifest
+                .packages
+                .get("rust")
+                .expect("manifest should contain a rust package");
+            let targ_pkg = rust_pkg
+                .targets
+                .get(&toolchain.target)
+                .expect("installed manifest should have a known target");
+
+            for component in &targ_pkg.components {
+                let installed = config
+                    .as_ref()
+                    .map(|c| component.contained_within(&c.components))
+                    .unwrap_or(false);
+
+                let component_target = TargetTriple::new(&component.target());
+
+                // Get the component so we can check if it is available
+                let component_pkg = manifest
+                    .get_package(&component.short_name_in_manifest())
+                    .unwrap_or_else(|_| {
+                        panic!(
+                            "manifest should contain component {}",
+                            &component.short_name(&manifest)
+                        )
+                    });
+                let component_target_pkg = component_pkg
+                    .targets
+                    .get(&component_target)
+                    .expect("component should have target toolchain");
+
+                res.push(ComponentStatus {
+                    component: component.clone(),
+                    name: component.name(&manifest),
+                    installed,
+                    available: component_target_pkg.available(),
+                });
+            }
+
+            res.sort_by(|a, b| a.component.cmp(&b.component));
+
+            Ok(res)
+        } else {
+            Err(ErrorKind::ComponentsUnsupported(self.0.name.to_string()).into())
+        }
+    }
+
     // Installed only.
     pub fn remove_component(&self, mut component: Component) -> Result<()> {
         // Overlapping code with get_manifest :/.