From 58d8556160cc582fd891e81351e44cae7f9d6519 Mon Sep 17 00:00:00 2001 From: ychampion Date: Tue, 21 Jul 2026 18:26:27 +0000 Subject: [PATCH 1/2] 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() { From e480ed8eadf83ea5246c031378d0cb8837d63cf8 Mon Sep 17 00:00:00 2001 From: ychampion <68075205+ychampion@users.noreply.github.com> Date: Tue, 28 Jul 2026 02:20:31 +0000 Subject: [PATCH 2/2] Keep unknown-key reporting in the parser path Constraint: Preserve warning-before-invalid behavior for unknown-only files. Confidence: high Scope-risk: narrow Tested: cargo test --lib --features=test --locked; cargo test --test test_bonanza --features=test rust_toolchain_toml_warns_on_unknown_keys --locked; cargo clippy --all-targets --features=test --locked -- -D warnings; cargo fmt --all -- --check --- src/config.rs | 86 ++++++++++++++++++++------------------------------- 1 file changed, 34 insertions(+), 52 deletions(-) diff --git a/src/config.rs b/src/config.rs index bb83d6fccd..2829cede1a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,11 +1,9 @@ -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}; @@ -292,7 +290,6 @@ 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. /// @@ -373,7 +370,6 @@ impl<'a> Cfg<'a> { quiet, current_dir, process, - warned_unknown_toolchain_keys: Mutex::new(HashSet::new()), allow_auto_install, }; @@ -692,28 +688,16 @@ 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, 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() - ); - } - } + let (override_file, unknown_keys) = Cfg::parse_override_file(contents, parse_mode) + .with_context(|| RustupError::ParsingFile { + name: "override", + path: toolchain_file.clone(), + })?; + for key in unknown_keys { + 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(|| { @@ -772,22 +756,9 @@ 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(); @@ -1138,7 +1109,6 @@ impl Debug for Cfg<'_> { quiet, current_dir, allow_auto_install, - warned_unknown_toolchain_keys: _, process: _, } = self; @@ -1263,7 +1233,7 @@ mod tests { let result = Cfg::parse_override_file(contents, ParseMode::Both); assert_eq!( - result.unwrap(), + result.unwrap().0, OverrideFile { toolchain: ToolchainSection { channel: Some(contents.into()), @@ -1287,7 +1257,7 @@ profile = "default" let result = Cfg::parse_override_file(contents, ParseMode::Both); assert_eq!( - result.unwrap(), + result.unwrap().0, OverrideFile { toolchain: ToolchainSection { channel: Some("nightly-2020-07-10".into()), @@ -1311,7 +1281,7 @@ channel = "nightly-2020-07-10" let result = Cfg::parse_override_file(contents, ParseMode::Both); assert_eq!( - result.unwrap(), + result.unwrap().0, OverrideFile { toolchain: ToolchainSection { channel: Some("nightly-2020-07-10".into()), @@ -1332,7 +1302,7 @@ path = "foobar" let result = Cfg::parse_override_file(contents, ParseMode::Both); assert_eq!( - result.unwrap(), + result.unwrap().0, OverrideFile { toolchain: ToolchainSection { channel: None, @@ -1354,7 +1324,7 @@ components = [] let result = Cfg::parse_override_file(contents, ParseMode::Both); assert_eq!( - result.unwrap(), + result.unwrap().0, OverrideFile { toolchain: ToolchainSection { channel: Some("nightly-2020-07-10".into()), @@ -1376,7 +1346,7 @@ targets = [] let result = Cfg::parse_override_file(contents, ParseMode::Both); assert_eq!( - result.unwrap(), + result.unwrap().0, OverrideFile { toolchain: ToolchainSection { channel: Some("nightly-2020-07-10".into()), @@ -1397,7 +1367,7 @@ components = [ "rustfmt" ] let result = Cfg::parse_override_file(contents, ParseMode::Both); assert_eq!( - result.unwrap(), + result.unwrap().0, OverrideFile { toolchain: ToolchainSection { channel: None, @@ -1416,11 +1386,23 @@ components = [ "rustfmt" ] [toolchain] "#; - let result = Cfg::parse_override_file(contents, ParseMode::Both); - assert!(matches!( - result.unwrap_err().downcast::(), - Ok(OverrideFileConfigError::Invalid) - )); + let (override_file, unknown_keys) = + Cfg::parse_override_file(contents, ParseMode::Both).unwrap(); + assert!(override_file.is_empty()); + assert!(unknown_keys.is_empty()); + } + + #[test] + fn parse_toml_toolchain_file_collects_unknown_keys() { + let contents = r#"[toolchain] +channel = "nightly" +channnel = "stable" +"#; + + let (override_file, unknown_keys) = + Cfg::parse_override_file(contents, ParseMode::Both).unwrap(); + assert_eq!(override_file.toolchain.channel.as_deref(), Some("nightly")); + assert_eq!(unknown_keys, ["toolchain.channnel"]); } #[test]