Do not eagerly download rustfmt in bootstrap

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

@@ -9,7 +9,8 @@ use std::sync::mpsc::SyncSender;
 use build_helper::git::get_git_modified_files;
 use ignore::WalkBuilder;
 
-use crate::core::builder::{Builder, Kind};
+use crate::core::builder::{Builder, Kind, Step};
+use crate::core::download::maybe_download_rustfmt;
 use crate::utils::build_stamp::BuildStamp;
 use crate::utils::exec::command;
 use crate::utils::helpers::{self, t};
@@ -58,7 +59,8 @@ 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.config.initial_rustfmt.as_ref()?);
+    let rustfmt = build.ensure(InternalRustfmt);
+    let mut cmd = command(rustfmt.as_ref()?);
     cmd.arg("--version");
 
     let output = cmd.allow_failure().run_capture(build);
@@ -101,6 +103,25 @@ fn get_modified_rs_files(build: &Builder<'_>) -> Result<Option<Vec<String>>, Str
     get_git_modified_files(&build.config.git_config(), Some(&build.config.src), &["rs"]).map(Some)
 }
 
+/// Rustfmt set via the config, or downloaded from CI, used to format local Rust code.
+///
+/// We never ship this rustfmt, it is designed only for internal usage.
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
+pub struct InternalRustfmt;
+
+impl Step for InternalRustfmt {
+    type Output = Option<PathBuf>;
+
+    fn run(self, builder: &Builder<'_>) -> Self::Output {
+        // Rustfmt configured through the config
+        if let Some(initial_rustfmt) = &builder.config.external_rustfmt {
+            return Some(initial_rustfmt.clone());
+        }
+        // No rustfmt was configured, try to download it
+        maybe_download_rustfmt(&builder.config, &builder.config.out)
+    }
+}
+
 #[derive(serde_derive::Deserialize)]
 struct RustfmtConfig {
     ignore: Vec<String>,
@@ -121,7 +142,13 @@ fn print_paths(verb: &str, adjective: Option<&str>, paths: &[String]) {
     }
 }
 
-pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
+pub fn format(
+    build: &Builder<'_>,
+    rustfmt_path: PathBuf,
+    check: bool,
+    all: bool,
+    paths: &[PathBuf],
+) {
     if build.kind == Kind::Format && build.top_stage != 0 {
         eprintln!("ERROR: `x fmt` only supports stage 0.");
         eprintln!("HELP: Use `x run rustfmt` to run in-tree rustfmt.");
@@ -243,10 +270,6 @@ 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.config.initial_rustfmt.clone().unwrap_or_else(|| {
-        eprintln!("fmt error: `x fmt` is not supported on this channel");
-        crate::exit!(1);
-    });
     assert!(rustfmt_path.exists(), "{}", rustfmt_path.display());
     let src = build.src.clone();
     let (tx, rx): (SyncSender<PathBuf>, _) = std::sync::mpsc::sync_channel(128);

src/bootstrap/src/core/build_steps/setup.rs

@@ -18,6 +18,7 @@ use std::{fmt, fs, io};
 use serde_derive::{Deserialize, Serialize};
 use sha2::Digest;
 
+use crate::core::build_steps::format;
 use crate::core::builder::{Builder, CommandLineStep, RunConfig, ShouldRun};
 use crate::utils::change_tracker::CONFIG_CHANGE_HISTORY;
 use crate::utils::exec::command;
@@ -676,6 +677,10 @@ impl CommandLineStep for Editor {
             Ok(editor_kind) => {
                 if let Some(editor_kind) = editor_kind {
                     while !t!(create_editor_settings_maybe(config, &editor_kind)) {}
+
+                    // Also pre-download stage 0 rustfmt, so that the IDE configs which point to
+                    // `build/host/rustfmt` have an available binary to work with.
+                    builder.ensure(format::InternalRustfmt);
                 } else {
                     println!("Ok, skipping editor setup!");
                 }

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

@@ -17,6 +17,7 @@ use build_helper::git::get_closest_upstream_commit;
 
 use crate::core::build_steps::compile::{ArtifactKeepMode, Std, run_cargo};
 use crate::core::build_steps::doc::{DocumentationFormat, prepare_doc_compiler};
+use crate::core::build_steps::format::InternalRustfmt;
 use crate::core::build_steps::gcc::{Gcc, GccTargetPair, add_cg_gcc_cargo_flags};
 use crate::core::build_steps::llvm::get_llvm_version;
 use crate::core::build_steps::run::{get_completion_paths, get_help_path};
@@ -1095,7 +1096,7 @@ impl CommandLineStep for IntrinsicTest {
         cmd.env("CFLAGS", cflags);
         // intrinsic-test shells out to `cargo` and `rustfmt` make bootstrap's
         // managed binaries findable by prepending their dirs to PATH.
-        let Some(rustfmt_path) = builder.config.initial_rustfmt.clone() else {
+        let Some(rustfmt_path) = builder.ensure(InternalRustfmt) else {
             eprintln!(
                 "WARNING: intrinsic-test skipped because rustfmt is required but not available on this channel"
             );
@@ -1629,7 +1630,10 @@ impl CommandLineStep for Tidy {
         if builder.config.channel == "dev" || builder.config.channel == "nightly" {
             if !builder.config.json_output {
                 builder.info("fmt check");
-                if builder.config.initial_rustfmt.is_none() {
+
+                // Note: this actually sets up or downloads rustfmt, so running this step here is
+                // load-bearing
+                let Some(rustfmt) = builder.ensure(InternalRustfmt) else {
                     let inferred_rustfmt_dir = builder.initial_sysroot.join("bin");
                     eprintln!(
                         "\
@@ -1641,10 +1645,11 @@ HELP: to skip test's attempt to check tidiness, pass `--skip src/tools/tidy` to
                         CHAN = builder.config.channel,
                     );
                     crate::exit!(1);
-                }
+                };
                 let all = false;
                 crate::core::build_steps::format::format(
                     builder,
+                    rustfmt,
                     !builder.config.cmd.bless(),
                     all,
                     &[],

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

@@ -53,9 +53,7 @@ use crate::core::config::{
     GccCiMode, LlvmLibunwind, Merge, ReplaceOpt, RustcLto, SplitDebuginfo, StringOrBool,
     threads_from_config,
 };
-use crate::core::download::{
-    DownloadContext, download_beta_toolchain, is_download_ci_available, maybe_download_rustfmt,
-};
+use crate::core::download::{DownloadContext, download_beta_toolchain, is_download_ci_available};
 use crate::utils::channel;
 use crate::utils::exec::{ExecutionContext, command};
 use crate::utils::helpers::{exe, fail, get_host_target};
@@ -310,7 +308,11 @@ pub struct Config {
     pub initial_rustdoc: PathBuf,
     pub initial_cargo_clippy: Option<PathBuf>,
     pub initial_sysroot: PathBuf,
-    pub initial_rustfmt: Option<PathBuf>,
+
+    /// Externally configured `rustfmt` binary for formatting in-tree source code.
+    /// If you want to use rustfmt for formatting, use the `InternalRustfmt` step, instead of
+    /// accessing this directly.
+    pub external_rustfmt: Option<PathBuf>,
 
     /// The paths to work with. For example: with `./x check foo bar` we get
     /// `paths=["foo", "bar"]`.
@@ -1168,8 +1170,6 @@ impl Config {
             }
         }
 
-        let initial_rustfmt = build_rustfmt.or_else(|| maybe_download_rustfmt(&dwn_ctx, &out));
-
         if matches!(bootstrap_override_lld, BootstrapOverrideLld::SelfContained)
             && !lld_enabled
             && flags_stage.unwrap_or(0) > 0
@@ -1447,6 +1447,7 @@ NOTE: Please add `--stage 2` to your command line, or if you're sure you want to
             explicit_stage_from_cli: flags_stage.is_some(),
             explicit_stage_from_config,
             extended: build_extended.unwrap_or(false),
+            external_rustfmt: build_rustfmt,
             free_args: flags_free_args,
             full_bootstrap: build_full_bootstrap.unwrap_or(false),
             gcc_ci_mode,
@@ -1461,7 +1462,6 @@ NOTE: Please add `--stage 2` to your command line, or if you're sure you want to
             initial_cargo_clippy: build_cargo_clippy,
             initial_rustc,
             initial_rustdoc,
-            initial_rustfmt,
             initial_sysroot,
             jobs: Some(threads_from_config(flags_jobs.or(build_jobs).unwrap_or(0))),
             json_output: flags_json_output,

src/bootstrap/src/core/download.rs

@@ -530,25 +530,20 @@ pub(crate) fn is_download_ci_available(target_triple: &str, llvm_assertions: boo
 
 /// NOTE: rustfmt is a completely different toolchain than the bootstrap compiler, so it can't
 /// reuse target directories or artifacts
-pub(crate) fn maybe_download_rustfmt<'a>(
-    dwn_ctx: impl AsRef<DownloadContext<'a>>,
-    out: &Path,
-) -> Option<PathBuf> {
+pub(crate) fn maybe_download_rustfmt(config: &Config, out: &Path) -> Option<PathBuf> {
     // Don't actually download rustfmt during unit tests.
     if cfg!(test) {
         return Some(PathBuf::new());
     }
 
-    let dwn_ctx = dwn_ctx.as_ref();
-
-    if dwn_ctx.exec_ctx.dry_run() {
+    if config.dry_run() {
         return Some(PathBuf::new());
     }
 
-    let VersionMetadata { date, version, .. } = dwn_ctx.stage0_metadata.rustfmt.as_ref()?;
+    let VersionMetadata { date, version, .. } = config.stage0_metadata.rustfmt.as_ref()?;
     let channel = format!("{version}-{date}");
 
-    let host = dwn_ctx.host_target;
+    let host = config.host_target;
     let bin_root = out.join(host).join("rustfmt");
     let rustfmt_path = bin_root.join("bin").join(exe("rustfmt", host));
     let rustfmt_stamp = BuildStamp::new(&bin_root).with_prefix("rustfmt").add_stamp(channel);
@@ -557,7 +552,7 @@ pub(crate) fn maybe_download_rustfmt<'a>(
     }
 
     download_component(
-        dwn_ctx,
+        DownloadContext::from(config),
         out,
         DownloadSource::Dist,
         format!("rustfmt-{version}-{build}.tar.xz", build = host.triple),
@@ -567,7 +562,7 @@ pub(crate) fn maybe_download_rustfmt<'a>(
     );
 
     download_component(
-        dwn_ctx,
+        DownloadContext::from(config),
         out,
         DownloadSource::Dist,
         format!("rustc-{version}-{build}.tar.xz", build = host.triple),
@@ -576,14 +571,14 @@ pub(crate) fn maybe_download_rustfmt<'a>(
         "rustfmt",
     );
 
-    if should_fix_bins_and_dylibs(dwn_ctx.patch_binaries_for_nix, dwn_ctx.exec_ctx) {
-        fix_bin_or_dylib(out, &bin_root.join("bin").join("rustfmt"), dwn_ctx.exec_ctx);
-        fix_bin_or_dylib(out, &bin_root.join("bin").join("cargo-fmt"), dwn_ctx.exec_ctx);
+    if should_fix_bins_and_dylibs(config.patch_binaries_for_nix, &config.exec_ctx) {
+        fix_bin_or_dylib(out, &bin_root.join("bin").join("rustfmt"), &config.exec_ctx);
+        fix_bin_or_dylib(out, &bin_root.join("bin").join("cargo-fmt"), &config.exec_ctx);
         let lib_dir = bin_root.join("lib");
         for lib in t!(fs::read_dir(&lib_dir), lib_dir.display().to_string()) {
             let lib = t!(lib);
             if path_is_dylib(&lib.path()) {
-                fix_bin_or_dylib(out, &lib.path(), dwn_ctx.exec_ctx);
+                fix_bin_or_dylib(out, &lib.path(), &config.exec_ctx);
             }
         }
     }

src/bootstrap/src/lib.rs

@@ -32,6 +32,7 @@ use utils::build_stamp::BuildStamp;
 use utils::channel::GitInfo;
 use utils::exec::ExecutionContext;
 
+use crate::core::build_steps::format::InternalRustfmt;
 use crate::core::builder;
 use crate::core::builder::Kind;
 use crate::core::config::{BootstrapOverrideLld, DryRun, LlvmLibunwind, TargetSelection, flags};
@@ -759,8 +760,14 @@ impl Build {
 
             match &self.config.cmd {
                 Subcommand::Format { check, all } => {
+                    let builder = builder::Builder::new(self);
+                    let rustfmt_path = builder.ensure(InternalRustfmt).unwrap_or_else(|| {
+                        eprintln!("fmt error: `x fmt` is not supported on this channel");
+                        crate::exit!(1);
+                    });
                     return core::build_steps::format::format(
-                        &builder::Builder::new(self),
+                        &builder,
+                        rustfmt_path,
                         *check,
                         *all,
                         &self.config.paths,