Make component removal best-effort and preserve single-error behavior

Author: Inconnu08

src/cli/rustup_mode.rs

@@ -1492,12 +1492,59 @@ async fn component_remove(
     let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?;
     let target = get_target(target, &distributable);
 
+    let mut parsed_components = Vec::new();
+    let mut unknown_components = Vec::new();
+    // Preserve the original UnknownComponent error to keep
+    // single-component error behavior unchanged
+    let mut first_unknown_component_error = None;
+    let mut first_error = None;
+
     for component in &components {
-        let new_component = Component::try_new(component, &distributable, target.as_ref())?;
-        distributable.remove_component(new_component).await?;
+        match Component::try_new(component, &distributable, target.as_ref()) {
+            Ok(new_component) => parsed_components.push((new_component, component)),
+            Err(err) => {
+                first_error.get_or_insert(err);
+            }
+        }
     }
 
-    Ok(ExitCode::SUCCESS)
+    for (component, original_name) in parsed_components {
+        let Err(err) = distributable.remove_component(component).await else {
+            continue;
+        };
+
+        if let Some(RustupError::UnknownComponent { suggestion, .. }) =
+            err.downcast_ref::<RustupError>()
+        {
+            unknown_components.push((original_name.clone(), suggestion.clone()));
+            first_unknown_component_error.get_or_insert(err);
+            continue;
+        }
+
+        first_error.get_or_insert(err);
+    }
+
+    match unknown_components.len() {
+        0 => {}
+        1 => {
+            return Err(
+                first_unknown_component_error.expect("missing first unknown component error")
+            );
+        }
+        _ => {
+            return Err(RustupError::UnknownComponents {
+                desc: distributable.desc().clone(),
+                components: unknown_components,
+            }
+            .into());
+        }
+    }
+
+    if let Some(err) = first_error {
+        Err(err)
+    } else {
+        Ok(ExitCode::SUCCESS)
+    }
 }
 
 async fn toolchain_link(cfg: &Cfg<'_>, dest: &CustomToolchainName, src: &Path) -> Result<ExitCode> {

src/errors.rs

@@ -1,7 +1,7 @@
 #![allow(clippy::large_enum_variant)]
 
 use std::ffi::OsString;
-use std::fmt::Debug;
+use std::fmt::{Debug, Write as FmtWrite};
 use std::io;
 use std::io::Write;
 use std::path::PathBuf;
@@ -134,6 +134,11 @@ pub enum RustupError {
         component: String,
         suggestion: Option<String>,
     },
+    #[error("{}", unknown_components_msg(.desc, .components))]
+    UnknownComponents {
+        desc: ToolchainDesc,
+        components: Vec<(String, Option<String>)>,
+    },
     #[error(
         "toolchain '{desc}' has no prebuilt artifacts available for target '{platform}'\n\
         note: this may happen to a low-tier target as per https://doc.rust-lang.org/nightly/rustc/platform-support.html\n\
@@ -242,3 +247,27 @@ fn component_unavailable_msg(cs: &[Component], manifest: &Manifest, toolchain: &
 
     String::from_utf8(buf).unwrap()
 }
+
+fn unknown_components_msg(desc: &ToolchainDesc, components: &[(String, Option<String>)]) -> String {
+    let mut buf = String::new();
+
+    match components {
+        [] => panic!("`unknown_components_msg` should not be called with an empty collection"),
+        [(component, suggestion)] => {
+            let _ = write!(
+                buf,
+                "toolchain '{desc}' does not contain component '{component}'{}",
+                suggest_message(suggestion),
+            );
+        }
+        components => {
+            let _ = writeln!(buf, "toolchain '{desc}' does not contain these components:");
+
+            for (component, suggestion) in components {
+                let _ = writeln!(buf, "  - '{component}'{}", suggest_message(suggestion),);
+            }
+        }
+    }
+
+    buf
+}

tests/suite/cli_rustup.rs

@@ -2136,6 +2136,145 @@ async fn add_remove_multiple_components() {
     }
 }
 
