From 58d8556160cc582fd891e81351e44cae7f9d6519 Mon Sep 17 00:00:00 2001 From: ychampion Date: Tue, 21 Jul 2026 18:26:27 +0000 Subject: [PATCH] Surface toolchain file mistakes before strict parsing Constraint: Preserve current behavior during the warning-first migration Rejected: Immediate rejection | Maintainer requested a compatibility warning before hard errors Confidence: high Scope-risk: narrow Directive: Keep warning paths stable until the hard-error follow-up lands Tested: targeted integration test, library tests, Clippy, rustfmt, mdBook, diff check Not-tested: full integration suite --- Cargo.lock | 11 ++++++ Cargo.toml | 1 + doc/user-guide/src/overrides.md | 5 +++ src/config.rs | 64 +++++++++++++++++++++++++++------ tests/suite/cli_rustup.rs | 62 ++++++++++++++++++++++++++++++++ 5 files changed, 133 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 498843507b..511090b33e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2090,6 +2090,7 @@ dependencies = [ "scopeguard", "semver", "serde", + "serde_ignored", "sha2", "sharded-slab", "snapbox", @@ -2219,6 +2220,16 @@ dependencies = [ "syn", ] +[[package]] +name = "serde_ignored" +version = "0.1.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "115dffd5f3853e06e746965a20dcbae6ee747ae30b543d91b0e089668bb07798" +dependencies = [ + "serde", + "serde_core", +] + [[package]] name = "serde_json" version = "1.0.150" diff --git a/Cargo.toml b/Cargo.toml index e5053774aa..4562b6c944 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -83,6 +83,7 @@ rustls-platform-verifier = { version = "0.7", optional = true } same-file = "1" semver = "1.0" serde = { version = "1.0", features = ["derive"] } +serde_ignored = "0.1" sha2 = "0.11" sharded-slab = "0.1.1" strsim = "0.11" diff --git a/doc/user-guide/src/overrides.md b/doc/user-guide/src/overrides.md index 849ed515d2..2d13bf6911 100644 --- a/doc/user-guide/src/overrides.md +++ b/doc/user-guide/src/overrides.md @@ -114,6 +114,11 @@ issues with dependencies. ### Toolchain file settings +Rustup reports unrecognized settings as warnings. It ignores them when other +valid settings are present, so these warnings can indicate a misspelling that +would otherwise leave part of the requested configuration unapplied. A future +rustup release will reject unrecognized settings. + #### channel The `channel` setting specifies which [toolchain] to use. The value is a diff --git a/src/config.rs b/src/config.rs index 84e267ea42..bb83d6fccd 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,9 +1,11 @@ +use std::collections::HashSet; use std::fmt::{self, Debug, Display}; use std::io; use std::io::Write; use std::ops::Deref; use std::path::{Path, PathBuf}; use std::str::FromStr; +use std::sync::Mutex; use std::time::{SystemTime, UNIX_EPOCH}; use anyhow::{Context, Result, anyhow, bail}; @@ -290,6 +292,7 @@ pub(crate) struct Cfg<'a> { pub quiet: bool, pub current_dir: PathBuf, pub process: &'a Process, + warned_unknown_toolchain_keys: Mutex>, /// Allows the auto-installation of the active toolchain when it is missing. /// @@ -370,6 +373,7 @@ impl<'a> Cfg<'a> { quiet, current_dir, process, + warned_unknown_toolchain_keys: Mutex::new(HashSet::new()), allow_auto_install, }; @@ -688,13 +692,37 @@ impl<'a> Cfg<'a> { if let Ok(contents) = contents { // XXX Should not return the unvalidated contents; but a new // internal only safe struct - let override_file = - Cfg::parse_override_file(contents, parse_mode).with_context(|| { + let (override_file, unknown_keys) = Cfg::parse_override_file_with_unknown_keys( + contents, parse_mode, + ) + .with_context(|| RustupError::ParsingFile { + name: "override", + path: toolchain_file.clone(), + })?; + { + let mut warned_unknown_toolchain_keys = self + .warned_unknown_toolchain_keys + .lock() + .unwrap_or_else(|e| e.into_inner()); + for key in unknown_keys { + if warned_unknown_toolchain_keys + .insert((toolchain_file.clone(), key.clone())) + { + warn!( + "unknown key `{key}` in '{}'; it is currently ignored, but will become an error in a future release", + toolchain_file.display() + ); + } + } + } + if override_file.is_empty() { + return Err(anyhow!(OverrideFileConfigError::Invalid)).with_context(|| { RustupError::ParsingFile { name: "override", path: toolchain_file.clone(), } - })?; + }); + } if let Some(toolchain_name_str) = &override_file.toolchain.channel { let toolchain_name = ResolvableToolchainName::try_from( toolchain_name_str.as_str(), @@ -744,10 +772,23 @@ impl<'a> Cfg<'a> { Ok(None) } + #[cfg(test)] fn parse_override_file>( contents: S, parse_mode: ParseMode, ) -> Result { + let (override_file, _) = Self::parse_override_file_with_unknown_keys(contents, parse_mode)?; + if override_file.is_empty() { + Err(anyhow!(OverrideFileConfigError::Invalid)) + } else { + Ok(override_file) + } + } + + fn parse_override_file_with_unknown_keys>( + contents: S, + parse_mode: ParseMode, + ) -> Result<(OverrideFile, Vec)> { let contents = contents.as_ref(); match (contents.lines().count(), parse_mode) { @@ -758,18 +799,20 @@ impl<'a> Cfg<'a> { if channel.is_empty() { Err(anyhow!(OverrideFileConfigError::Empty)) } else { - Ok(channel.into()) + Ok((channel.into(), Vec::new())) } } _ => { - let override_file = toml::from_str::(contents) + let deserializer = toml::Deserializer::parse(contents) + .context(OverrideFileConfigError::Parsing)?; + let mut unknown_keys = Vec::new(); + let override_file: OverrideFile = + serde_ignored::deserialize(deserializer, |path| { + unknown_keys.push(path.to_string()); + }) .context(OverrideFileConfigError::Parsing)?; - if override_file.is_empty() { - Err(anyhow!(OverrideFileConfigError::Invalid)) - } else { - Ok(override_file) - } + Ok((override_file, unknown_keys)) } } } @@ -1095,6 +1138,7 @@ impl Debug for Cfg<'_> { quiet, current_dir, allow_auto_install, + warned_unknown_toolchain_keys: _, process: _, } = self; diff --git a/tests/suite/cli_rustup.rs b/tests/suite/cli_rustup.rs index ee69fdb81d..c30cc9a400 100644 --- a/tests/suite/cli_rustup.rs +++ b/tests/suite/cli_rustup.rs @@ -3968,6 +3968,68 @@ error: rustup could not choose a version of rustc to run, because one wasn't spe .is_ok(); } +#[tokio::test] +async fn rust_toolchain_toml_warns_on_unknown_keys() { + let cx = CliTestContext::new(Scenario::SimpleV2).await; + cx.config + .expect(["rustup", "toolchain", "install", "nightly"]) + .await + .is_ok(); + let cwd = cx.config.current_dir(); + let toolchain_file = cwd.join("rust-toolchain.toml"); + + raw::write_file( + &toolchain_file, + "[toolchain]\nchannel = \"nightly\"\nchannnel = \"stable\"", + ) + .unwrap(); + cx.config + .expect(["rustup", "show", "active-toolchain"]) + .await + .extend_redactions([("[CWD]", &cwd)]) + .with_stdout(snapbox::str![[r#" +nightly-[HOST_TUPLE] (overridden by '[CWD]/rust-toolchain.toml') + +"#]]) + .with_stderr(snapbox::str![[r#" +warn: unknown key `toolchain.channnel` in '[CWD]/rust-toolchain.toml'; it is currently ignored, but will become an error in a future release + +"#]]) + .is_ok(); + + raw::write_file( + &toolchain_file, + "[toolchain]\nchannel = \"nightly\"\n\n[future]\nkey = \"value\"", + ) + .unwrap(); + cx.config + .expect(["rustup", "show", "active-toolchain"]) + .await + .extend_redactions([("[CWD]", &cwd)]) + .with_stdout(snapbox::str![[r#" +nightly-[HOST_TUPLE] (overridden by '[CWD]/rust-toolchain.toml') + +"#]]) + .with_stderr(snapbox::str![[r#" +warn: unknown key `future` in '[CWD]/rust-toolchain.toml'; it is currently ignored, but will become an error in a future release + +"#]]) + .is_ok(); + + raw::write_file(&toolchain_file, "[toolchain]\nchannnel = \"nightly\"").unwrap(); + cx.config + .expect(["rustup", "show", "active-toolchain"]) + .await + .extend_redactions([("[CWD]", &cwd)]) + .with_stdout(snapbox::str![[""]]) + .with_stderr(snapbox::str![[r#" +warn: unknown key `toolchain.channnel` in '[CWD]/rust-toolchain.toml'; it is currently ignored, but will become an error in a future release +error: could not parse override file: '[CWD]/rust-toolchain.toml': missing toolchain properties in toolchain override file + +"#]]) + .is_err(); +} + /// Ensures that `rust-toolchain.toml` files (with `.toml` extension) only allow TOML contents #[tokio::test] async fn only_toml_in_rust_toolchain_toml() {