Merge pull request #2653 from phil-opp/rust_toolchain_toml

Allow `.toml` extension for `rust-toolchain` files

doc/src/overrides.md

@@ -8,14 +8,14 @@ and override which toolchain is used:
    +beta`.
 2. The `RUSTUP_TOOLCHAIN` environment variable.
 3. A [directory override], set with the `rustup override` command.
-4. The [`rust-toolchain`] file.
+4. The [`rust-toolchain.toml`] file.
 5. The [default toolchain].
 
 The toolchain is chosen in the order listed above, using the first one that is
 specified. There is one exception though: directory overrides and the
-`rust-toolchain` file are also preferred by their proximity to the current
+`rust-toolchain.toml` file are also preferred by their proximity to the current
 directory. That is, these two override methods are discovered by walking up
-the directory tree toward the filesystem root, and a `rust-toolchain` file
+the directory tree toward the filesystem root, and a `rust-toolchain.toml` file
 that is closer to the current directory will be preferred over a directory
 override that is further away.
 
@@ -24,7 +24,7 @@ To verify which toolchain is active use `rustup show`.
 [toolchain]: concepts/toolchains.md
 [toolchain override shorthand]: #toolchain-override-shorthand
 [directory override]: #directory-overrides
-[`rust-toolchain`]: #the-toolchain-file
+[`rust-toolchain.toml`]: #the-toolchain-file
 [default toolchain]: #default-toolchain
 
 ## Toolchain override shorthand
@@ -74,8 +74,11 @@ 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`, the content of which is either the name of a single
-`rustup` toolchain, or a TOML file with the following layout:
+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:
+
+[TOML]: https://toml.io/
 
 ``` toml
 [toolchain]
@@ -85,14 +88,19 @@ targets = [ "wasm32-unknown-unknown", "thumbv2-none-eabi" ]
 profile = "minimal"
 ```
 
-If the TOML format is used, the `[toolchain]` section is mandatory, and at
-least one property must be specified.
+The `[toolchain]` section is mandatory, and at least one property must be
+specified.
+
+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 this case
+(if you are on Windows, check the encoding and that it does not starts with a
+BOM). The legacy format is not available in `rust-toolchain.toml` files.
 
-The `rust-toolchain` file is suitable to check in to source control. This file
-has to be encoded in US-ASCII (if you are on Windows, check the encoding and
-that it does not starts with a BOM).
+The `rust-toolchain.toml`/`rust-toolchain` files are suitable to check in to
+source control.
 
-The toolchains named in this file have a more restricted form than `rustup`
+The toolchains named in these files have a more restricted form than `rustup`
 toolchains generally, and may only contain the names of the three release
 channels, 'stable', 'beta', 'nightly', Rust version numbers, like '1.0.0', and
 optionally an archive date, like 'nightly-2017-01-01'. They may not name

src/config.rs

@@ -567,10 +567,36 @@ impl Cfg {
                 return Ok(Some((name.into(), reason)));
             }
 
