Skip to content
Draft
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
8 changes: 4 additions & 4 deletions src/cli/rustup_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -858,13 +858,13 @@ async fn default_(
cfg.set_default(Some(&toolchain_name.into()))?;
}
MaybeResolvableToolchainName::Some(ResolvableToolchainName::Official(toolchain)) => {
let desc = toolchain.resolve(&cfg.default_host_tuple()?)?;
let desc = toolchain.clone().resolve(&cfg.default_host_tuple()?)?;
let status = cfg
.ensure_installed(&desc, vec![], vec![], None, force_non_host, true)
.await?
.status;

cfg.set_default(Some(&desc.clone().into()))?;
cfg.set_default(Some(&toolchain.into()))?;

writeln!(cfg.process.stdout().lock())?;

Expand Down Expand Up @@ -1063,7 +1063,7 @@ async fn update(
force_non_host,
)?;
}
let desc = name.resolve(&cfg.default_host_tuple()?)?;
let desc = name.clone().resolve(&cfg.default_host_tuple()?)?;

let components = opts.component.iter().map(|s| &**s).collect::<Vec<_>>();
let targets = opts.target.iter().map(|s| &**s).collect::<Vec<_>>();
Expand Down Expand Up @@ -1104,7 +1104,7 @@ async fn update(
}

if cfg.get_default()?.is_none() && matches!(status, UpdateStatus::Installed) {
cfg.set_default(Some(&desc.into()))?;
cfg.set_default(Some(&name.into()))?;
}
}
exit_code &= self_update_mode.update(should_self_update, &dl_cfg).await?;
Expand Down
28 changes: 10 additions & 18 deletions src/cli/self_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ use crate::{
process::Process,
toolchain::{
DistributableToolchain, MaybeOfficialToolchainName, ResolvableToolchainName, Toolchain,
ToolchainName,
},
utils::{self, ExitCode},
};
Expand Down Expand Up @@ -107,7 +106,7 @@ pub(crate) struct InstallOpts<'a> {
}

impl InstallOpts<'_> {
fn install(self, cfg: &mut Cfg<'_>) -> Result<Option<ToolchainDesc>> {
fn install(self, cfg: &mut Cfg<'_>) -> Result<Option<PartialToolchainDesc>> {

@rami3l rami3l Jul 9, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nit: Pre-existing but this naming is quite confusing, how can InstallOpts::install() be actually not installing anything?

View changes since the review

let Self {
default_host_tuple,
default_toolchain,
Expand Down Expand Up @@ -165,18 +164,14 @@ impl InstallOpts<'_> {
MaybeOfficialToolchainName::None => unreachable!(),
MaybeOfficialToolchainName::Some(n) => n,
};
Some(toolchain_name.resolve(&cfg.default_host_tuple()?)?)
Some(toolchain_name)
}
None => match cfg.get_default()? {
None => match cfg.get_default_resolvable()? {
// Default is installable
Some(ToolchainName::Official(t)) => Some(t),
Some(ResolvableToolchainName::Official(t)) => Some(t),
// Default is custom, presumably from a prior install. Do nothing.
Some(ToolchainName::Custom(_)) => None,
None => Some(
"stable"
.parse::<PartialToolchainDesc>()?
.resolve(&cfg.default_host_tuple()?)?,
),
Some(ResolvableToolchainName::Custom(_)) => None,
None => Some(PartialToolchainDesc::from_str("stable")?),
},
})
} else {
Expand Down Expand Up @@ -1022,7 +1017,8 @@ async fn maybe_install_rust(opts: InstallOpts<'_>, cfg: &mut Cfg<'_>) -> Result<

let (components, targets) = (opts.components, opts.targets);
let toolchain = opts.install(cfg)?;
if let Some(desc) = &toolchain {
if let Some(partial_desc) = toolchain {
let desc = &partial_desc.clone().resolve(&cfg.default_host_tuple()?)?;

@rami3l rami3l Jul 9, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nit: Pre-existing, but later on we have desc.into() which is converting &ToolchainDesc to something owned. Looks like d494459 (#4930) is not complete? cc @djc

View changes since the review

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That commit just fixed some instances of it, it didn't claim to fix all of it.

let options = DistOptions::new(components, targets, desc, cfg.get_profile()?, true, cfg)?;
let status = if Toolchain::exists(cfg, &desc.into())? {
warn!("Updating existing toolchain, profile choice will be ignored");
Expand All @@ -1041,7 +1037,7 @@ async fn maybe_install_rust(opts: InstallOpts<'_>, cfg: &mut Cfg<'_>) -> Result<

check_proxy_sanity(cfg.process, components, desc)?;

cfg.set_default(Some(&desc.clone().into()))?;
cfg.set_default(Some(&partial_desc.into()))?;
writeln!(cfg.process.stdout().lock())?;
common::show_channel_update(cfg, PackageUpdate::Toolchain(desc.clone()), Ok(status))?;
}
Expand Down Expand Up @@ -1521,11 +1517,7 @@ mod tests {
};

assert_eq!(
"stable"
.parse::<PartialToolchainDesc>()
.unwrap()
.resolve(&cfg.default_host_tuple().unwrap())
.unwrap(),
"stable".parse::<PartialToolchainDesc>().unwrap(),
opts.install(&mut cfg)
.unwrap() // result
.unwrap() // option
Expand Down
21 changes: 15 additions & 6 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ impl<'a> Cfg<'a> {
Ok(cfg)
}

pub(crate) fn set_default(&self, toolchain: Option<&ToolchainName>) -> Result<()> {
pub(crate) fn set_default(&self, toolchain: Option<&ResolvableToolchainName>) -> Result<()> {
self.settings_file.with_mut(|s| {
s.default_toolchain = toolchain.map(|t| t.to_string());
Ok(())
Expand Down Expand Up @@ -891,6 +891,16 @@ impl<'a> Cfg<'a> {
/// If none is configured, returns None
/// If a bad toolchain name is configured, errors.
pub(crate) fn get_default(&self) -> Result<Option<ToolchainName>> {
let Some(toolchain) = self.get_default_resolvable()? else {
return Ok(None);
};
Ok(Some(toolchain.resolve(&self.default_host_tuple()?)?))
}

/// Gets the configured default toolchain.
/// If none is configured, returns None
/// If a bad toolchain name is configured, errors.
pub(crate) fn get_default_resolvable(&self) -> Result<Option<ResolvableToolchainName>> {
let user_opt = self.settings_file.with(|s| Ok(s.default_toolchain.clone()));
let toolchain_maybe_str = if let Some(fallback_settings) = &self.fallback_settings {
match user_opt {
Expand All @@ -900,11 +910,10 @@ impl<'a> Cfg<'a> {
} else {
user_opt
}?;
toolchain_maybe_str
.map(ResolvableToolchainName::try_from)
.transpose()?
.map(|t| t.resolve(&self.default_host_tuple()?))
.transpose()
let Some(toolchain) = toolchain_maybe_str else {
return Ok(None);
};
Ok(Some(ResolvableToolchainName::try_from(toolchain)?))
}

/// List all the installed toolchains: that is paths in the toolchain dir
Expand Down
11 changes: 11 additions & 0 deletions src/toolchain/names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,17 @@ impl ResolvableLocalToolchainName {
}
}

from_variant!(
PartialToolchainDesc,
ResolvableToolchainName,
ResolvableToolchainName::Official
);
from_variant!(
CustomToolchainName,
ResolvableToolchainName,
ResolvableToolchainName::Custom
);

try_from_str!(ResolvableLocalToolchainName);

impl Display for ResolvableLocalToolchainName {
Expand Down
Loading