Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 50 additions & 3 deletions src/cli/rustup_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
31 changes: 30 additions & 1 deletion src/errors.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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\
Expand Down Expand Up @@ -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),
Comment on lines +259 to +260
Copy link
Copy Markdown
Contributor

@FranciscoTGouveia FranciscoTGouveia Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

View changes since the review

What if we insert a newline before calling suggest_message?
Otherwise, we could easily exceed 80 chars :/

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I will update this.

);
}
components => {
let _ = writeln!(buf, "toolchain '{desc}' does not contain these components:");

for (component, suggestion) in components {
let _ = writeln!(buf, " - '{component}'{}", suggest_message(suggestion),);
}
}
}

buf
}
139 changes: 139 additions & 0 deletions tests/suite/cli_rustup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
}
Copy link
Copy Markdown
Contributor

@FranciscoTGouveia FranciscoTGouveia Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

View changes since the review

There should also be a third case in which a valid component appears between invalid ones, ensuring that errors are correctly reported for both invalid components.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That’s a useful edge case to cover. I will add it to the test.


#[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;
Expand Down
Loading