Skip to content

Manifest names #4600

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Manifest names #4600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aa3ec32
Move Component name helpers to Manifest
djc Nov 14, 2025
7a3545b
Yield references from Manifest::short_name()
djc Nov 14, 2025
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
69 changes: 37 additions & 32 deletions src/dist/manifest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,34 @@ pub struct Manifest {
pub profiles: HashMap<Profile, Vec<String>>,
}

impl Manifest {
pub(crate) fn name(&self, component: &Component) -> String {
let pkg = self.short_name(component);
if let Some(t) = &component.target {
format!("{pkg}-{t}")
} else {
pkg.to_owned()
}
}

pub(crate) fn description(&self, component: &Component) -> String {
let pkg = self.short_name(component);
if let Some(t) = &component.target {
format!("'{pkg}' for target '{t}'")
} else {
format!("'{pkg}'")
}
}

pub(crate) fn short_name<'a>(&'a self, component: &'a Component) -> &'a str {
if let Some(from) = self.reverse_renames.get(&component.pkg) {
from
} else {
&component.pkg
}
}
}

#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
pub struct Renamed {
pub to: String,
Expand Down Expand Up @@ -344,12 +372,12 @@ impl Manifest {

fn validate_targeted_package(&self, tpkg: &TargetedPackage) -> Result<()> {
for c in tpkg.components.iter() {
let cpkg = self
.get_package(&c.pkg)
.with_context(|| RustupError::MissingPackageForComponent(c.short_name(self)))?;
let _ctpkg = cpkg
.get_target(c.target.as_ref())
.with_context(|| RustupError::MissingPackageForComponent(c.short_name(self)))?;
let cpkg = self.get_package(&c.pkg).with_context(|| {
RustupError::MissingPackageForComponent(self.short_name(c).to_owned())
})?;
let _ctpkg = cpkg.get_target(c.target.as_ref()).with_context(|| {
RustupError::MissingPackageForComponent(self.short_name(c).to_owned())
})?;
}
Ok(())
}
Expand Down Expand Up @@ -423,7 +451,7 @@ impl Manifest {
.unwrap_or_else(|_| {
panic!(
"manifest should contain component {}",
&component.short_name(self)
&self.short_name(component)
)
});
let component_target_pkg = component_pkg
Expand All @@ -433,7 +461,7 @@ impl Manifest {

res.push(ComponentStatus {
component: component.clone(),
name: component.name(self),
name: self.name(component),
installed,
available: component_target_pkg.available(),
});
Expand Down Expand Up @@ -501,7 +529,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 == component.name(&manifest) {
if name == component.name_in_manifest() || name == manifest.name(&component) {
return Ok(component);
}
}
Expand All @@ -521,29 +549,6 @@ impl Component {
}
}

pub(crate) fn name(&self, manifest: &Manifest) -> String {
let pkg = self.short_name(manifest);
if let Some(t) = &self.target {
format!("{pkg}-{t}")
} else {
pkg
}
}
pub(crate) fn short_name(&self, manifest: &Manifest) -> String {
if let Some(from) = manifest.reverse_renames.get(&self.pkg) {
from.to_owned()
} else {
self.pkg.clone()
}
}
pub(crate) fn description(&self, manifest: &Manifest) -> String {
let pkg = self.short_name(manifest);
if let Some(t) = &self.target {
format!("'{pkg}' for target '{t}'")
} else {
format!("'{pkg}'")
}
}
pub fn short_name_in_manifest(&self) -> &String {
&self.pkg
}
Expand Down
26 changes: 13 additions & 13 deletions src/dist/manifestation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,12 @@ impl Manifestation {
match &component.target {
Some(t) if t != &self.target_triple => warn!(
"skipping unavailable component {} for target {}",
component.short_name(new_manifest),
new_manifest.short_name(component),
t
),
_ => warn!(
"skipping unavailable component {}",
component.short_name(new_manifest)
new_manifest.short_name(component)
),
}
}
Expand All @@ -155,7 +155,7 @@ impl Manifestation {
res.map(|(component, binary)| ComponentBinary {
component,
binary,
status: download_cfg.status_for(component.short_name(new_manifest)),
status: download_cfg.status_for(new_manifest.short_name(component).to_owned()),
manifest: new_manifest,
download_cfg,
})
Expand Down Expand Up @@ -189,25 +189,25 @@ impl Manifestation {
(true, Some(t)) if t != &self.target_triple => {
info!(
"removing previous version of component {} for target {}",
component.short_name(new_manifest),
new_manifest.short_name(component),
t
);
}
(false, Some(t)) if t != &self.target_triple => {
info!(
"removing component {} for target {}",
component.short_name(new_manifest),
new_manifest.short_name(component),
t
);
}
(true, _) => {
info!(
"removing previous version of component {}",
component.short_name(new_manifest),
new_manifest.short_name(component),
);
}
(false, _) => {
info!("removing component {}", component.short_name(new_manifest));
info!("removing component {}", new_manifest.short_name(component));
}
}

Expand Down Expand Up @@ -314,7 +314,7 @@ impl Manifestation {
} else {
warn!(
"component {} not found during uninstall",
component.short_name(manifest),
manifest.short_name(component),
);
}

Expand Down Expand Up @@ -524,12 +524,12 @@ impl Update {
match &component.target {
Some(t) if t != &manifestation.target_triple => info!(
"component {} for target {} is up to date",
component.short_name(new_manifest),
new_manifest.short_name(component),
t,
),
_ => info!(
"component {} is up to date",
component.short_name(new_manifest)
new_manifest.short_name(component)
),
}
}
Expand Down Expand Up @@ -699,7 +699,7 @@ impl<'a> ComponentBinary<'a> {
)
.await
.with_context(|| {
RustupError::ComponentDownloadFailed(self.component.name(self.manifest))
RustupError::ComponentDownloadFailed(self.manifest.name(self.component))
})?;

Ok((self, downloaded_file))
Expand All @@ -718,7 +718,7 @@ impl<'a> ComponentBinary<'a> {
let component = self.component;
let pkg_name = component.name_in_manifest();
let short_pkg_name = component.short_name_in_manifest();
let short_name = component.short_name(self.manifest);
let short_name = self.manifest.short_name(component);

self.status.installing();

Expand All @@ -729,7 +729,7 @@ impl<'a> ComponentBinary<'a> {
// If the package doesn't contain the component that the
// manifest says it does then somebody must be playing a joke on us.
if !package.contains(&pkg_name, Some(short_pkg_name)) {
return Err(RustupError::CorruptComponent(short_name).into());
return Err(RustupError::CorruptComponent(short_name.to_owned()).into());
}

let tx = package.install(
Expand Down
10 changes: 5 additions & 5 deletions src/dist/manifestation/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ async fn unavailable_component() {
assert_eq!(toolchain, "nightly");
let descriptions = components
.iter()
.map(|c| c.description(&manifest))
.map(|c| manifest.description(c))
.collect::<Vec<_>>();
assert_eq!(descriptions, ["'bonus' for target 'x86_64-apple-darwin'"])
}
Expand Down Expand Up @@ -703,7 +703,7 @@ async fn unavailable_component_from_profile() {
assert_eq!(toolchain, "nightly");
let descriptions = components
.iter()
.map(|c| c.description(&manifest))
.map(|c| manifest.description(&c))
.collect::<Vec<_>>();
assert_eq!(descriptions, ["'rustc' for target 'x86_64-apple-darwin'"])
}
Expand Down Expand Up @@ -757,7 +757,7 @@ async fn removed_component() {
assert_eq!(toolchain, "nightly");
let descriptions = components
.iter()
.map(|c| c.description(&manifest))
.map(|c| manifest.description(c))
.collect::<Vec<_>>();
assert_eq!(descriptions, ["'bonus' for target 'x86_64-apple-darwin'"])
}
Expand Down Expand Up @@ -821,7 +821,7 @@ async fn unavailable_components_is_target() {
assert_eq!(toolchain, "nightly");
let descriptions = components
.iter()
.map(|c| c.description(&manifest))
.map(|c| manifest.description(c))
.collect::<Vec<_>>();
assert_eq!(
descriptions,
Expand Down Expand Up @@ -886,7 +886,7 @@ async fn unavailable_components_with_same_target() {
assert_eq!(toolchain, "nightly");
let descriptions = components
.iter()
.map(|c| c.description(&manifest))
.map(|c| manifest.description(c))
.collect::<Vec<_>>();
assert_eq!(
descriptions,
Expand Down
8 changes: 4 additions & 4 deletions src/dist/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,14 @@ fn components_missing_msg(cs: &[Component], manifest: &ManifestV2, toolchain: &s
let _ = writeln!(
buf,
"component {} is unavailable for download for channel '{}'",
c.description(manifest),
manifest.description(c),
toolchain,
);
}
cs => {
let cs_str = cs
.iter()
.map(|c| c.description(manifest))
.map(|c| manifest.description(c))
.collect::<Vec<_>>()
.join(", ");
let _ = write!(
Expand Down Expand Up @@ -1048,8 +1048,8 @@ impl<'cfg, 'a> DistOptions<'cfg, 'a> {
.iter()
.map(|component| {
match component.target.as_ref() == Some(&toolchain.target) {
true => component.short_name(manifest),
false => component.name(manifest),
true => manifest.short_name(component),
false => manifest.short_name(component),
}
})
.join(", ");
Expand Down
6 changes: 3 additions & 3 deletions src/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ fn component_unavailable_msg(cs: &[Component], manifest: &Manifest, toolchain: &
let _ = writeln!(
buf,
"component {} is unavailable for download for channel '{}'",
c.description(manifest),
manifest.description(c),
toolchain,
);

Expand All @@ -217,12 +217,12 @@ fn component_unavailable_msg(cs: &[Component], manifest: &Manifest, toolchain: &

let cs_str = if same_target {
cs.iter()
.map(|c| format!("'{}'", c.short_name(manifest)))
.map(|c| format!("'{}'", manifest.short_name(c)))
.collect::<Vec<_>>()
.join(", ")
} else {
cs.iter()
.map(|c| c.description(manifest))
.map(|c| manifest.description(c))
.collect::<Vec<_>>()
.join(", ")
};
Expand Down
25 changes: 13 additions & 12 deletions src/toolchain/distributable.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ impl<'a> DistributableToolchain<'a> {
}
return Err(RustupError::UnknownComponent {
desc,
component: component.description(&manifest),
component: manifest.short_name(&component).to_owned(),
suggestion,
}
.into());
Expand Down Expand Up @@ -169,8 +169,7 @@ impl<'a> DistributableToolchain<'a> {
// check if all the components we want are installed
let wanted_components = components.iter().all(|name| {
installed_components.iter().any(|status| {
let cname = status.component.short_name(&manifest);
let cname = cname.as_str();
let cname = manifest.short_name(&status.component);
let cnameim = status.component.short_name_in_manifest();
let cnameim = cnameim.as_str();
(cname == *name || cnameim == *name) && status.installed
Expand Down Expand Up @@ -259,8 +258,8 @@ impl<'a> DistributableToolchain<'a> {
.map(|c| {
(
damerau_levenshtein(
&c.component.name(manifest)[..],
&component.name(manifest)[..],
&manifest.name(&c.component)[..],
&manifest.name(component)[..],
),
c,
)
Expand All @@ -275,7 +274,7 @@ impl<'a> DistributableToolchain<'a> {
(
damerau_levenshtein(
&c.component.name_in_manifest()[..],
&component.name(manifest)[..],
&manifest.name(component)[..],
),
c,
)
Expand All @@ -284,7 +283,9 @@ impl<'a> DistributableToolchain<'a> {
.expect("There should be always at least one component");

let mut closest_distance = short_name_distance;
let mut closest_match = short_name_distance.1.component.short_name(manifest);
let mut closest_match = manifest
.short_name(&short_name_distance.1.component)
.to_owned();

// Find closer suggestion
if short_name_distance.0 > long_name_distance.0 {
Expand All @@ -304,8 +305,8 @@ impl<'a> DistributableToolchain<'a> {
}
} else {
// Check if only targets differ
if closest_distance.1.component.short_name(manifest)
== component.short_name(manifest)
if manifest.short_name(&closest_distance.1.component)
== manifest.short_name(component)
{
closest_match = short_name_distance.1.component.target();
}
Expand Down Expand Up @@ -355,9 +356,9 @@ impl<'a> DistributableToolchain<'a> {
if let Some(component_name) = component_for_bin(&binary_lossy) {
let component_status = component_statuses
.iter()
.find(|cs| cs.component.short_name(&manifest) == component_name)
.find(|cs| manifest.short_name(&cs.component) == component_name)
.ok_or_else(|| anyhow!("component {component_name} should be in the manifest"))?;
let short_name = component_status.component.short_name(&manifest);
let short_name = manifest.short_name(&component_status.component);
if !component_status.available {
Err(anyhow!(
"the '{short_name}' component which provides the command '{binary_lossy}' is not available for the '{desc}' toolchain"
Expand Down Expand Up @@ -417,7 +418,7 @@ impl<'a> DistributableToolchain<'a> {
}
return Err(RustupError::UnknownComponent {
desc: self.desc.clone(),
component: component.description(&manifest),
component: manifest.short_name(&component).to_owned(),
suggestion,
}
.into());
Expand Down
Loading