refactor(toolchain)!: remove multiple components at once

src/cli/rustup_mode.rs

@@ -25,7 +25,6 @@ use clap_complete::{
 };
 use futures_util::stream::StreamExt;
 use indicatif::{MultiProgress, ProgressBar, ProgressStyle};
-use itertools::Itertools;
 use serde::Serialize;
 use tokio::sync::Semaphore;
 use tracing::{info, warn};
@@ -551,7 +550,7 @@ enum TargetSubcmd {
     Remove {
         /// List of targets to uninstall
         #[arg(required = true, num_args = 1..)]
-        target: Vec<String>,
+        target: Vec<TargetTuple>,
 
         #[arg(long, help = official_toolchain_arg_help())]
         toolchain: Option<PartialToolchainDesc>,
@@ -1494,7 +1493,7 @@ async fn target_add(
         distributable
             .add_components(distributable.components()?.into_iter().filter_map(|c| {
                 (c.available && !c.installed && c.component.short_name() == "rust-std")
-                    .then_some(c.component)
+                    .then_some(Ok(c.component))
             }))
             .await?;
 
@@ -1505,7 +1504,7 @@ async fn target_add(
         .add_components(
             targets
                 .into_iter()
-                .map(|target| Component::std(TargetTuple::new(target))),
+                .map(|target| Ok(Component::std(TargetTuple::new(target)))),
         )
         .await?;
 
@@ -1514,7 +1513,7 @@ async fn target_add(
 
 async fn target_remove(
     cfg: &Cfg<'_>,
-    targets: Vec<String>,
+    targets: Vec<TargetTuple>,
     toolchain: Option<PartialToolchainDesc>,
 ) -> anyhow::Result<ExitCode> {
     let distributable = DistributableToolchain::from_partial(
@@ -1523,33 +1522,21 @@ async fn target_remove(
     )
     .await?;
 
-    for target in targets {
-        let target = TargetTuple::new(target);
-        let default_target = cfg.default_host_tuple()?;
-        if target == default_target {
-            warn!(
-                "removing the default host target; proc-macros and build scripts might no longer build"
-            );
-        }
-        // Whether we have at most 1 component target that is not `None` (wildcard).
-        let has_at_most_one_target = distributable
-            .components()?
-            .into_iter()
-            .filter_map(|c| match (c.installed, c.component.target) {
-                (true, Some(t)) => Some(t),
-                _ => None,
-            })
-            .unique()
-            .at_most_one()
-            .is_ok();
-        if has_at_most_one_target {
-            warn!("removing the last target; no build targets will be available");
-        }
-        distributable
-            .remove_component(Component::std(target))
-            .await?;
+    if targets.contains(&cfg.default_host_tuple()?) {
+        warn!(
+            "removing the default host target; proc-macros and build scripts might no longer build"
+        );
     }
 
+    let mut remaining_targets = distributable.toolchain.installed_targets()?;
+    remaining_targets.retain(|it| !targets.contains(it));
+    if remaining_targets.is_empty() {
+        warn!("removing the last target; no build targets will be available");
+    }
+
+    distributable
+        .remove_components(targets.into_iter().map(|c| Ok(Component::std(c))))
+        .await?;
     Ok(ExitCode::SUCCESS)
 }
 
@@ -1604,9 +1591,7 @@ async fn component_add(
         .add_components(
             components
                 .into_iter()
-                .map(|component| Component::try_new(&component, &distributable, target.as_ref()))
-                .collect::<anyhow::Result<Vec<_>>>()?
-                .into_iter(),
+                .map(|component| Component::try_new(&component, &distributable, target.as_ref())),
         )
         .await?;
 
@@ -1632,37 +1617,14 @@ async fn component_remove(
     let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?;
     let target = get_target(target, &distributable);
 
-    let parsed_components = components
-        .iter()
-        .map(|component| Component::try_new(component, &distributable, target.as_ref()))
-        .collect::<anyhow::Result<Vec<_>>>()?;
-
-    let mut unknown_components = Vec::new();
-
-    for component in parsed_components {
-        let Err(err) = distributable.remove_component(component).await else {
-            continue;
-        };
-
-        if let Some(RustupError::UnknownComponents { components, .. }) =
-            err.downcast_ref::<RustupError>()
-        {
-            unknown_components.extend(components.iter().cloned());
-            continue;
-        }
-
-        return Err(err);
-    }
-
-    if unknown_components.is_empty() {
-        Ok(ExitCode::SUCCESS)
-    } else {
-        Err(RustupError::UnknownComponents {
-            desc: distributable.desc().clone(),
-            components: unknown_components,
-        }
-        .into())
-    }
+    distributable
+        .remove_components(
+            components
+                .iter()
+                .map(|component| Component::try_new(component, &distributable, target.as_ref())),
+        )
+        .await?;
+    Ok(ExitCode::SUCCESS)
 }
 
 async fn toolchain_link(

src/dist/mod.rs

@@ -678,6 +678,12 @@ impl TargetTuple {
     }
 }
 
+impl From<String> for TargetTuple {
+    fn from(s: String) -> Self {
+        Self(s)
+    }
+}
+
 impl fmt::Display for TargetTuple {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         self.0.fmt(f)

src/toolchain/distributable.rs

@@ -65,7 +65,7 @@ impl<'a> DistributableToolchain<'a> {
 
     pub(crate) async fn add_components(
         &self,
-        components: impl Iterator<Item = Component>,
+        components: impl IntoIterator<Item = anyhow::Result<Component>>,
     ) -> anyhow::Result<()> {
         let manifestation = self.get_manifestation()?;
         let manifest = self.get_manifest()?;
@@ -79,9 +79,11 @@ impl<'a> DistributableToolchain<'a> {
             .get(&self.desc.target)
             .expect("installed manifest should have a known target");
 
+        let components = components.into_iter();
         let mut validated_components = Vec::with_capacity(components.size_hint().0);
 
-        for mut component in components {
+        for component in components {
+            let mut component = component?;
             if let Some(c) = manifest.rename_component(&component) {
                 component = c;
             }
@@ -383,59 +385,72 @@ impl<'a> DistributableToolchain<'a> {
         }
     }
 
-    pub(crate) async fn remove_component(&self, mut component: Component) -> anyhow::Result<()> {
-        // TODO: take multiple components?
+    pub(crate) async fn remove_components(
+        &self,
+        components: impl IntoIterator<Item = anyhow::Result<Component>>,
+    ) -> anyhow::Result<()> {
         let manifestation = self.get_manifestation()?;
         let config = manifestation.read_config()?.unwrap_or_default();
         let manifest = self.get_manifest()?;
 
-        // Rename the component if necessary.
-        if let Some(c) = manifest.rename_component(&component) {
-            component = c;
-        }
+        let components = components.into_iter();
+        let mut renamed_components = Vec::with_capacity(components.size_hint().0);
+        let mut unknown_components = vec![];
+        for component in components {
+            let mut component = component?;
+            if let Some(renamed) = manifest.rename_component(&component) {
+                component = renamed;
+            }
+            if config.components.contains(&component) {
+                renamed_components.push(component);
+                continue;
+            }
 
-        if !config.components.contains(&component) {
             let wildcard_component = component.wildcard();
             if config.components.contains(&wildcard_component) {
-                component = wildcard_component;
-            } else {
-                let suggestion =
-                    self.get_component_suggestion(&component, &config, &manifest, true);
-                // Check if the target is installed.
-                if !config
-                    .components
-                    .iter()
-                    .any(|c| c.target() == component.target())
-                {
-                    return Err(RustupError::TargetNotInstalled {
-                        desc: Box::new(self.desc.clone()),
-                        target: component.target.expect("component target should be known"),
-                        suggestion,
-                    }
-                    .into());
-                }
-                return Err(RustupError::UnknownComponents {
-                    desc: self.desc.clone(),
-                    components: vec![UnknownComponentInfo {
-                        name: manifest.short_name(&component).to_string(),
-                        description: manifest.description(&component),
-                        suggestion,
-                    }],
+                renamed_components.push(wildcard_component);
+                continue;
+            }
+
+            let suggestion = self.get_component_suggestion(&component, &config, &manifest, true);
+            // Check if the target is installed.
+            if !config
+                .components
+                .iter()
+                .any(|c| c.target() == component.target())
+            {
+                return Err(RustupError::TargetNotInstalled {
+                    desc: Box::new(self.desc.clone()),
+                    target: component.target.expect("component target should be known"),
+                    suggestion,
                 }
                 .into());
             }
+            unknown_components.push(UnknownComponentInfo {
+                name: manifest.short_name(&component).to_string(),
+                description: manifest.description(&component),
+                suggestion,
+            });
         }
 
         let changes = Changes {
             explicit_add_components: vec![],
-            remove_components: vec![component],
+            remove_components: renamed_components,
         };
 
         let download_cfg = DownloadCfg::new(self.toolchain.cfg);
         manifestation
             .update(manifest, changes, false, &download_cfg, &self.desc, false)
             .await?;
 
+        if !unknown_components.is_empty() {
+            return Err(RustupError::UnknownComponents {
+                desc: self.desc().clone(),
+                components: unknown_components,
+            }
+            .into());
+        }
+
         Ok(())
     }