From 898303adab4f3fbc396ac4f5576864907ec300b5 Mon Sep 17 00:00:00 2001 From: rami3l Date: Tue, 6 Oct 2026 18:20:49 +0200 Subject: [PATCH 1/9] style(docs): simplify log formatting --- src/cli/docs.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cli/docs.rs b/src/cli/docs.rs index 7e514c5b51..d94520ff0d 100644 --- a/src/cli/docs.rs +++ b/src/cli/docs.rs @@ -201,9 +201,8 @@ pub(crate) async fn doc( .as_slice() { info!( - "`rust-docs` not installed in toolchain `{}`\nhelp: run `rustup component add --toolchain {} rust-docs` to install it", + "`rust-docs` not installed in toolchain `{0}`\nhelp: run `rustup component add --toolchain {0} rust-docs` to install it", distributable.desc(), - distributable.desc() ); return Err(anyhow!( "unable to view documentation which is not installed" From ee6f7deb20325b0fcd6853bb271b1b9581afd14f Mon Sep 17 00:00:00 2001 From: rami3l Date: Wed, 7 Oct 2026 10:28:39 +0200 Subject: [PATCH 2/9] refactor(docs): stop returning a `Result<>` from `Toolchain::doc_path()` --- src/cli/docs.rs | 8 ++++---- src/toolchain.rs | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/cli/docs.rs b/src/cli/docs.rs index d94520ff0d..6f906b9e0a 100644 --- a/src/cli/docs.rs +++ b/src/cli/docs.rs @@ -212,12 +212,12 @@ pub(crate) async fn doc( let (doc_path, fragment) = match (topic, doc_page.name()) { (Some(topic), Some(name)) => { let (doc_path, fragment) = doc_page - .resolve(&toolchain.doc_path("")?, topic) + .resolve(&toolchain.doc_path(""), topic) .context(format!("no document for {name} on {topic}"))?; (Cow::Owned(doc_path), fragment) } (Some(topic), None) => { - let doc_path = topical_doc::local_path(&toolchain.doc_path("").unwrap(), topic)?; + let doc_path = topical_doc::local_path(&toolchain.doc_path(""), topic)?; (Cow::Owned(doc_path), None) } (None, name) => { @@ -228,13 +228,13 @@ pub(crate) async fn doc( }; if path_only { - let doc_path = toolchain.doc_path(&doc_path)?; + let doc_path = toolchain.doc_path(&doc_path); writeln!(cfg.process.stdout().lock(), "{}", doc_path.display())?; return Ok(ExitCode::SUCCESS); } if serve { - let root = toolchain.doc_path("")?; + let root = toolchain.doc_path(""); serve_and_open(root, &doc_path, fragment).await?; return Ok(ExitCode::SUCCESS); } diff --git a/src/toolchain.rs b/src/toolchain.rs index 82fd040fc1..ea2648476f 100644 --- a/src/toolchain.rs +++ b/src/toolchain.rs @@ -504,17 +504,17 @@ impl<'a> Toolchain<'a> { buf } - pub fn doc_path(&self, relative: impl AsRef) -> anyhow::Result { + pub fn doc_path(&self, relative: impl AsRef) -> PathBuf { let relative = relative.as_ref(); if relative.is_absolute() { - return Ok(relative.to_owned()); + return relative.to_owned(); } let mut doc_dir = self.path.clone(); doc_dir.extend(["share", "doc", "rust", "html"]); doc_dir.push(relative); - Ok(doc_dir) + doc_dir } pub fn open_docs( @@ -523,7 +523,7 @@ impl<'a> Toolchain<'a> { fragment: Option<&str>, ) -> anyhow::Result<()> { let relative = relative.as_ref(); - let mut doc_url = Url::from_file_path(self.doc_path(relative)?) + let mut doc_url = Url::from_file_path(self.doc_path(relative)) .ok() .with_context(|| anyhow!("invalid doc file absolute path `{}`", relative.display()))?; doc_url.set_fragment(fragment); From fcb87f8ceedb230b01fc4cc5955162dee8e45029 Mon Sep 17 00:00:00 2001 From: rami3l Date: Tue, 6 Oct 2026 17:51:26 +0200 Subject: [PATCH 3/9] refactor(toolchain): rename `DistributableToolchain` to `ChannelToolchain` --- src/cli/docs.rs | 4 ++-- src/cli/rustup_mode.rs | 32 ++++++++++++++------------------ src/cli/self_update.rs | 8 +++----- src/config.rs | 12 ++++++------ src/dist/manifest.rs | 4 ++-- src/dist/mod.rs | 4 ++-- src/toolchain.rs | 8 ++++---- src/toolchain/distributable.rs | 10 +++++----- 8 files changed, 38 insertions(+), 44 deletions(-) diff --git a/src/cli/docs.rs b/src/cli/docs.rs index 6f906b9e0a..9625052f04 100644 --- a/src/cli/docs.rs +++ b/src/cli/docs.rs @@ -35,7 +35,7 @@ use super::topical_doc; use crate::{ config::{ActiveSource, Cfg}, dist::{PartialChannelToolchainName, manifest::ComponentStatus}, - toolchain::DistributableToolchain, + toolchain::ChannelToolchain, utils::{self, ExitCode}, }; @@ -191,7 +191,7 @@ pub(crate) async fn doc( let toolchain = toolchain.map(|desc| (desc, ActiveSource::CommandLine)); let toolchain = cfg.toolchain_from_partial(toolchain).await?.0; - if let Ok(distributable) = DistributableToolchain::try_from(&toolchain) + if let Ok(distributable) = ChannelToolchain::try_from(&toolchain) && let [_] = distributable .components()? .into_iter() diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 6940cbc2a7..362adac5fd 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -53,7 +53,7 @@ use crate::{ install::{InstallMethod, UpdateStatus}, process::{ColorableTerminal, Process}, toolchain::{ - CustomToolchainName, DistributableToolchain, MaybePartialToolchainName, Override, + ChannelToolchain, CustomToolchainName, MaybePartialToolchainName, Override, PartialToolchainName, PartialToolchainNameOrPath, Toolchain, ToolchainName, ToolchainNameOrPath, }, @@ -911,7 +911,7 @@ fn completion_command(cfg: &Cfg<'_>) -> clap::Command { let ToolchainName::Channel(desc) = name else { return None; }; - let dist = DistributableToolchain::new(cfg, desc.clone()).ok()?; + let dist = ChannelToolchain::new(cfg, desc.clone()).ok()?; let components = dist.components().ok()?; Some( components @@ -1183,7 +1183,7 @@ async fn update( cfg, )?; - let status = match DistributableToolchain::new(cfg, desc.clone()) { + let status = match ChannelToolchain::new(cfg, desc.clone()) { Ok(d) => { if !opts.no_update { InstallMethod::Dist(dist_opts.for_update(&d, opts.allow_downgrade)) @@ -1194,7 +1194,7 @@ async fn update( } } Err(RustupError::ToolchainNotInstalled { .. }) => { - DistributableToolchain::install(dist_opts).await?.status + ChannelToolchain::install(dist_opts).await?.status } Err(e) => Err(e)?, }; @@ -1391,7 +1391,7 @@ async fn show(cfg: &Cfg<'_>, verbose: bool) -> anyhow::Result { writeln!(t.lock(), "installed targets:")?; let active_toolchain_targets = match active_toolchain_name { - ToolchainName::Channel(desc) => DistributableToolchain::new(cfg, desc)? + ToolchainName::Channel(desc) => ChannelToolchain::new(cfg, desc)? .components()? .into_iter() .filter_map(|c| { @@ -1467,7 +1467,7 @@ async fn target_list( // If a toolchain is Distributable, we can assume it has a manifest and thus print all possible targets and the installed ones. // However, if it is a custom toolchain, we can only print the installed targets. // NB: this decision is made based on the absence of a manifest in custom toolchains. - if let Ok(distributable) = DistributableToolchain::from_partial(toolchain.clone(), cfg).await { + if let Ok(distributable) = ChannelToolchain::from_partial(toolchain.clone(), cfg).await { common::list_items( distributable.components()?.into_iter().filter_map(|c| { if c.component.short_name() == "rust-std" && c.available { @@ -1501,7 +1501,7 @@ async fn target_add( // isn't a feature yet. // list_components *and* add_components would both be inappropriate for // custom toolchains. - let distributable = DistributableToolchain::from_partial( + let distributable = ChannelToolchain::from_partial( toolchain.map(|desc| (desc, ActiveSource::CommandLine)), cfg, ) @@ -1542,7 +1542,7 @@ async fn target_remove( targets: Vec, toolchain: Option, ) -> anyhow::Result { - let distributable = DistributableToolchain::from_partial( + let distributable = ChannelToolchain::from_partial( toolchain.map(|desc| (desc, ActiveSource::CommandLine)), cfg, ) @@ -1575,7 +1575,7 @@ async fn component_list( let toolchain = toolchain.map(|desc| (desc, ActiveSource::CommandLine)); // downcasting required because the toolchain files can name any toolchain - if let Ok(distributable) = DistributableToolchain::from_partial(toolchain.clone(), cfg).await { + if let Ok(distributable) = ChannelToolchain::from_partial(toolchain.clone(), cfg).await { common::list_items( distributable .components()? @@ -1605,7 +1605,7 @@ async fn component_add( toolchain: Option, target: Option, ) -> anyhow::Result { - let distributable = DistributableToolchain::from_partial( + let distributable = ChannelToolchain::from_partial( toolchain.map(|desc| (desc, ActiveSource::CommandLine)), cfg, ) @@ -1624,10 +1624,7 @@ async fn component_add( Ok(ExitCode::SUCCESS) } -fn get_target( - target: Option, - distributable: &DistributableToolchain<'_>, -) -> Option { +fn get_target(target: Option, distributable: &ChannelToolchain<'_>) -> Option { target .map(TargetTuple::new) .or_else(|| Some(distributable.desc().target.clone())) @@ -1640,7 +1637,7 @@ async fn component_remove( target: Option, ) -> anyhow::Result { let toolchain = toolchain.map(|desc| (desc, ActiveSource::CommandLine)); - let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?; + let distributable = ChannelToolchain::from_partial(toolchain, cfg).await?; let target = get_target(target, &distributable); distributable @@ -1725,8 +1722,7 @@ fn pin_active_toolchain(qualified: bool, cfg: &Cfg<'_>) -> anyhow::Result { - let tc = - DistributableToolchain::new(cfg, desc.clone().complete(&default_host)?)?; + let tc = ChannelToolchain::new(cfg, desc.clone().complete(&default_host)?)?; let manifest = tc.get_manifest()?; Some( @@ -1793,7 +1789,7 @@ async fn override_add( ToolchainName::Custom(_) => Err(e)?, ToolchainName::Channel(desc) => { let options = DistOptions::new(&[], &[], desc, cfg.get_profile()?, false, cfg)?; - let status = DistributableToolchain::install(options).await?.status; + let status = ChannelToolchain::install(options).await?.status; writeln!(cfg.process.stdout().lock())?; common::show_channel_update( cfg, diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index ad2c2f1776..08cc072a2d 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -68,9 +68,7 @@ use crate::{ install::{InstallMethod, UpdateStatus}, process::Process, settings::SettingsFile, - toolchain::{ - DistributableToolchain, MaybeChannelToolchainName, PartialToolchainName, Toolchain, - }, + toolchain::{ChannelToolchain, MaybeChannelToolchainName, PartialToolchainName, Toolchain}, utils::{self, ExitCode}, }; @@ -281,12 +279,12 @@ impl InstallOpts<'_> { // - silently ignore it (and provide inconsistent metadata for reporting the install/update change) // - delete the partial install and start over // For now, we error. - let toolchain = DistributableToolchain::new(&cfg, desc.clone())?; + let toolchain = ChannelToolchain::new(&cfg, desc.clone())?; InstallMethod::Dist(options.for_update(&toolchain, false)) .install(None) .await? } else { - DistributableToolchain::install(options).await?.status + ChannelToolchain::install(options).await?.status }; check_proxy_sanity(&cargo_bin, components, &desc)?; diff --git a/src/config.rs b/src/config.rs index 2038e72d29..854ce5acb2 100644 --- a/src/config.rs +++ b/src/config.rs @@ -25,7 +25,7 @@ use crate::{ process::Process, settings::{MetadataVersion, Settings, SettingsFile}, toolchain::{ - CustomToolchainName, DistributableToolchain, Override, PartialToolchainName, + ChannelToolchain, CustomToolchainName, Override, PartialToolchainName, PartialToolchainNameOrPath, Toolchain, ToolchainName, ToolchainNameOrPath, ToolchainPath, }, utils, @@ -925,9 +925,9 @@ impl<'a> Cfg<'a> { self, )?; - Ok(match DistributableToolchain::new(self, toolchain.clone()) { + Ok(match ChannelToolchain::new(self, toolchain.clone()) { Err(RustupError::ToolchainNotInstalled { .. }) => { - let tc = DistributableToolchain::install(options).await?; + let tc = ChannelToolchain::install(options).await?; EnsureInstalled::new(tc.inner.into(), tc.status) } Ok(distributable) => { @@ -1034,13 +1034,13 @@ impl<'a> Cfg<'a> { pub(crate) fn list_channels( &self, - ) -> anyhow::Result)>> { + ) -> anyhow::Result)>> { let mut channels = self .list_toolchains(true)? .into_iter() .filter_map(|t| match t { ToolchainName::Channel(n) if n.is_tracking() => { - Some(DistributableToolchain::new(self, n.clone()).map(|t| (n, t))) + Some(ChannelToolchain::new(self, n.clone()).map(|t| (n, t))) } _ => None, }) @@ -1114,7 +1114,7 @@ impl<'a> Cfg<'a> { let default_host = self.default_host_tuple()?; let stable_desc = PartialChannelToolchainName::from_str("stable")?.complete(&default_host)?; - let stable = match DistributableToolchain::new(self, stable_desc) { + let stable = match ChannelToolchain::new(self, stable_desc) { Ok(stable) => stable, // If the `stable` toolchain is not installed, we don't notify the user. Err(RustupError::ToolchainNotInstalled { .. }) => return Ok(()), diff --git a/src/dist/manifest.rs b/src/dist/manifest.rs index 15c8fa2fcd..ad22abb03b 100644 --- a/src/dist/manifest.rs +++ b/src/dist/manifest.rs @@ -25,7 +25,7 @@ use serde::{Deserialize, Serialize}; use crate::{ dist::{ChannelToolchainName, DistError, Profile, TargetTuple, config::Config}, errors::RustupError, - toolchain::DistributableToolchain, + toolchain::ChannelToolchain, }; /// A download manifest paired with its respective hash, as returned by the dist server. @@ -558,7 +558,7 @@ impl Component { pub(crate) fn try_new( name: &str, - distributable: &DistributableToolchain<'_>, + distributable: &ChannelToolchain<'_>, fallback_target: Option<&TargetTuple>, ) -> anyhow::Result { let manifest = distributable.get_manifest()?; diff --git a/src/dist/mod.rs b/src/dist/mod.rs index dd6009f690..af39793ff8 100644 --- a/src/dist/mod.rs +++ b/src/dist/mod.rs @@ -18,7 +18,7 @@ use crate::{ config::Cfg, errors::{NIGHTLY_COMPONENT_NOTE, RustupError}, process::Process, - toolchain::DistributableToolchain, + toolchain::ChannelToolchain, utils, }; @@ -884,7 +884,7 @@ impl<'cfg, 'a> DistOptions<'cfg, 'a> { pub(super) fn for_update( mut self, - toolchain: &'a DistributableToolchain<'cfg>, + toolchain: &'a ChannelToolchain<'cfg>, allow_downgrade: bool, ) -> Self { self.allow_downgrade = allow_downgrade; diff --git a/src/toolchain.rs b/src/toolchain.rs index ea2648476f..8f285b8218 100644 --- a/src/toolchain.rs +++ b/src/toolchain.rs @@ -33,7 +33,7 @@ use crate::{ }; mod distributable; -pub(crate) use distributable::DistributableToolchain; +pub(crate) use distributable::ChannelToolchain; mod names; pub(crate) use names::{ @@ -63,7 +63,7 @@ impl<'a> Toolchain<'a> { .. }) if install_if_missing => { let options = DistOptions::new(&[], &[], &desc, cfg.get_profile()?, true, cfg)?; - let tc = DistributableToolchain::install(options).await?; + let tc = ChannelToolchain::install(options).await?; Ok(EnsureInstalled::new(tc.inner.into(), tc.status)) } Err(e) => Err(e.into()), @@ -379,7 +379,7 @@ impl<'a> Toolchain<'a> { for fallback in ["nightly", "beta", "stable"] { let resolved = PartialChannelToolchainName::from_str(fallback)?.complete(&default_host_tuple)?; - if let Ok(fallback) = DistributableToolchain::new(self.cfg, resolved) { + if let Ok(fallback) = ChannelToolchain::new(self.cfg, resolved) { let cmd = fallback.create_fallback_command("cargo", self)?; return Ok(Some(cmd)); } @@ -463,7 +463,7 @@ impl<'a> Toolchain<'a> { &self.name, ToolchainNameOrPath::Named(ToolchainName::Channel(_)) ) { - let distributable = DistributableToolchain::try_from(self)?; + let distributable = ChannelToolchain::try_from(self)?; // Design note: this is a bit of an awkward cast from // general (toolchain) to more specialised (distributable); // perhaps this function should something implemented on a diff --git a/src/toolchain/distributable.rs b/src/toolchain/distributable.rs index ccece06a21..b6b30189b8 100644 --- a/src/toolchain/distributable.rs +++ b/src/toolchain/distributable.rs @@ -27,12 +27,12 @@ use crate::{ /// An official toolchain installed on the local disk #[derive(Debug)] -pub(crate) struct DistributableToolchain<'a> { +pub(crate) struct ChannelToolchain<'a> { pub(crate) toolchain: Toolchain<'a>, desc: ChannelToolchainName, } -impl<'a> DistributableToolchain<'a> { +impl<'a> ChannelToolchain<'a> { #[tracing::instrument(level = "trace", err(level = "trace"), skip_all)] pub(crate) async fn install( options: DistOptions<'a, '_>, @@ -414,7 +414,7 @@ impl<'a> DistributableToolchain<'a> { } } -impl<'a> TryFrom<&Toolchain<'a>> for DistributableToolchain<'a> { +impl<'a> TryFrom<&Toolchain<'a>> for ChannelToolchain<'a> { type Error = RustupError; fn try_from(value: &Toolchain<'a>) -> Result { @@ -428,8 +428,8 @@ impl<'a> TryFrom<&Toolchain<'a>> for DistributableToolchain<'a> { } } -impl<'a> From> for Toolchain<'a> { - fn from(value: DistributableToolchain<'a>) -> Self { +impl<'a> From> for Toolchain<'a> { + fn from(value: ChannelToolchain<'a>) -> Self { value.toolchain } } From d7669d358370ab80a81ea3e7171d7cd5724e0c34 Mon Sep 17 00:00:00 2001 From: rami3l Date: Tue, 6 Oct 2026 19:23:33 +0200 Subject: [PATCH 4/9] refactor(toolchain): rename `mod distributable` to `mod channels` --- src/toolchain.rs | 4 ++-- src/toolchain/{distributable.rs => channels.rs} | 0 2 files changed, 2 insertions(+), 2 deletions(-) rename src/toolchain/{distributable.rs => channels.rs} (100%) diff --git a/src/toolchain.rs b/src/toolchain.rs index 8f285b8218..96ef25bf3e 100644 --- a/src/toolchain.rs +++ b/src/toolchain.rs @@ -32,8 +32,8 @@ use crate::{ utils::{self, raw::open_dir_following_links}, }; -mod distributable; -pub(crate) use distributable::ChannelToolchain; +mod channels; +pub(crate) use channels::ChannelToolchain; mod names; pub(crate) use names::{ diff --git a/src/toolchain/distributable.rs b/src/toolchain/channels.rs similarity index 100% rename from src/toolchain/distributable.rs rename to src/toolchain/channels.rs From 89638e82967c70f1594976af74fb07de8e331911 Mon Sep 17 00:00:00 2001 From: rami3l Date: Tue, 6 Oct 2026 18:20:00 +0200 Subject: [PATCH 5/9] refactor(toolchain): move `Cfg::toolchain_path()` to `ToolchainNameExt` trait --- src/config.rs | 8 -------- src/install.rs | 4 ++-- src/toolchain.rs | 10 +++++----- src/toolchain/names.rs | 19 +++++++++++++++++++ 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/config.rs b/src/config.rs index 854ce5acb2..f8d132c469 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1082,14 +1082,6 @@ impl<'a> Cfg<'a> { .with(|s| Ok(default_host_tuple(s, self.process))) } - /// The path on disk of any concrete toolchain - pub(crate) fn toolchain_path(&self, toolchain: &ToolchainNameOrPath) -> PathBuf { - match toolchain { - ToolchainNameOrPath::Named(name) => self.toolchains_dir.join(name.to_string()), - ToolchainNameOrPath::Path(p) => p.to_path_buf(), - } - } - /// Notifies a user with a hint whenever a new Rust release is available. /// This is only shown at max once per day and only if not in proxy mode. pub(crate) fn notify_release(&self) -> anyhow::Result<()> { diff --git a/src/install.rs b/src/install.rs index be076081b6..61676502ee 100644 --- a/src/install.rs +++ b/src/install.rs @@ -8,7 +8,7 @@ use crate::{ config::Cfg, dist::{DistOptions, manifest::ManifestWithHash, prefix::InstallPrefix}, errors::RustupError, - toolchain::{CustomToolchainName, Toolchain, ToolchainNameOrPath}, + toolchain::{CustomToolchainName, Toolchain, ToolchainNameExt as _, ToolchainNameOrPath}, utils, }; @@ -65,7 +65,7 @@ impl InstallMethod<'_, '_> { } }; - let toolchain_path = &cfg.toolchain_path(&toolchain); + let toolchain_path = &toolchain.path(cfg); debug!("toolchain directory: {}", toolchain_path.display()); if toolchain_path.exists() && !matches!(self, Self::Dist { .. }) { uninstall(toolchain_path)?; diff --git a/src/toolchain.rs b/src/toolchain.rs index 96ef25bf3e..3e660d8626 100644 --- a/src/toolchain.rs +++ b/src/toolchain.rs @@ -38,8 +38,8 @@ pub(crate) use channels::ChannelToolchain; mod names; pub(crate) use names::{ CustomToolchainName, MaybeChannelToolchainName, MaybePartialToolchainName, Override, - PartialToolchainName, PartialToolchainNameOrPath, ToolchainName, ToolchainNameOrPath, - ToolchainPath, + PartialToolchainName, PartialToolchainNameOrPath, ToolchainName, ToolchainNameExt, + ToolchainNameOrPath, ToolchainPath, }; /// A toolchain installed on the local disk @@ -109,7 +109,7 @@ impl<'a> Toolchain<'a> { } pub(crate) fn new(cfg: &'a Cfg<'a>, name: ToolchainNameOrPath) -> Result { - let path = cfg.toolchain_path(&name); + let path = name.path(cfg); if !Toolchain::exists(cfg, &name)? { return Err(match name { ToolchainNameOrPath::Named(name) => { @@ -125,7 +125,7 @@ impl<'a> Toolchain<'a> { /// Ok(True) if the toolchain exists. Ok(False) if the toolchain or its /// containing directory don't exist. Err otherwise. pub(crate) fn exists(cfg: &Cfg<'_>, name: &ToolchainNameOrPath) -> Result { - let path = cfg.toolchain_path(name); + let path = name.path(cfg); // toolchain validation should have prevented a situation where there is // no base dir, but defensive programming is defensive. let parent = path @@ -534,7 +534,7 @@ impl<'a> Toolchain<'a> { /// /// pub fn ensure_removed(cfg: &Cfg<'_>, name: ToolchainNameOrPath) -> anyhow::Result<()> { - let path = cfg.toolchain_path(&name); + let path = name.path(cfg); let name = match name { ToolchainNameOrPath::Named(t) => t, ToolchainNameOrPath::Path(_) => bail!("Cannot remove a path based toolchain"), diff --git a/src/toolchain/names.rs b/src/toolchain/names.rs index aa7bfe1005..3236c4abfb 100644 --- a/src/toolchain/names.rs +++ b/src/toolchain/names.rs @@ -280,6 +280,8 @@ pub enum ToolchainName { Custom(CustomToolchainName), } +impl ToolchainNameExt for ToolchainName {} + impl From for ToolchainName { fn from(value: ChannelToolchainName) -> Self { Self::Channel(value) @@ -395,6 +397,15 @@ pub(crate) enum ToolchainNameOrPath { Path(ToolchainPath), } +impl ToolchainNameExt for ToolchainNameOrPath { + fn path(&self, cfg: &Cfg<'_>) -> PathBuf { + match self { + Self::Named(name) => name.path(cfg), + Self::Path(p) => p.to_path_buf(), + } + } +} + impl From for ToolchainNameOrPath { fn from(value: ToolchainName) -> Self { Self::Named(value) @@ -530,6 +541,14 @@ impl Deref for ToolchainPath { } } +/// Extension methods for a generic toolchain name. +pub trait ToolchainNameExt: Display { + /// Provides the path to the toolchain's root directory. + fn path(&self, cfg: &Cfg<'_>) -> PathBuf { + cfg.toolchains_dir.join(self.to_string()) + } +} + /// Common validate rules for toolchain names that aren't paths. /// /// Beyond the shared [`normalize_name`] rules, every character must be allowed in an From ded01d6f39d24a7015a491f476e3fd37c5cc8d0b Mon Sep 17 00:00:00 2001 From: rami3l Date: Tue, 6 Oct 2026 18:20:00 +0200 Subject: [PATCH 6/9] refactor(toolchain): move `Toolchain::exists()` to `ToolchainNameExt` trait --- src/cli/self_update.rs | 7 +++++-- src/config.rs | 5 +++-- src/dist/mod.rs | 4 +++- src/install.rs | 4 ++-- src/toolchain.rs | 33 ++++----------------------------- src/toolchain/names.rs | 30 ++++++++++++++++++++++++++++++ 6 files changed, 47 insertions(+), 36 deletions(-) diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index 08cc072a2d..9e153decd0 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -68,7 +68,10 @@ use crate::{ install::{InstallMethod, UpdateStatus}, process::Process, settings::SettingsFile, - toolchain::{ChannelToolchain, MaybeChannelToolchainName, PartialToolchainName, Toolchain}, + toolchain::{ + ChannelToolchain, MaybeChannelToolchainName, PartialToolchainName, Toolchain, + ToolchainNameExt as _, + }, utils::{self, ExitCode}, }; @@ -272,7 +275,7 @@ impl InstallOpts<'_> { let desc = partial_desc.clone().complete(&cfg.default_host_tuple()?)?; let options = DistOptions::new(components, targets, &desc, cfg.get_profile()?, true, &cfg)?; - let status = if Toolchain::exists(&cfg, &desc.clone().into())? { + let status = if desc.exists(&cfg)? { warn!("Updating existing toolchain, profile choice will be ignored"); // If we have a partial install we might not be able to read content here. We could: // - fail and folk have to delete the partially present toolchain to recover diff --git a/src/config.rs b/src/config.rs index f8d132c469..1b15357ef9 100644 --- a/src/config.rs +++ b/src/config.rs @@ -26,7 +26,8 @@ use crate::{ settings::{MetadataVersion, Settings, SettingsFile}, toolchain::{ ChannelToolchain, CustomToolchainName, Override, PartialToolchainName, - PartialToolchainNameOrPath, Toolchain, ToolchainName, ToolchainNameOrPath, ToolchainPath, + PartialToolchainNameOrPath, Toolchain, ToolchainName, ToolchainNameExt as _, + ToolchainNameOrPath, ToolchainPath, }, utils, }; @@ -767,7 +768,7 @@ impl<'a> Cfg<'a> { // XXX: this awkwardness deals with settings file being locked already let toolchain_name = toolchain_name.complete(&default_host)?; - if !Toolchain::exists(self, &toolchain_name.clone().into())? + if !toolchain_name.exists(self)? && matches!(toolchain_name, ToolchainName::Custom(_)) { bail!( diff --git a/src/dist/mod.rs b/src/dist/mod.rs index af39793ff8..bef5fa85e3 100644 --- a/src/dist/mod.rs +++ b/src/dist/mod.rs @@ -18,7 +18,7 @@ use crate::{ config::Cfg, errors::{NIGHTLY_COMPONENT_NOTE, RustupError}, process::Process, - toolchain::ChannelToolchain, + toolchain::{ChannelToolchain, ToolchainNameExt}, utils, }; @@ -301,6 +301,8 @@ impl ChannelToolchainName { } } +impl ToolchainNameExt for ChannelToolchainName {} + impl FromStr for ChannelToolchainName { type Err = anyhow::Error; fn from_str(name: &str) -> anyhow::Result { diff --git a/src/install.rs b/src/install.rs index 61676502ee..6fc4bc758d 100644 --- a/src/install.rs +++ b/src/install.rs @@ -8,7 +8,7 @@ use crate::{ config::Cfg, dist::{DistOptions, manifest::ManifestWithHash, prefix::InstallPrefix}, errors::RustupError, - toolchain::{CustomToolchainName, Toolchain, ToolchainNameExt as _, ToolchainNameOrPath}, + toolchain::{CustomToolchainName, ToolchainNameExt as _, ToolchainNameOrPath}, utils, }; @@ -95,7 +95,7 @@ impl InstallMethod<'_, '_> { }; // Final check, to ensure we're installed - if !Toolchain::exists(cfg, &toolchain)? { + if !toolchain.exists(cfg)? { return Err(RustupError::ToolchainNotInstallable(toolchain.to_string()).into()); } diff --git a/src/toolchain.rs b/src/toolchain.rs index 3e660d8626..0bb16b9b46 100644 --- a/src/toolchain.rs +++ b/src/toolchain.rs @@ -5,7 +5,7 @@ use std::{ ffi::{OsStr, OsString}, fmt::Debug, fs, - io::{self, BufRead, BufReader}, + io::{BufRead, BufReader}, path::{Path, PathBuf}, process::{Command, Stdio}, str::FromStr, @@ -13,7 +13,6 @@ use std::{ }; use anyhow::{Context, anyhow, bail}; -use fs_at::OpenOptions; use same_file::is_same_file; use tracing::info; use url::Url; @@ -29,7 +28,7 @@ use crate::{ }, env_var, install::{self, UpdateStatus}, - utils::{self, raw::open_dir_following_links}, + utils, }; mod channels; @@ -110,7 +109,7 @@ impl<'a> Toolchain<'a> { pub(crate) fn new(cfg: &'a Cfg<'a>, name: ToolchainNameOrPath) -> Result { let path = name.path(cfg); - if !Toolchain::exists(cfg, &name)? { + if !name.exists(cfg)? { return Err(match name { ToolchainNameOrPath::Named(name) => { let is_active = matches!(cfg.active_toolchain(), Ok(Some((t, _))) if t == name); @@ -122,30 +121,6 @@ impl<'a> Toolchain<'a> { Ok(Self { cfg, name, path }) } - /// Ok(True) if the toolchain exists. Ok(False) if the toolchain or its - /// containing directory don't exist. Err otherwise. - pub(crate) fn exists(cfg: &Cfg<'_>, name: &ToolchainNameOrPath) -> Result { - let path = name.path(cfg); - // toolchain validation should have prevented a situation where there is - // no base dir, but defensive programming is defensive. - let parent = path - .parent() - .ok_or_else(|| RustupError::InvalidToolchainName(name.to_string()))?; - let base_name = path - .file_name() - .ok_or_else(|| RustupError::InvalidToolchainName(name.to_string()))?; - let parent_dir = match open_dir_following_links(parent) { - Ok(d) => d, - Err(e) if e.kind() == io::ErrorKind::NotFound => return Ok(false), - e => e?, - }; - let opened = OpenOptions::default() - .read(true) - .follow(true) - .open_dir_at(&parent_dir, base_name); - Ok(opened.is_ok()) - } - pub(crate) fn name(&self) -> &ToolchainNameOrPath { &self.name } @@ -539,7 +514,7 @@ impl<'a> Toolchain<'a> { ToolchainNameOrPath::Named(t) => t, ToolchainNameOrPath::Path(_) => bail!("Cannot remove a path based toolchain"), }; - let fs_modified = match Self::exists(cfg, &name.clone().into())? { + let fs_modified = match name.exists(cfg)? { true => { info!("uninstalling toolchain {name}"); let installed_paths = match &name { diff --git a/src/toolchain/names.rs b/src/toolchain/names.rs index 3236c4abfb..7f14b35c1b 100644 --- a/src/toolchain/names.rs +++ b/src/toolchain/names.rs @@ -43,17 +43,21 @@ use std::{ fmt::{self, Display}, + io, ops::Deref, path::{Path, PathBuf}, str::FromStr, }; +use fs_at::OpenOptions; use thiserror::Error; use unicode_security::GeneralSecurityProfile; use crate::{ config::{Cfg, no_toolchain_error}, dist::{ChannelToolchainName, PartialChannelToolchainName, TargetTuple}, + errors::RustupError, + utils::raw::open_dir_following_links, }; /// Errors related to toolchains @@ -543,6 +547,32 @@ impl Deref for ToolchainPath { /// Extension methods for a generic toolchain name. pub trait ToolchainNameExt: Display { + /// Checks if the toolchain is already installed. + /// + /// # Returns + /// + /// - `Ok(true)` if the toolchain exists. + /// - `Ok(false)` if the toolchain or its containing directory don't exist. + /// - `Err(_)` if there was an error checking for the toolchain's existence. + fn exists(&self, cfg: &Cfg<'_>) -> Result { + let path = self.path(cfg); + // toolchain validation should have prevented a situation where there is + // no base dir, but defensive programming is defensive. + let invalid_name = || RustupError::InvalidToolchainName(self.to_string()); + let parent = path.parent().ok_or_else(invalid_name)?; + let base_name = path.file_name().ok_or_else(invalid_name)?; + let parent_dir = match open_dir_following_links(parent) { + Ok(d) => d, + Err(e) if e.kind() == io::ErrorKind::NotFound => return Ok(false), + e => e?, + }; + let opened = OpenOptions::default() + .read(true) + .follow(true) + .open_dir_at(&parent_dir, base_name); + Ok(opened.is_ok()) + } + /// Provides the path to the toolchain's root directory. fn path(&self, cfg: &Cfg<'_>) -> PathBuf { cfg.toolchains_dir.join(self.to_string()) From 1d8fadd371a3824235199cdc6ab458bdc12d552f Mon Sep 17 00:00:00 2001 From: rami3l Date: Thu, 8 Oct 2026 10:52:16 +0200 Subject: [PATCH 7/9] refactor(toolchain): reorder items in `impl ChannelToolchain` --- src/toolchain/channels.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/toolchain/channels.rs b/src/toolchain/channels.rs index b6b30189b8..20a5ef9adf 100644 --- a/src/toolchain/channels.rs +++ b/src/toolchain/channels.rs @@ -58,10 +58,6 @@ impl<'a> ChannelToolchain<'a> { Toolchain::new(cfg, desc.clone().into()).map(|toolchain| Self { toolchain, desc }) } - pub(crate) fn desc(&self) -> &ChannelToolchainName { - &self.desc - } - pub(crate) async fn add_components( &self, components: impl IntoIterator>, @@ -412,6 +408,10 @@ impl<'a> ChannelToolchain<'a> { None => Ok(None), } } + + pub(crate) fn desc(&self) -> &ChannelToolchainName { + &self.desc + } } impl<'a> TryFrom<&Toolchain<'a>> for ChannelToolchain<'a> { From b48a9d11f24ea10b0f3071f216f92c4f90755f17 Mon Sep 17 00:00:00 2001 From: rami3l Date: Wed, 7 Oct 2026 10:19:56 +0200 Subject: [PATCH 8/9] refactor(toolchain): remove redundant `ChannelToolchain::desc` field --- src/toolchain/channels.rs | 71 ++++++++++++++++++++++++--------------- 1 file changed, 43 insertions(+), 28 deletions(-) diff --git a/src/toolchain/channels.rs b/src/toolchain/channels.rs index 20a5ef9adf..d9e0530f0c 100644 --- a/src/toolchain/channels.rs +++ b/src/toolchain/channels.rs @@ -29,7 +29,6 @@ use crate::{ #[derive(Debug)] pub(crate) struct ChannelToolchain<'a> { pub(crate) toolchain: Toolchain<'a>, - desc: ChannelToolchainName, } impl<'a> ChannelToolchain<'a> { @@ -55,13 +54,14 @@ impl<'a> ChannelToolchain<'a> { } pub(crate) fn new(cfg: &'a Cfg<'a>, desc: ChannelToolchainName) -> Result { - Toolchain::new(cfg, desc.clone().into()).map(|toolchain| Self { toolchain, desc }) + Toolchain::new(cfg, desc.into()).map(|toolchain| Self { toolchain }) } pub(crate) async fn add_components( &self, components: impl IntoIterator>, ) -> anyhow::Result<()> { + let desc = self.desc(); let manifestation = self.get_manifestation()?; let manifest = self.get_manifest()?; @@ -71,7 +71,7 @@ impl<'a> ChannelToolchain<'a> { .expect("manifest should contain a rust package"); let targ_pkg = rust_pkg .targets - .get(&self.desc.target) + .get(&desc.target) .expect("installed manifest should have a known target"); let components = components.into_iter(); @@ -95,9 +95,7 @@ impl<'a> ChannelToolchain<'a> { } let config = manifestation.read_config()?.unwrap_or_default(); - let suggestion = - component_suggestion(&self.desc, &component, &config, &manifest, false); - let desc = self.desc.clone(); + let suggestion = component_suggestion(desc, &component, &config, &manifest, false); if targ_pkg .components @@ -105,7 +103,7 @@ impl<'a> ChannelToolchain<'a> { .any(|c| c.target() == component.target()) { return Err(RustupError::UnknownComponents { - desc, + desc: desc.clone(), components: vec![UnknownComponentInfo { name: manifest.short_name(&component).to_string(), description: manifest.description(&component), @@ -117,11 +115,15 @@ impl<'a> ChannelToolchain<'a> { let target = component.target.expect("component target should be known"); if let Some(platform) = Platform::find(&target) { - return Err(RustupError::UnavailableTarget { desc, platform }.into()); + return Err(RustupError::UnavailableTarget { + desc: desc.clone(), + platform, + } + .into()); } return Err(RustupError::UnknownTarget { - desc: Box::new(desc), + desc: Box::new(desc.clone()), target, suggestion, } @@ -135,7 +137,7 @@ impl<'a> ChannelToolchain<'a> { let download_cfg = DownloadCfg::new(self.toolchain.cfg); manifestation - .update(manifest, changes, false, &download_cfg, &self.desc, false) + .update(manifest, changes, false, &download_cfg, desc, false) .await?; Ok(()) @@ -170,7 +172,7 @@ impl<'a> ChannelToolchain<'a> { }; let config = manifestation.read_config()?.unwrap_or_default(); - let installed_components = manifest.query_components(&self.desc, &config)?; + let installed_components = manifest.query_components(self.desc(), &config)?; // check if all the components we want are installed let wanted_components = components.iter().all(|name| { installed_components.iter().any(|status| { @@ -245,7 +247,7 @@ impl<'a> ChannelToolchain<'a> { #[tracing::instrument(level = "trace", skip_all)] pub(crate) fn get_manifestation(&self) -> anyhow::Result { let prefix = InstallPrefix::from(self.toolchain.path()); - Manifestation::open(prefix, self.desc.target.clone()) + Manifestation::open(prefix, self.desc().target.clone()) } /// Get the manifest associated with this distribution @@ -254,9 +256,12 @@ impl<'a> ChannelToolchain<'a> { self.get_manifestation()? .load_manifest() .transpose() - .unwrap_or_else(|| match self.guess_v1_manifest() { - true => Err(RustupError::ComponentsUnsupportedV1(self.desc.to_string()).into()), - false => Err(RustupError::MissingManifest(self.desc.clone()).into()), + .unwrap_or_else(|| { + let desc = self.desc(); + match self.guess_v1_manifest() { + true => Err(RustupError::ComponentsUnsupportedV1(desc.to_string()).into()), + false => Err(RustupError::MissingManifest(desc.clone()).into()), + } }) } @@ -266,12 +271,12 @@ impl<'a> ChannelToolchain<'a> { } pub fn recursion_error(&self, binary_lossy: String) -> Result { + let desc = self.desc(); let prefix = InstallPrefix::from(self.toolchain.path()); - let manifestation = Manifestation::open(prefix, self.desc.target.clone())?; + let manifestation = Manifestation::open(prefix, desc.target.clone())?; let manifest = self.get_manifest()?; let config = manifestation.read_config()?.unwrap_or_default(); - let component_statuses = manifest.query_components(&self.desc, &config)?; - let desc = &self.desc; + let component_statuses = manifest.query_components(desc, &config)?; if let Some(component_name) = component_for_bin(&binary_lossy) { let component_status = component_statuses .iter() @@ -289,7 +294,7 @@ impl<'a> ChannelToolchain<'a> { } else { // available, not installed, recommend installation let selector = match self.toolchain.cfg.get_default()? { - Some(ToolchainName::Channel(n)) if n == self.desc => String::new(), + Some(ToolchainName::Channel(n)) if &n == desc => String::new(), _ => format!("--toolchain {} ", self.toolchain.name()), }; Err(anyhow!( @@ -308,6 +313,7 @@ impl<'a> ChannelToolchain<'a> { &self, components: impl IntoIterator>, ) -> anyhow::Result<()> { + let desc = self.desc(); let manifestation = self.get_manifestation()?; let config = manifestation.read_config()?.unwrap_or_default(); let manifest = self.get_manifest()?; @@ -339,7 +345,7 @@ impl<'a> ChannelToolchain<'a> { .any(|c| c.target() == target.deref()) { let suggestion = TargetSuggestion::from_target( - &self.desc, + desc, target, &component, &config, @@ -347,7 +353,7 @@ impl<'a> ChannelToolchain<'a> { self.toolchain.cfg, ); return Err(RustupError::TargetNotInstalled { - desc: Box::new(self.desc.clone()), + desc: Box::new(desc.clone()), target: target.clone(), suggestion, } @@ -355,7 +361,7 @@ impl<'a> ChannelToolchain<'a> { } let suggestion = ComponentSuggestion::for_removal( - &self.desc, + desc, &component, &config, &manifest, @@ -376,7 +382,7 @@ impl<'a> ChannelToolchain<'a> { let download_cfg = DownloadCfg::new(self.toolchain.cfg); manifestation - .update(manifest, changes, false, &download_cfg, &self.desc, false) + .update(manifest, changes, false, &download_cfg, desc, false) .await?; if !unknown_components.is_empty() { @@ -391,14 +397,15 @@ impl<'a> ChannelToolchain<'a> { } pub async fn fetch_dist_manifest(&self) -> anyhow::Result> { + let desc = self.desc(); let prefix = InstallPrefix::from(self.toolchain.path()); let update_hash = if prefix.dist_manifest().is_some() { - Some(self.toolchain.cfg.get_hash_file(&self.desc, false)?) + Some(self.toolchain.cfg.get_hash_file(desc, false)?) } else { None }; DownloadCfg::new(self.toolchain.cfg) - .dl_v2_manifest(update_hash.as_deref(), &self.desc, self.toolchain.cfg) + .dl_v2_manifest(update_hash.as_deref(), desc, self.toolchain.cfg) .await } @@ -410,7 +417,16 @@ impl<'a> ChannelToolchain<'a> { } pub(crate) fn desc(&self) -> &ChannelToolchainName { - &self.desc + match self.toolchain.name() { + ToolchainNameOrPath::Named(ToolchainName::Channel(name)) => name, + // TODO: `self.toolchain` is in principle ensured to have a `ChannelToolchainName` since + // it has been constructed as such in `Self::new()`, so this case should be unreachable. + // However, this may not be clear to the outside caller, so further cleanup is needed to + // make sure `Self` cannot be constructed with (or mutated to have) a non-channel name. + // For example, this should be possible with the following feature when it lands: + // + _ => unreachable!("ChannelToolchain should always have a ChannelToolchainName"), + } } } @@ -419,9 +435,8 @@ impl<'a> TryFrom<&Toolchain<'a>> for ChannelToolchain<'a> { fn try_from(value: &Toolchain<'a>) -> Result { match value.name() { - ToolchainNameOrPath::Named(ToolchainName::Channel(desc)) => Ok(Self { + ToolchainNameOrPath::Named(ToolchainName::Channel(_)) => Ok(Self { toolchain: value.clone(), - desc: desc.clone(), }), n => Err(RustupError::ComponentsUnsupported(n.to_string())), } From 9d1a7b23f436c81e55ebf12be2b4ddad92f920e4 Mon Sep 17 00:00:00 2001 From: rami3l Date: Thu, 8 Oct 2026 11:06:20 +0200 Subject: [PATCH 9/9] refactor(toolchain): require explicit cloning when converting to `ChannelToolchain` --- src/cli/docs.rs | 2 +- src/toolchain.rs | 11 ++++++----- src/toolchain/channels.rs | 13 ++++++------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/cli/docs.rs b/src/cli/docs.rs index 9625052f04..ad459c1d0d 100644 --- a/src/cli/docs.rs +++ b/src/cli/docs.rs @@ -191,7 +191,7 @@ pub(crate) async fn doc( let toolchain = toolchain.map(|desc| (desc, ActiveSource::CommandLine)); let toolchain = cfg.toolchain_from_partial(toolchain).await?.0; - if let Ok(distributable) = ChannelToolchain::try_from(&toolchain) + if let Ok(distributable) = ChannelToolchain::try_from(toolchain.clone()) && let [_] = distributable .components()? .into_iter() diff --git a/src/toolchain.rs b/src/toolchain.rs index 0bb16b9b46..1fa1275956 100644 --- a/src/toolchain.rs +++ b/src/toolchain.rs @@ -438,11 +438,12 @@ impl<'a> Toolchain<'a> { &self.name, ToolchainNameOrPath::Named(ToolchainName::Channel(_)) ) { - let distributable = ChannelToolchain::try_from(self)?; - // Design note: this is a bit of an awkward cast from - // general (toolchain) to more specialised (distributable); - // perhaps this function should something implemented on a - // trait, permitting removal of that case. + // NOTE: This is a bit of an awkward cast from general `Toolchain` to more + // specialised `ChannelToolchain` which is just a newtype over the former; in + // theory, a `.clone()` is not needed because for `.recursion_error()` we would + // only need a `&ChannelToolchain`, which is safely convertible from + // `&Toolchain`. + let distributable = ChannelToolchain::try_from(self.clone())?; return Err(distributable.recursion_error(binary_lossy).unwrap_err()); } else { let t = &self.name; diff --git a/src/toolchain/channels.rs b/src/toolchain/channels.rs index d9e0530f0c..3429f7da71 100644 --- a/src/toolchain/channels.rs +++ b/src/toolchain/channels.rs @@ -27,6 +27,7 @@ use crate::{ /// An official toolchain installed on the local disk #[derive(Debug)] +#[repr(transparent)] pub(crate) struct ChannelToolchain<'a> { pub(crate) toolchain: Toolchain<'a>, } @@ -49,7 +50,7 @@ impl<'a> ChannelToolchain<'a> { cfg: &'a Cfg<'a>, ) -> anyhow::Result { Ok(Self::try_from( - &cfg.toolchain_from_partial(toolchain).await?.0, + cfg.toolchain_from_partial(toolchain).await?.0, )?) } @@ -430,14 +431,12 @@ impl<'a> ChannelToolchain<'a> { } } -impl<'a> TryFrom<&Toolchain<'a>> for ChannelToolchain<'a> { +impl<'a> TryFrom> for ChannelToolchain<'a> { type Error = RustupError; - fn try_from(value: &Toolchain<'a>) -> Result { - match value.name() { - ToolchainNameOrPath::Named(ToolchainName::Channel(_)) => Ok(Self { - toolchain: value.clone(), - }), + fn try_from(toolchain: Toolchain<'a>) -> Result { + match toolchain.name() { + ToolchainNameOrPath::Named(ToolchainName::Channel(_)) => Ok(Self { toolchain }), n => Err(RustupError::ComponentsUnsupported(n.to_string())), } }