Skip to content

feat(config)!: use unqualified names for default_toolchain#4947

Draft
rami3l wants to merge 3 commits into
rust-lang:mainfrom
rami3l:feat/unqualified-default-toolchain
Draft

feat(config)!: use unqualified names for default_toolchain#4947
rami3l wants to merge 3 commits into
rust-lang:mainfrom
rami3l:feat/unqualified-default-toolchain

Conversation

@rami3l

@rami3l rami3l commented Jul 9, 2026

Copy link
Copy Markdown
Member

Closes #4945.

The breakage should be minimal anyway because the default host tuple almost never changes.

@rami3l rami3l changed the title refactor(config): extract Cfg::get_default_resolvable() feat(config)!: use unqualified names for default_toolchain Jul 9, 2026
Comment thread src/cli/self_update.rs
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.

Comment thread src/cli/self_update.rs

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Should default_toolchain in settings.toml allow unresolved names?

2 participants