Rollup merge of rust-lang#141027 - onur-ozkan:simpler-rustfmt-initialization, r=albertlarsan68

remove `RustfmtState` to reduce `initial_rustfmt` complexity

The current use of `RustfmtState` doesn't serve its main purpose as it never does the lazy evaulation since `Build::build` forces it to be ready on the early stage. If we want rustfmt to be ready on the early stage, we don't need to have `RustfmtState` complexity at all.

Author: matthiaskrgr

src/bootstrap/src/core/build_steps/format.rs

@@ -58,7 +58,7 @@ fn rustfmt(
 fn get_rustfmt_version(build: &Builder<'_>) -> Option<(String, BuildStamp)> {
     let stamp_file = BuildStamp::new(&build.out).with_prefix("rustfmt");
 
-    let mut cmd = command(build.initial_rustfmt()?);
+    let mut cmd = command(build.config.initial_rustfmt.as_ref()?);
     cmd.arg("--version");
 
     let output = cmd.allow_failure().run_capture(build);
@@ -243,7 +243,7 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
 
     let override_ = override_builder.build().unwrap(); // `override` is a reserved keyword
 
-    let rustfmt_path = build.initial_rustfmt().unwrap_or_else(|| {
+    let rustfmt_path = build.config.initial_rustfmt.clone().unwrap_or_else(|| {
         eprintln!("fmt error: `x fmt` is not supported on this channel");
         crate::exit!(1);
     });

src/bootstrap/src/core/build_steps/test.rs

@@ -1102,7 +1102,7 @@ impl Step for Tidy {
         if builder.config.channel == "dev" || builder.config.channel == "nightly" {
             if !builder.config.json_output {
                 builder.info("fmt check");
-                if builder.initial_rustfmt().is_none() {
+                if builder.config.initial_rustfmt.is_none() {
                     let inferred_rustfmt_dir = builder.initial_sysroot.join("bin");
                     eprintln!(
                         "\

src/bootstrap/src/core/config/config.rs

@@ -3,7 +3,7 @@
 //! This module implements parsing `bootstrap.toml` configuration files to tweak
 //! how the build runs.
 
-use std::cell::{Cell, RefCell};
+use std::cell::Cell;
 use std::collections::{BTreeSet, HashMap, HashSet};
 use std::fmt::{self, Display};
 use std::hash::Hash;
@@ -406,11 +406,7 @@ pub struct Config {
     pub initial_rustc: PathBuf,
     pub initial_cargo_clippy: Option<PathBuf>,
     pub initial_sysroot: PathBuf,
-
-    #[cfg(not(test))]
-    initial_rustfmt: RefCell<RustfmtState>,
-    #[cfg(test)]
-    pub initial_rustfmt: RefCell<RustfmtState>,
+    pub initial_rustfmt: Option<PathBuf>,
 
     /// The paths to work with. For example: with `./x check foo bar` we get
     /// `paths=["foo", "bar"]`.
@@ -428,15 +424,6 @@ pub struct Config {
     pub path_modification_cache: Arc<Mutex<HashMap<Vec<&'static str>, PathFreshness>>>,
 }
 
-#[derive(Clone, Debug, Default)]
-pub enum RustfmtState {
-    SystemToolchain(PathBuf),
-    Downloaded(PathBuf),
-    Unavailable,
-    #[default]
-    LazyEvaluated,
-}
-
 #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
 pub enum LlvmLibunwind {
     #[default]
@@ -2448,13 +2435,8 @@ impl Config {
             });
         }
 
-        if let Some(r) = rustfmt {
-            *config.initial_rustfmt.borrow_mut() = if r.exists() {
-                RustfmtState::SystemToolchain(r)
-            } else {
-                RustfmtState::Unavailable
-            };
-        }
+        config.initial_rustfmt =
+            if let Some(r) = rustfmt { Some(r) } else { config.maybe_download_rustfmt() };
 
         // Now that we've reached the end of our configuration, infer the
         // default values for all options that we haven't otherwise stored yet.
@@ -2851,25 +2833,6 @@ impl Config {
             .as_deref()
     }
 
-    pub(crate) fn initial_rustfmt(&self) -> Option<PathBuf> {
-        match &mut *self.initial_rustfmt.borrow_mut() {
-            RustfmtState::SystemToolchain(p) | RustfmtState::Downloaded(p) => Some(p.clone()),
-            RustfmtState::Unavailable => None,
-            r @ RustfmtState::LazyEvaluated => {
-                if self.dry_run() {
-                    return Some(PathBuf::new());
-                }
-                let path = self.maybe_download_rustfmt();
-                *r = if let Some(p) = &path {
-                    RustfmtState::Downloaded(p.clone())
-                } else {
-                    RustfmtState::Unavailable
-                };
-                path
-            }
-        }
-    }
-
     /// Runs a function if verbosity is greater than 0
     pub fn verbose(&self, f: impl Fn()) {
         if self.is_verbose() {

src/bootstrap/src/core/download.rs

@@ -446,7 +446,7 @@ impl Config {
 
     #[cfg(test)]
     pub(crate) fn maybe_download_rustfmt(&self) -> Option<PathBuf> {
-        None
+        Some(PathBuf::new())
     }
 
     /// NOTE: rustfmt is a completely different toolchain than the bootstrap compiler, so it can't
@@ -455,6 +455,10 @@ impl Config {
     pub(crate) fn maybe_download_rustfmt(&self) -> Option<PathBuf> {
         use build_helper::stage0_parser::VersionMetadata;
 
+        if self.dry_run() {
+            return Some(PathBuf::new());
+        }
+
         let VersionMetadata { date, version } = self.stage0_metadata.rustfmt.as_ref()?;
         let channel = format!("{version}-{date}");
 

src/bootstrap/src/lib.rs

@@ -325,7 +325,6 @@ forward! {
     tempdir() -> PathBuf,
     llvm_link_shared() -> bool,
     download_rustc() -> bool,
-    initial_rustfmt() -> Option<PathBuf>,
 }
 
 impl Build {
@@ -614,10 +613,6 @@ impl Build {
             crate::utils::job::setup(self);
         }
 
-        // Download rustfmt early so that it can be used in rust-analyzer configs.
-        trace!("downloading rustfmt early");
-        let _ = &builder::Builder::new(self).initial_rustfmt();
-
         // Handle hard-coded subcommands.
         {
             #[cfg(feature = "tracing")]