From d0df3172c85f4761986a9aba754529e87a48b7cc Mon Sep 17 00:00:00 2001 From: Cloud0310 <60375730+Cloud0310@users.noreply.github.com> Date: Fri, 27 Mar 2026 01:51:11 +0800 Subject: [PATCH 1/2] refactor: Rename `Component`'s `name_in_manifest` to `name` and `short_name` accordingly In `Component`, these methods are not related to the final name associated with manifest. On the `Manifest` side, it have `name` and `short_name` for get the re-mapped component names. So, I think renaming these func names in Component helps clarify the the code. --- src/cli/rustup_mode.rs | 10 ++++------ src/dist/manifest.rs | 16 ++++++++-------- src/dist/manifestation.rs | 11 +++++------ src/dist/mod.rs | 2 +- src/toolchain/distributable.rs | 19 +++++-------------- 5 files changed, 23 insertions(+), 35 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index f1b8f986c8..3fce625a33 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -1144,7 +1144,7 @@ async fn show(cfg: &Cfg<'_>, verbose: bool) -> Result { .components()? .into_iter() .filter_map(|c| { - (c.installed && c.component.short_name_in_manifest() == "rust-std") + (c.installed && c.component.short_name() == "rust-std") .then(|| c.component.target.expect("rust-std should have a target")) }) .collect(), @@ -1287,7 +1287,7 @@ async fn target_list( if let Ok(distributable) = DistributableToolchain::from_partial(toolchain.clone(), cfg).await { common::list_items( distributable.components()?.into_iter().filter_map(|c| { - if c.component.short_name_in_manifest() == "rust-std" && c.available { + if c.component.short_name() == "rust-std" && c.available { c.component.target.map(|target| (target, c.installed)) } else { None @@ -1335,7 +1335,7 @@ async fn target_add( targets.clear(); for component in components { - if component.component.short_name_in_manifest() == "rust-std" + if component.component.short_name() == "rust-std" && component.available && !component.installed { @@ -1763,9 +1763,7 @@ async fn doc( && let [_] = distributable .components()? .into_iter() - .filter(|cstatus| { - cstatus.component.short_name_in_manifest() == "rust-docs" && !cstatus.installed - }) + .filter(|cstatus| cstatus.component.short_name() == "rust-docs" && !cstatus.installed) .take(1) .collect::>() .as_slice() diff --git a/src/dist/manifest.rs b/src/dist/manifest.rs index b58bf0341b..0526748419 100644 --- a/src/dist/manifest.rs +++ b/src/dist/manifest.rs @@ -317,7 +317,7 @@ impl Manifest { } pub(super) fn binary(&self, component: &Component) -> Result> { - let package = self.get_package(component.short_name_in_manifest())?; + let package = self.get_package(component.short_name())?; let target_package = package.get_target(component.target.as_ref())?; // We prefer the first format in the list, since the parsing of the // manifest leaves us with the files/hash pairs in preference order. @@ -456,7 +456,7 @@ impl Manifest { // Get the component so we can check if it is available let component_pkg = self - .get_package(component.short_name_in_manifest()) + .get_package(component.short_name()) .unwrap_or_else(|_| { panic!( "manifest should contain component {}", @@ -538,7 +538,7 @@ impl Component { let manifest = distributable.get_manifest()?; for component_status in distributable.components()? { let component = component_status.component; - if name == component.name_in_manifest() || name == manifest.name(&component) { + if name == component.name() || name == manifest.name(&component) { return Ok(component); } } @@ -558,11 +558,11 @@ impl Component { } } - pub fn short_name_in_manifest(&self) -> &String { + pub fn short_name(&self) -> &String { &self.pkg } - pub(crate) fn name_in_manifest(&self) -> String { - let pkg = self.short_name_in_manifest(); + pub(crate) fn name(&self) -> String { + let pkg = self.short_name(); if let Some(t) = &self.target { format!("{pkg}-{t}") } else { @@ -670,11 +670,11 @@ mod tests { assert_eq!(rust_target_pkg.bins[0].hash, "..."); let component = &rust_target_pkg.components[0]; - assert_eq!(component.short_name_in_manifest(), "rustc"); + assert_eq!(component.short_name(), "rustc"); assert_eq!(component.target.as_ref(), Some(&x86_64_unknown_linux_gnu)); let component = &rust_target_pkg.components[4]; - assert_eq!(component.short_name_in_manifest(), "rust-std"); + assert_eq!(component.short_name(), "rust-std"); assert_eq!(component.target.as_ref(), Some(&x86_64_unknown_linux_musl)); let docs_pkg = pkg.get_package("rust-docs").unwrap(); diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index 1995572ec8..4b97074efb 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -336,8 +336,8 @@ impl Manifestation { // names are not the same as the dist manifest component // names. Some are just the component name some are the // component name plus the target tuple. - let name = component.name_in_manifest(); - let short_name = component.short_name_in_manifest(); + let name = component.name(); + let short_name = component.short_name(); if let Some(c) = self.installation.find(&name)? { tx = c.uninstall(tx)?; } else if let Some(c) = self.installation.find(short_name)? { @@ -712,8 +712,7 @@ impl Update { .iter() .filter(|c| { use crate::dist::manifest::{Package, TargetedPackage}; - let pkg: Option<&Package> = - new_manifest.get_package(c.short_name_in_manifest()).ok(); + let pkg: Option<&Package> = new_manifest.get_package(c.short_name()).ok(); let target_pkg: Option<&TargetedPackage> = pkg.and_then(|p| p.get_target(c.target.as_ref()).ok()); target_pkg.map(TargetedPackage::available) != Some(true) @@ -827,8 +826,8 @@ impl ComponentInstall { // names are not the same as the dist manifest component // names. Some are just the component name some are the // component name plus the target tuple. - let pkg_name = self.component.name_in_manifest(); - let short_pkg_name = self.component.short_name_in_manifest(); + let pkg_name = self.component.name(); + let short_pkg_name = self.component.short_name(); let reader = self.status.unpack(utils::buffered(&self.installer)?); let package = DirectoryPackage::compressed( reader, diff --git a/src/dist/mod.rs b/src/dist/mod.rs index d3d822bf6a..bf1e6b3148 100644 --- a/src/dist/mod.rs +++ b/src/dist/mod.rs @@ -1175,7 +1175,7 @@ async fn try_update_from_dist_( if let Some(c) = rust_target_package .components .iter() - .find(|c| c.short_name_in_manifest() == component.short_name_in_manifest()) + .find(|c| c.short_name() == component.short_name()) && c.target.is_none() { component = component.wildcard(); diff --git a/src/toolchain/distributable.rs b/src/toolchain/distributable.rs index e05cff3faa..c96fb8124e 100644 --- a/src/toolchain/distributable.rs +++ b/src/toolchain/distributable.rs @@ -163,7 +163,7 @@ impl<'a> DistributableToolchain<'a> { let wanted_components = components.iter().all(|name| { installed_components.iter().any(|status| { let cname = manifest.short_name(&status.component); - let cnameim = status.component.short_name_in_manifest(); + let cnameim = status.component.short_name(); let cnameim = cnameim.as_str(); (cname == *name || cnameim == *name) && status.installed }) @@ -172,7 +172,7 @@ impl<'a> DistributableToolchain<'a> { let wanted_targets = targets.iter().all(|name| { installed_components .iter() - .filter(|c| c.component.short_name_in_manifest() == "rust-std") + .filter(|c| c.component.short_name() == "rust-std") .any(|status| { let ctarg = status.component.target(); (ctarg == *name) && status.installed @@ -265,10 +265,7 @@ impl<'a> DistributableToolchain<'a> { .filter(|c| !only_installed || c.installed) .map(|c| { ( - damerau_levenshtein( - &c.component.name_in_manifest()[..], - &manifest.name(component)[..], - ), + damerau_levenshtein(&c.component.name()[..], &manifest.name(component)[..]), c, ) }) @@ -285,16 +282,10 @@ impl<'a> DistributableToolchain<'a> { closest_distance = long_name_distance; // Check if only targets differ - if closest_distance.1.component.short_name_in_manifest() - == component.short_name_in_manifest() - { + if closest_distance.1.component.short_name() == component.short_name() { closest_match = long_name_distance.1.component.target(); } else { - closest_match = long_name_distance - .1 - .component - .short_name_in_manifest() - .to_string(); + closest_match = long_name_distance.1.component.short_name().to_string(); } } else { // Check if only targets differ From ec817490bb8b7b8f35a4a5b744b746b2f169a39e Mon Sep 17 00:00:00 2001 From: Cloud0310 <60375730+Cloud0310@users.noreply.github.com> Date: Sat, 28 Mar 2026 20:39:54 +0800 Subject: [PATCH 2/2] chore(doc): Added comments for clarify the usage of `Component::name` `Manifest::name` and the `short_name` funcc accordingly. --- src/dist/manifest.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/dist/manifest.rs b/src/dist/manifest.rs index 0526748419..aa8ad0e354 100644 --- a/src/dist/manifest.rs +++ b/src/dist/manifest.rs @@ -52,6 +52,8 @@ pub struct Manifest { } impl Manifest { + /// Returns the [`Component`]'s name **before** renaming, including the target if present. + /// For the name after renaming, see [`Component::name()`]. pub(crate) fn name(&self, component: &Component) -> String { let pkg = self.short_name(component); if let Some(t) = &component.target { @@ -70,6 +72,8 @@ impl Manifest { } } + /// Returns the [`Component`]'s short name (not including the target) **before** renaming. + /// For the name after renaming, see [`Component::short_name()`]. pub(crate) fn short_name<'a>(&'a self, component: &'a Component) -> &'a str { if let Some(from) = self.reverse_renames.get(&component.pkg) { from @@ -558,9 +562,13 @@ impl Component { } } + /// Returns the [`Component`]'s short name (not including the target) **after** renaming. + /// For the name before renaming, see [`Manifest::short_name()`]. pub fn short_name(&self) -> &String { &self.pkg } + /// Returns the [`Component`]'s name **after** renaming, including the target if present. + /// For the short name before renaming, see [`Manifest::name()`]. pub(crate) fn name(&self) -> String { let pkg = self.short_name(); if let Some(t) = &self.target {