From 4866b41f5e05a08affad2513dcb5750e057d792f Mon Sep 17 00:00:00 2001 From: ZSchoen Date: Sun, 5 Feb 2023 17:22:32 +0100 Subject: [PATCH 1/6] added initial support for multiple toolchain files --- src/config.rs | 54 +++++++++++++++++++++++--------------------- src/notifications.rs | 39 +++++++++++++++----------------- 2 files changed, 46 insertions(+), 47 deletions(-) diff --git a/src/config.rs b/src/config.rs index d869efa410..1c02f45f35 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,4 +1,5 @@ use std::borrow::Cow; +use std::collections::VecDeque; use std::fmt::{self, Display}; use std::io; use std::path::{Path, PathBuf}; @@ -650,35 +651,36 @@ impl Cfg { return Ok(Some((name.into(), reason))); } - // Then look for 'rust-toolchain' or 'rust-toolchain.toml' - let path_rust_toolchain = d.join("rust-toolchain"); - let path_rust_toolchain_toml = d.join("rust-toolchain.toml"); - - let (toolchain_file, contents, parse_mode) = match ( - utils::read_file("toolchain file", &path_rust_toolchain), - utils::read_file("toolchain file", &path_rust_toolchain_toml), - ) { - (contents, Err(_)) => { - // no `rust-toolchain.toml` exists - (path_rust_toolchain, contents, ParseMode::Both) - } - (Err(_), Ok(contents)) => { - // only `rust-toolchain.toml` exists - (path_rust_toolchain_toml, Ok(contents), ParseMode::OnlyToml) - } - (Ok(contents), Ok(_)) => { - // both `rust-toolchain` and `rust-toolchain.toml` exist + let mut toolchain_files: VecDeque<_> = [ + "rust-toolchain", + "rust-toolchain.toml", + ".rust-toolchain.toml", + ] + .into_iter() + .filter_map(|toolchain_file_name| { + let toolchain_file_path = d.join(toolchain_file_name); + let content = utils::read_file("toolchain file", &toolchain_file_path).ok()?; + let parse_mode = if toolchain_file_name.ends_with(".toml") { + ParseMode::OnlyToml + } else { + ParseMode::Both + }; + Some((toolchain_file_path, content, parse_mode)) + }) + .collect(); - notify(Notification::DuplicateToolchainFile { - rust_toolchain: &path_rust_toolchain, - rust_toolchain_toml: &path_rust_toolchain_toml, - }); + if toolchain_files.len() > 1 { + notify(Notification::MultipleToolchainFiles( + toolchain_files + .iter() + .map(|(file_path, _, _)| file_path.as_path()) + .collect(), + )); + } - (path_rust_toolchain, Ok(contents), ParseMode::Both) - } - }; + let first_toolchain_file = toolchain_files.pop_front(); - if let Ok(contents) = contents { + if let Some((toolchain_file, contents, parse_mode)) = first_toolchain_file { let add_file_context = || format!("in {}", toolchain_file.to_string_lossy()); let override_file = Cfg::parse_override_file(contents, parse_mode) .with_context(add_file_context)?; diff --git a/src/notifications.rs b/src/notifications.rs index a1b171898f..cbef6ad493 100644 --- a/src/notifications.rs +++ b/src/notifications.rs @@ -32,11 +32,7 @@ pub enum Notification<'a> { UpgradeRemovesToolchains, MissingFileDuringSelfUninstall(PathBuf), PlainVerboseMessage(&'a str), - /// Both `rust-toolchain` and `rust-toolchain.toml` exist within a directory - DuplicateToolchainFile { - rust_toolchain: &'a Path, - rust_toolchain_toml: &'a Path, - }, + MultipleToolchainFiles(Vec<&'a Path>), } impl<'a> From> for Notification<'a> { @@ -84,7 +80,7 @@ impl<'a> Notification<'a> { NonFatalError(_) => NotificationLevel::Error, UpgradeRemovesToolchains | MissingFileDuringSelfUninstall(_) - | DuplicateToolchainFile { .. } => NotificationLevel::Warn, + | MultipleToolchainFiles(_) => NotificationLevel::Warn, } } } @@ -139,21 +135,22 @@ impl<'a> Display for Notification<'a> { p.display() ), PlainVerboseMessage(r) => write!(f, "{}", r), - DuplicateToolchainFile { - rust_toolchain, - rust_toolchain_toml, - } => write!( - f, - "both `{0}` and `{1}` exist. Using `{0}`", - rust_toolchain - .canonicalize() - .unwrap_or_else(|_| PathBuf::from(rust_toolchain)) - .display(), - rust_toolchain_toml - .canonicalize() - .unwrap_or_else(|_| PathBuf::from(rust_toolchain_toml)) - .display(), - ), + MultipleToolchainFiles(rust_toolchain_paths) => { + assert!(rust_toolchain_paths.len() > 1); + let used_path = rust_toolchain_paths[0]; + let all_paths = rust_toolchain_paths + .iter() + .skip(1) + .fold(format!("`{}`", used_path.display()), |all_paths, path| { + format!("{} and `{}`", all_paths, path.display()) + }); + write!( + f, + "both {} exist. Using `{}`", + all_paths, + used_path.display() + ) + } } } } From 415a61418fba8317a4db977996c839792e537339 Mon Sep 17 00:00:00 2001 From: ZSchoen Date: Sun, 5 Feb 2023 17:59:21 +0100 Subject: [PATCH 2/6] readded canonicalize for displaying MultipleToolchainFiles --- src/notifications.rs | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/notifications.rs b/src/notifications.rs index cbef6ad493..37d4393db1 100644 --- a/src/notifications.rs +++ b/src/notifications.rs @@ -137,13 +137,19 @@ impl<'a> Display for Notification<'a> { PlainVerboseMessage(r) => write!(f, "{}", r), MultipleToolchainFiles(rust_toolchain_paths) => { assert!(rust_toolchain_paths.len() > 1); - let used_path = rust_toolchain_paths[0]; - let all_paths = rust_toolchain_paths - .iter() - .skip(1) - .fold(format!("`{}`", used_path.display()), |all_paths, path| { - format!("{} and `{}`", all_paths, path.display()) - }); + let canonicalize_when_possible = + |path: &Path| path.canonicalize().unwrap_or_else(|_| path.to_path_buf()); + let used_path = canonicalize_when_possible(rust_toolchain_paths[0]); + let all_paths = rust_toolchain_paths.iter().skip(1).fold( + format!("`{}`", used_path.display()), + |all_paths, path| { + format!( + "{} and `{}`", + all_paths, + canonicalize_when_possible(path).display() + ) + }, + ); write!( f, "both {} exist. Using `{}`", From cffad5f22e5e1364ef57d203f5156a16985d1bdd Mon Sep 17 00:00:00 2001 From: ZSchoen Date: Sun, 5 Feb 2023 18:19:06 +0100 Subject: [PATCH 3/6] added test cases --- tests/cli-rustup.rs | 78 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/tests/cli-rustup.rs b/tests/cli-rustup.rs index 29c3345484..49defe0597 100644 --- a/tests/cli-rustup.rs +++ b/tests/cli-rustup.rs @@ -2242,6 +2242,40 @@ fn only_toml_in_rust_toolchain_toml() { }); } +/// Checks that `.rust-toolchain.toml` files are considered +#[test] +fn rust_hidden_toolchain_toml() { + setup(&|config| { + expect_err( + config, + &["rustc", "--version"], + "rustup could not choose a version of rustc to run", + ); + + let cwd = config.current_dir(); + let toolchain_file = cwd.join(".rust-toolchain.toml"); + raw::write_file(&toolchain_file, "[toolchain]\nchannel = \"nightly\"").unwrap(); + + expect_stdout_ok(config, &["rustc", "--version"], "hash-nightly-2"); + }); +} + +/// Ensures that `.rust-toolchain.toml` files (with `.toml` extension) only allow TOML contents +#[test] +fn only_toml_in_rust_hidden_toolchain_toml() { + setup(&|config| { + let cwd = config.current_dir(); + let toolchain_file = cwd.join(".rust-toolchain.toml"); + raw::write_file(&toolchain_file, "nightly").unwrap(); + + expect_err( + config, + &["rustc", "--version"], + "error parsing override file", + ); + }); +} + /// Checks that a warning occurs if both `rust-toolchain` and `rust-toolchain.toml` files exist #[test] fn warn_on_duplicate_rust_toolchain_file() { @@ -2263,3 +2297,47 @@ fn warn_on_duplicate_rust_toolchain_file() { ); }); } + +/// Checks that a warning occurs if both `rust-toolchain` and `.rust-toolchain.toml` files exist +#[test] +fn warn_on_duplicate_rust_toolchain_and_hidden_file() { + setup(&|config| { + let cwd = config.current_dir(); + let toolchain_file_1 = cwd.join("rust-toolchain"); + raw::write_file(&toolchain_file_1, "stable").unwrap(); + let toolchain_file_2 = cwd.join(".rust-toolchain.toml"); + raw::write_file(&toolchain_file_2, "[toolchain]").unwrap(); + + expect_stderr_ok( + config, + &["rustc", "--version"], + &format!( + "warning: both `{0}` and `{1}` exist. Using `{0}`", + toolchain_file_1.canonicalize().unwrap().display(), + toolchain_file_2.canonicalize().unwrap().display(), + ), + ); + }); +} + +/// Checks that a warning occurs if both `rust-toolchain.toml` and `.rust-toolchain.toml` files exist +#[test] +fn warn_on_duplicate_rust_toolchain_toml_file() { + setup(&|config| { + let cwd = config.current_dir(); + let toolchain_file_1 = cwd.join("rust-toolchain.toml"); + raw::write_file(&toolchain_file_1, "[toolchain]\nchannel=\"stable\"").unwrap(); + let toolchain_file_2 = cwd.join(".rust-toolchain.toml"); + raw::write_file(&toolchain_file_2, "[toolchain]").unwrap(); + + expect_stderr_ok( + config, + &["rustc", "--version"], + &format!( + "warning: both `{0}` and `{1}` exist. Using `{0}`", + toolchain_file_1.canonicalize().unwrap().display(), + toolchain_file_2.canonicalize().unwrap().display(), + ), + ); + }); +} From 8f21f937f831e433b16d6cbbfa22306b3f30e225 Mon Sep 17 00:00:00 2001 From: ZSchoen Date: Mon, 13 Feb 2023 10:46:44 +0100 Subject: [PATCH 4/6] added hidden toolchain file to overrides documentation --- doc/src/overrides.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/doc/src/overrides.md b/doc/src/overrides.md index 3115b96bfc..eef08b491e 100644 --- a/doc/src/overrides.md +++ b/doc/src/overrides.md @@ -77,9 +77,10 @@ case for nightly-only software that pins to a revision from the release archives. In these cases the toolchain can be named in the project's directory in a file -called `rust-toolchain.toml` or `rust-toolchain`. If both files are present in -a directory, the latter is used for backwards compatibility. The files use the -[TOML] format and have the following layout: +called `.rust-toolchain.toml`, `rust-toolchain.toml` or `rust-toolchain`. +If multiple files are present in a directory, the latter present one is used +for backwards compatibility. The files use the [TOML] format and have the +following layout: [TOML]: https://toml.io/ @@ -110,10 +111,10 @@ it means you're running `rustup` pre-1.23.0 and trying to interact with a projec that uses the new TOML encoding in the `rust-toolchain` file. You need to upgrade `rustup` to 1.23.0+. -The `rust-toolchain.toml`/`rust-toolchain` files are suitable to check in to -source control. If that's done, `Cargo.lock` should probably be tracked too if -the toolchain is pinned to a specific release, to avoid potential compatibility -issues with dependencies. +The `.rust-toolchain.toml`/`rust-toolchain.toml`/`rust-toolchain` files are +suitable to check in to source control. If that's done, `Cargo.lock` should +probably be tracked too if the toolchain is pinned to a specific release, to +avoid potential compatibility issues with dependencies. ### Toolchain file settings From 82bfb7786bccfdbdf4471529eb7516884d7f5757 Mon Sep 17 00:00:00 2001 From: zSchoen Date: Mon, 13 Feb 2023 12:32:07 +0100 Subject: [PATCH 5/6] added hidden toolchain file to current format --- doc/src/overrides.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/src/overrides.md b/doc/src/overrides.md index eef08b491e..018ac66ab8 100644 --- a/doc/src/overrides.md +++ b/doc/src/overrides.md @@ -99,7 +99,8 @@ For backwards compatibility, `rust-toolchain` files also support a legacy format that only contains a toolchain name without any TOML encoding, e.g. just `nightly-2021-01-21`. The file has to be encoded in US-ASCII in this case (if you are on Windows, check the encoding and that it does not start with a -BOM). The legacy format is not available in `rust-toolchain.toml` files. +BOM). The legacy format is not available in `.rust-toolchain.toml`/ +`rust-toolchain.toml` files. If you see the following error (when running `rustc`, `cargo` or other command) From aa6e7d900b8aa41e400b4796098738cd6c379b6b Mon Sep 17 00:00:00 2001 From: zSchoen Date: Mon, 13 Feb 2023 12:42:05 +0100 Subject: [PATCH 6/6] fixed clearer toolchain file precedence in override docs --- doc/src/overrides.md | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/doc/src/overrides.md b/doc/src/overrides.md index 018ac66ab8..cdebc57c59 100644 --- a/doc/src/overrides.md +++ b/doc/src/overrides.md @@ -77,10 +77,15 @@ case for nightly-only software that pins to a revision from the release archives. In these cases the toolchain can be named in the project's directory in a file -called `.rust-toolchain.toml`, `rust-toolchain.toml` or `rust-toolchain`. -If multiple files are present in a directory, the latter present one is used -for backwards compatibility. The files use the [TOML] format and have the -following layout: +called `.rust-toolchain.toml`, `rust-toolchain.toml` or `rust-toolchain`. If +multiple files are present in a directory, the following precedence will be +applied for backwards compatibility: + +1. `rust-toolchain` +2. `rust-toolchain.toml` +3. `.rust-toolchain.toml` + +The files use the [TOML] format and have the following layout: [TOML]: https://toml.io/