Make component removal best-effort and order-independent

Author: Inconnu08

src/cli/rustup_mode.rs

@@ -1481,12 +1481,33 @@ 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 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),
+            Err(err) => {
+                if first_error.is_none() {
+                    first_error = Some(err);
+                }
+            }
+        }
     }
 
-    Ok(ExitCode::SUCCESS)
+    for component in parsed_components {
+        if let Err(err) = distributable.remove_component(component).await {
+            if first_error.is_none() {
+                first_error = Some(err);
+            }
+        }
+    }
+
+    if let Some(err) = first_error {
+        Err(err)
+    } else {
+        Ok(ExitCode::SUCCESS)
+    }
 }
 
 async fn toolchain_link(cfg: &Cfg<'_>, dest: &CustomToolchainName, src: &Path) -> Result<ExitCode> {

tests/suite/cli_rustup.rs

@@ -2136,6 +2136,88 @@ 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 file_override() {
     let cx = CliTestContext::new(Scenario::SimpleV2).await;