+#[tokio::test]
+async fn remove_multiple_components_is_best_effort_and_order_independent() {
+    let files = [
+        "lib/rustlib/src/rust-src/foo.rs".to_owned(),
+        format!("lib/rustlib/{}/analysis/libfoo.json", this_host_triple()),
+    ];
+
+    // Case 1: invalid component first
+    let cx = CliTestContext::new(Scenario::SimpleV2).await;
+    cx.config
+        .expect(&["rustup", "default", "nightly"])
+        .await
+        .is_ok();
+    cx.config
+        .expect(&["rustup", "component", "add", "rust-src", "rust-analysis"])
+        .await
+        .is_ok();
+
+    for file in &files {
+        let path = format!("toolchains/nightly-{}/{}", this_host_triple(), file);
+        assert!(cx.config.rustupdir.has(&path));
+    }
+
+    cx.config
+        .expect(&[
+            "rustup",
+            "component",
+            "remove",
+            "bad-component",
+            "rust-src",
+            "rust-analysis",
+        ])
+        .await
+        .is_err();
+
+    for file in &files {
+        let path = PathBuf::from(format!(
+            "toolchains/nightly-{}/{}",
+            this_host_triple(),
+            file
+        ));
+        assert!(!cx.config.rustupdir.has(path.parent().unwrap()));
+    }
+
+    // Case 2: invalid component last
+    let cx = CliTestContext::new(Scenario::SimpleV2).await;
+    cx.config
+        .expect(&["rustup", "default", "nightly"])
+        .await
+        .is_ok();
+    cx.config
+        .expect(&["rustup", "component", "add", "rust-src", "rust-analysis"])
+        .await
+        .is_ok();
+
+    for file in &files {
+        let path = format!("toolchains/nightly-{}/{}", this_host_triple(), file);
+        assert!(cx.config.rustupdir.has(&path));
+    }
+
+    cx.config
+        .expect(&[
+            "rustup",
+            "component",
+            "remove",
+            "rust-src",
+            "rust-analysis",
+            "bad-component",
+        ])
+        .await
+        .is_err();
+
+    for file in &files {
+        let path = PathBuf::from(format!(
+            "toolchains/nightly-{}/{}",
+            this_host_triple(),
+            file
+        ));
+        assert!(!cx.config.rustupdir.has(path.parent().unwrap()));
+    }
+}
+
+#[tokio::test]
+async fn remove_multiple_components_reports_all_invalid_names() {
+    let files = [
+        "lib/rustlib/src/rust-src/foo.rs".to_owned(),
+        format!("lib/rustlib/{}/analysis/libfoo.json", this_host_triple()),
+    ];
+
+    let cx = CliTestContext::new(Scenario::SimpleV2).await;
+    cx.config
+        .expect(["rustup", "default", "nightly"])
+        .await
+        .is_ok();
+
+    cx.config
+        .expect(["rustup", "component", "add", "rust-src", "rust-analysis"])
+        .await
+        .is_ok();
+
+    // Ensure components exist
+    for file in &files {
+        let path = format!("toolchains/nightly-{}/{}", this_host_triple(), file);
+        assert!(cx.config.rustupdir.has(&path));
+    }
+
+    cx.config
+        .expect([
+            "rustup",
+            "component",
+            "remove",
+            "bad-component-1",
+            "rust-src",
+            "bad-component-2",
+            "rust-analysis",
+        ])
+        .await
+        .with_stderr(snapbox::str![[r#"
+info: removing component rust-src
+info: removing component rust-analysis
+error: toolchain 'nightly-[HOST_TRIPLE]' does not contain these components:
+  - 'bad-component-1'
+  - 'bad-component-2'
+
+
+"#]])
+        .is_err();
+
+    // Ensure valid components were removed
+    for file in &files {
+        let path = PathBuf::from(format!(
+            "toolchains/nightly-{}/{}",
+            this_host_triple(),
+            file
+        ));
+        assert!(!cx.config.rustupdir.has(path.parent().unwrap()));
+    }
+}
+
 #[tokio::test]
 async fn file_override() {
     let cx = CliTestContext::new(Scenario::SimpleV2).await;