-            // Then look for 'rust-toolchain'
-            let toolchain_file = d.join("rust-toolchain");
-            if let Ok(contents) = utils::read_file("toolchain file", &toolchain_file) {
-                let override_file = Cfg::parse_override_file(contents)?;
+            // 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
+
+                    notify(Notification::DuplicateToolchainFile {
+                        rust_toolchain: &path_rust_toolchain,
+                        rust_toolchain_toml: &path_rust_toolchain_toml,
+                    });
+
+                    (path_rust_toolchain, Ok(contents), ParseMode::Both)
+                }
+            };
+
+            if let Ok(contents) = contents {
+                let override_file = Cfg::parse_override_file(contents, parse_mode)?;
                 if let Some(toolchain_name) = &override_file.toolchain.channel {
                     let all_toolchains = self.list_toolchains()?;
                     if !all_toolchains.iter().any(|s| s == toolchain_name) {
@@ -590,12 +616,15 @@ impl Cfg {
         Ok(None)
     }
 
-    fn parse_override_file<S: AsRef<str>>(contents: S) -> Result<OverrideFile> {
+    fn parse_override_file<S: AsRef<str>>(
+        contents: S,
+        parse_mode: ParseMode,
+    ) -> Result<OverrideFile> {
         let contents = contents.as_ref();
 
-        match contents.lines().count() {
-            0 => Err(ErrorKind::EmptyOverrideFile.into()),
-            1 => {
+        match (contents.lines().count(), parse_mode) {
+            (0, _) => Err(ErrorKind::EmptyOverrideFile.into()),
+            (1, ParseMode::Both) => {
                 let channel = contents.trim();
 
                 if channel.is_empty() {
@@ -898,6 +927,20 @@ impl Cfg {
     }
 }
 
+/// Specifies how a `rust-toolchain`/`rust-toolchain.toml` configuration file should be parsed.
+enum ParseMode {
+    /// Only permit TOML format in a configuration file.
+    ///
+    /// This variant is used for `rust-toolchain.toml` files (with `.toml` extension).
+    OnlyToml,
+    /// Permit both the legacy format (i.e. just the channel name) and the TOML format in
+    /// a configuration file.
+    ///
+    /// This variant is used for `rust-toolchain` files (no file extension) for backwards
+    /// compatibility.
+    Both,
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -906,7 +949,7 @@ mod tests {
     fn parse_legacy_toolchain_file() {
         let contents = "nightly-2020-07-10";
 
-        let result = Cfg::parse_override_file(contents);
+        let result = Cfg::parse_override_file(contents, ParseMode::Both);
         assert_eq!(
             result.unwrap(),
             OverrideFile {
@@ -929,7 +972,7 @@ targets = [ "wasm32-unknown-unknown", "thumbv2-none-eabi" ]
 profile = "default"
 "#;
 
-        let result = Cfg::parse_override_file(contents);
+        let result = Cfg::parse_override_file(contents, ParseMode::Both);
         assert_eq!(
             result.unwrap(),
             OverrideFile {
@@ -952,7 +995,7 @@ profile = "default"
 channel = "nightly-2020-07-10"
 "#;
 
-        let result = Cfg::parse_override_file(contents);
+        let result = Cfg::parse_override_file(contents, ParseMode::Both);
         assert_eq!(
             result.unwrap(),
             OverrideFile {
@@ -973,7 +1016,7 @@ channel = "nightly-2020-07-10"
 components = []
 "#;
 
-        let result = Cfg::parse_override_file(contents);
+        let result = Cfg::parse_override_file(contents, ParseMode::Both);
         assert_eq!(
             result.unwrap(),
             OverrideFile {
@@ -994,7 +1037,7 @@ channel = "nightly-2020-07-10"
 targets = []
 "#;
 
-        let result = Cfg::parse_override_file(contents);
+        let result = Cfg::parse_override_file(contents, ParseMode::Both);
         assert_eq!(
             result.unwrap(),
             OverrideFile {
@@ -1014,7 +1057,7 @@ targets = []
 components = [ "rustfmt" ]
 "#;
 
-        let result = Cfg::parse_override_file(contents);
+        let result = Cfg::parse_override_file(contents, ParseMode::Both);
         assert_eq!(
             result.unwrap(),
             OverrideFile {
@@ -1034,7 +1077,7 @@ components = [ "rustfmt" ]
 [toolchain]
 "#;
 
-        let result = Cfg::parse_override_file(contents);
+        let result = Cfg::parse_override_file(contents, ParseMode::Both);
         assert!(matches!(
             result.unwrap_err().kind(),
             ErrorKind::InvalidOverrideFile
@@ -1045,7 +1088,7 @@ components = [ "rustfmt" ]
     fn parse_empty_toolchain_file() {
         let contents = "";
 
-        let result = Cfg::parse_override_file(contents);
+        let result = Cfg::parse_override_file(contents, ParseMode::Both);
         assert!(matches!(
             result.unwrap_err().kind(),
             ErrorKind::EmptyOverrideFile
@@ -1056,7 +1099,7 @@ components = [ "rustfmt" ]
     fn parse_whitespace_toolchain_file() {
         let contents = "   ";
 
-        let result = Cfg::parse_override_file(contents);
+        let result = Cfg::parse_override_file(contents, ParseMode::Both);
         assert!(matches!(
             result.unwrap_err().kind(),
             ErrorKind::EmptyOverrideFile
@@ -1069,7 +1112,7 @@ components = [ "rustfmt" ]
 channel = nightly
 "#;
 
-        let result = Cfg::parse_override_file(contents);
+        let result = Cfg::parse_override_file(contents, ParseMode::Both);
         assert!(matches!(
             result.unwrap_err().kind(),
             ErrorKind::ParsingOverrideFile(..)

src/notifications.rs

@@ -33,6 +33,11 @@ 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,
+    },
 }
 
 impl<'a> From<crate::dist::Notification<'a>> for Notification<'a> {
@@ -77,7 +82,9 @@ impl<'a> Notification<'a> {
             | UpgradingMetadata(_, _)
             | MetadataUpgradeNotNeeded(_) => NotificationLevel::Info,
             NonFatalError(_) => NotificationLevel::Error,
-            UpgradeRemovesToolchains | MissingFileDuringSelfUninstall(_) => NotificationLevel::Warn,
+            UpgradeRemovesToolchains
+            | MissingFileDuringSelfUninstall(_)
+            | DuplicateToolchainFile { .. } => NotificationLevel::Warn,
         }
     }
 }
@@ -130,6 +137,15 @@ 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.display(),
+                rust_toolchain_toml.display()
+            ),
         }
     }
 }

tests/cli-rustup.rs

@@ -2065,3 +2065,59 @@ warning: If you meant to build software to target that platform, perhaps try `ru
         );
     });
 }
+
+/// Checks that `rust-toolchain.toml` files are considered
+#[test]
+fn rust_toolchain_toml() {
+    setup(&|config| {
+        expect_err(
+            config,
+            &["rustc", "--version"],
+            "no override and no default toolchain set",
+        );
+
+        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_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() {
+    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(),
+            ),
+        );
+    });
+}