From 64e4d8fe67ab8511957ddc3a7d997c7794c7115f Mon Sep 17 00:00:00 2001 From: taufiqrahman Date: Tue, 7 Apr 2026 23:08:07 -0400 Subject: [PATCH] Make component removal best-effort and preserve single-error behavior --- src/cli/rustup_mode.rs | 33 ++++++++-- src/errors.rs | 66 +++++++++++++++++--- src/toolchain/distributable.rs | 20 ++++-- tests/suite/cli_rustup.rs | 109 ++++++++++++++++++++++++++++++--- tests/suite/cli_v2.rs | 18 ++++-- 5 files changed, 214 insertions(+), 32 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 91f4af9c60..c9faccc1ee 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -1492,12 +1492,37 @@ async fn component_remove( let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?; let target = get_target(target, &distributable); - for component in &components { - let new_component = Component::try_new(component, &distributable, target.as_ref())?; - distributable.remove_component(new_component).await?; + let parsed_components = components + .iter() + .map(|component| Component::try_new(component, &distributable, target.as_ref())) + .collect::>>()?; + + 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::() + { + unknown_components.extend(components.iter().cloned()); + continue; + } + + return Err(err); } - Ok(ExitCode::SUCCESS) + if unknown_components.is_empty() { + Ok(ExitCode::SUCCESS) + } else { + Err(RustupError::UnknownComponents { + desc: distributable.desc().clone(), + components: unknown_components, + } + .into()) + } } async fn toolchain_link(cfg: &Cfg<'_>, dest: &CustomToolchainName, src: &Path) -> Result { diff --git a/src/errors.rs b/src/errors.rs index 79e6664d64..0a406f2699 100644 --- a/src/errors.rs +++ b/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; @@ -23,6 +23,13 @@ use crate::{ #[error(transparent)] pub struct OperationError(pub anyhow::Error); +#[derive(Debug, Clone)] +pub struct UnknownComponentInfo { + pub name: String, + pub description: String, + pub suggestion: Option, +} + #[derive(ThisError, Debug)] pub enum RustupError { #[error("partially downloaded file may have been damaged and was removed, please try again")] @@ -124,15 +131,10 @@ pub enum RustupError { help: run 'rustup default stable' to download the latest stable release of Rust and set it as your default toolchain." )] ToolchainNotSelected(String), - #[error("toolchain '{}' does not contain component {}{}{}", .desc, .component, suggest_message(.suggestion), if .component.contains("rust-std") { - format!("\nnote: not all platforms have the standard library pre-compiled: https://doc.rust-lang.org/nightly/rustc/platform-support.html{}", - if desc.channel == Channel::Nightly { "\nhelp: consider using `cargo build -Z build-std` instead" } else { "" } - ) - } else { "".to_string() })] - UnknownComponent { + #[error("{}", unknown_components_msg(.desc, .components))] + UnknownComponents { desc: ToolchainDesc, - component: String, - suggestion: Option, + components: Vec, }, #[error( "toolchain '{desc}' has no prebuilt artifacts available for target '{platform}'\n\ @@ -242,3 +244,49 @@ fn component_unavailable_msg(cs: &[Component], manifest: &Manifest, toolchain: & String::from_utf8(buf).unwrap() } + +fn unknown_components_msg(desc: &ToolchainDesc, components: &[UnknownComponentInfo]) -> String { + let mut buf = String::new(); + + match components { + [] => panic!("`unknown_components_msg` should not be called with an empty collection"), + [component] => { + let _ = write!( + buf, + "toolchain '{desc}' does not contain component {}", + component.description, + ); + + if let Some(suggestion) = &component.suggestion { + let _ = write!(buf, "\nhelp: did you mean '{suggestion}'?"); + } + + if component.description.contains("rust-std") { + let _ = write!( + buf, + "\nnote: not all platforms have the standard library pre-compiled: https://doc.rust-lang.org/nightly/rustc/platform-support.html" + ); + + if desc.channel == Channel::Nightly { + let _ = write!( + buf, + "\nhelp: consider using `cargo build -Z build-std` instead" + ); + } + } + } + components => { + let _ = writeln!(buf, "toolchain '{desc}' does not contain these components:"); + + for component in components { + let _ = writeln!(buf, " - '{}'", component.name); + + if let Some(suggestion) = &component.suggestion { + let _ = writeln!(buf, " help: did you mean '{suggestion}'?"); + } + } + } + } + + buf +} diff --git a/src/toolchain/distributable.rs b/src/toolchain/distributable.rs index 74e2874b70..ea4b2f676a 100644 --- a/src/toolchain/distributable.rs +++ b/src/toolchain/distributable.rs @@ -21,6 +21,8 @@ use crate::{ install::{InstallMethod, UpdateStatus}, }; +use crate::errors::UnknownComponentInfo; + use super::{ Toolchain, names::{LocalToolchainName, ToolchainName}, @@ -100,10 +102,13 @@ impl<'a> DistributableToolchain<'a> { .iter() .any(|c| c.target() == component.target()) { - return Err(RustupError::UnknownComponent { + return Err(RustupError::UnknownComponents { desc, - component: manifest.description(&component), - suggestion, + components: vec![UnknownComponentInfo { + name: manifest.short_name(&component).to_string(), + description: manifest.description(&component), + suggestion, + }], } .into()); } @@ -407,10 +412,13 @@ impl<'a> DistributableToolchain<'a> { } .into()); } - return Err(RustupError::UnknownComponent { + return Err(RustupError::UnknownComponents { desc: self.desc.clone(), - component: manifest.description(&component), - suggestion, + components: vec![UnknownComponentInfo { + name: manifest.short_name(&component).to_string(), + description: manifest.description(&component), + suggestion, + }], } .into()); } diff --git a/tests/suite/cli_rustup.rs b/tests/suite/cli_rustup.rs index f67d5f0d8e..b658d0007b 100644 --- a/tests/suite/cli_rustup.rs +++ b/tests/suite/cli_rustup.rs @@ -1,14 +1,19 @@ //! Test cases for new rustup UI -use std::fs; -use std::path::PathBuf; -use std::{env::consts::EXE_SUFFIX, path::Path}; +use std::{ + env::consts::EXE_SUFFIX, + fs, + path::{Path, PathBuf}, +}; -use rustup::test::{ - CROSS_ARCH1, CROSS_ARCH2, CliTestContext, MULTI_ARCH1, Scenario, this_host_triple, - topical_doc_data, +use rustup::{ + for_host, + test::{ + CROSS_ARCH1, CROSS_ARCH2, CliTestContext, MULTI_ARCH1, Scenario, this_host_triple, + topical_doc_data, + }, + utils::raw, }; -use rustup::utils::raw; #[tokio::test] async fn rustup_stable() { @@ -2136,6 +2141,96 @@ async fn add_remove_multiple_components() { } } +#[tokio::test] +async fn remove_multiple_components_best_effort() { + let files = [ + "lib/rustlib/src/rust-src/foo.rs", + for_host!("lib/rustlib/{}/analysis/libfoo.json"), + ]; + + for (one, two, three) in [ + ("bad-component", "rust-src", "rust-analysis"), + ("rust-src", "bad-component", "rust-analysis"), + ("rust-src", "rust-analysis", "bad-component"), + ] { + 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 = for_host!("toolchains/nightly-{0}/{file}"); + assert!(cx.config.rustupdir.has(path)); + } + + cx.config + .expect(&["rustup", "component", "remove", one, two, three]) + .await + .is_err(); + + for file in &files { + let path = PathBuf::from(for_host!("toolchains/nightly-{0}/{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", + for_host!("lib/rustlib/{}/analysis/libfoo.json"), + ]; + + 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 = for_host!("toolchains/nightly-{0}/{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(); + + for file in &files { + let path = PathBuf::from(for_host!("toolchains/nightly-{0}/{file}",)); + assert!(!cx.config.rustupdir.has(path.parent().unwrap())); + } +} + #[tokio::test] async fn file_override() { let cx = CliTestContext::new(Scenario::SimpleV2).await; diff --git a/tests/suite/cli_v2.rs b/tests/suite/cli_v2.rs index bff61ff1a0..3099c3b194 100644 --- a/tests/suite/cli_v2.rs +++ b/tests/suite/cli_v2.rs @@ -2133,7 +2133,8 @@ async fn add_component_suggest_best_match() { .expect(["rustup", "component", "add", "rsl"]) .await .with_stderr(snapbox::str![[r#" -error: toolchain 'nightly-[HOST_TRIPLE]' does not contain component 'rsl' for target '[HOST_TRIPLE]'; did you mean 'rls'? +error: toolchain 'nightly-[HOST_TRIPLE]' does not contain component 'rsl' for target '[HOST_TRIPLE]' +help: did you mean 'rls'? "#]]) .is_err(); @@ -2141,7 +2142,8 @@ error: toolchain 'nightly-[HOST_TRIPLE]' does not contain component 'rsl' for ta .expect(["rustup", "component", "add", "rsl-preview"]) .await .with_stderr(snapbox::str![[r#" -error: toolchain 'nightly-[HOST_TRIPLE]' does not contain component 'rsl-preview' for target '[HOST_TRIPLE]'; did you mean 'rls-preview'? +error: toolchain 'nightly-[HOST_TRIPLE]' does not contain component 'rsl-preview' for target '[HOST_TRIPLE]' +help: did you mean 'rls-preview'? "#]]) .is_err(); @@ -2149,7 +2151,8 @@ error: toolchain 'nightly-[HOST_TRIPLE]' does not contain component 'rsl-preview .expect(["rustup", "component", "add", "rustd"]) .await .with_stderr(snapbox::str![[r#" -error: toolchain 'nightly-[HOST_TRIPLE]' does not contain component 'rustd' for target '[HOST_TRIPLE]'; did you mean 'rustc'? +error: toolchain 'nightly-[HOST_TRIPLE]' does not contain component 'rustd' for target '[HOST_TRIPLE]' +help: did you mean 'rustc'? "#]]) .is_err(); @@ -2180,7 +2183,8 @@ async fn remove_component_suggest_best_match() { .expect(["rustup", "component", "remove", "rsl"]) .await .with_stderr(snapbox::str![[r#" -error: toolchain 'nightly-[HOST_TRIPLE]' does not contain component 'rsl' for target '[HOST_TRIPLE]'; did you mean 'rls'? +error: toolchain 'nightly-[HOST_TRIPLE]' does not contain component 'rsl' for target '[HOST_TRIPLE]' +help: did you mean 'rls'? "#]]) .is_err(); @@ -2192,7 +2196,8 @@ error: toolchain 'nightly-[HOST_TRIPLE]' does not contain component 'rsl' for ta .expect(["rustup", "component", "add", "rsl-preview"]) .await .with_stderr(snapbox::str![[r#" -error: toolchain 'nightly-[HOST_TRIPLE]' does not contain component 'rsl-preview' for target '[HOST_TRIPLE]'; did you mean 'rls-preview'? +error: toolchain 'nightly-[HOST_TRIPLE]' does not contain component 'rsl-preview' for target '[HOST_TRIPLE]' +help: did you mean 'rls-preview'? "#]]) .is_err(); @@ -2200,7 +2205,8 @@ error: toolchain 'nightly-[HOST_TRIPLE]' does not contain component 'rsl-preview .expect(["rustup", "component", "remove", "rustd"]) .await .with_stderr(snapbox::str![[r#" -error: toolchain 'nightly-[HOST_TRIPLE]' does not contain component 'rustd' for target '[HOST_TRIPLE]'; did you mean 'rustc'? +error: toolchain 'nightly-[HOST_TRIPLE]' does not contain component 'rustd' for target '[HOST_TRIPLE]' +help: did you mean 'rustc'? "#]]) .is_err();