Move beta rustfmt downloads to rustbuild

Author: jyn514

src/bootstrap/bootstrap.py

@@ -63,31 +63,30 @@ def support_xz():
     except tarfile.CompressionError:
         return False
 
-def get(base, url, path, checksums, verbose=False, do_verify=True):
+def get(base, url, path, checksums, verbose=False):
     with tempfile.NamedTemporaryFile(delete=False) as temp_file:
         temp_path = temp_file.name
 
     try:
-        if do_verify:
-            if url not in checksums:
-                raise RuntimeError(("src/stage0.json doesn't contain a checksum for {}. "
-                                    "Pre-built artifacts might not available for this "
-                                    "target at this time, see https://doc.rust-lang.org/nightly"
-                                    "/rustc/platform-support.html for more information.")
-                                   .format(url))
-            sha256 = checksums[url]
-            if os.path.exists(path):
-                if verify(path, sha256, False):
-                    if verbose:
-                        print("using already-download file", path)
-                    return
-                else:
-                    if verbose:
-                        print("ignoring already-download file",
-                            path, "due to failed verification")
-                    os.unlink(path)
+        if url not in checksums:
+            raise RuntimeError(("src/stage0.json doesn't contain a checksum for {}. "
+                                "Pre-built artifacts might not be available for this "
+                                "target at this time, see https://doc.rust-lang.org/nightly"
+                                "/rustc/platform-support.html for more information.")
+                                .format(url))
+        sha256 = checksums[url]
+        if os.path.exists(path):
+            if verify(path, sha256, False):
+                if verbose:
+                    print("using already-download file", path)
+                return
+            else:
+                if verbose:
+                    print("ignoring already-download file",
+                        path, "due to failed verification")
+                os.unlink(path)
         download(temp_path, "{}/{}".format(base, url), True, verbose)
-        if do_verify and not verify(temp_path, sha256, verbose):
+        if not verify(temp_path, sha256, verbose):
             raise RuntimeError("failed verification")
         if verbose:
             print("moving {} to {}".format(temp_path, path))
@@ -430,7 +429,6 @@ class RustBuild(object):
     def __init__(self):
         self.checksums_sha256 = {}
         self.stage0_compiler = None
-        self.stage0_rustfmt = None
         self._download_url = ''
         self.build = ''
         self.build_dir = ''
@@ -484,31 +482,10 @@ def download_toolchain(self):
             with output(self.rustc_stamp()) as rust_stamp:
                 rust_stamp.write(key)
 
-        if self.rustfmt() and self.rustfmt().startswith(bin_root) and (
-            not os.path.exists(self.rustfmt())
-            or self.program_out_of_date(
-                self.rustfmt_stamp(),
-                "" if self.stage0_rustfmt is None else self.stage0_rustfmt.channel()
-            )
-        ):
-            if self.stage0_rustfmt is not None:
-                tarball_suffix = '.tar.xz' if support_xz() else '.tar.gz'
-                filename = "rustfmt-{}-{}{}".format(
-                    self.stage0_rustfmt.version, self.build, tarball_suffix,
-                )
-                self._download_component_helper(
-                    filename, "rustfmt-preview", tarball_suffix, key=self.stage0_rustfmt.date
-                )
-                self.fix_bin_or_dylib("{}/bin/rustfmt".format(bin_root))
-                self.fix_bin_or_dylib("{}/bin/cargo-fmt".format(bin_root))
-                with output(self.rustfmt_stamp()) as rustfmt_stamp:
-                    rustfmt_stamp.write(self.stage0_rustfmt.channel())
-
     def _download_component_helper(
-        self, filename, pattern, tarball_suffix, key=None
+        self, filename, pattern, tarball_suffix,
     ):
-        if key is None:
-            key = self.stage0_compiler.date
+        key = self.stage0_compiler.date
         cache_dst = os.path.join(self.build_dir, "cache")
         rustc_cache = os.path.join(cache_dst, key)
         if not os.path.exists(rustc_cache):
@@ -524,7 +501,6 @@ def _download_component_helper(
                 tarball,
                 self.checksums_sha256,
                 verbose=self.verbose,
-                do_verify=True,
             )
         unpack(tarball, tarball_suffix, self.bin_root(), match=pattern, verbose=self.verbose)
 
@@ -634,16 +610,6 @@ def rustc_stamp(self):
         """
         return os.path.join(self.bin_root(), '.rustc-stamp')
 
-    def rustfmt_stamp(self):
-        """Return the path for .rustfmt-stamp
-
-        >>> rb = RustBuild()
-        >>> rb.build_dir = "build"
-        >>> rb.rustfmt_stamp() == os.path.join("build", "stage0", ".rustfmt-stamp")
-        True
-        """
-        return os.path.join(self.bin_root(), '.rustfmt-stamp')
-
     def program_out_of_date(self, stamp_path, key):
         """Check if the given program stamp is out of date"""
         if not os.path.exists(stamp_path) or self.clean:
@@ -717,12 +683,6 @@ def rustc(self):
         """Return config path for rustc"""
         return self.program_config('rustc')
 
-    def rustfmt(self):
-        """Return config path for rustfmt"""
-        if self.stage0_rustfmt is None:
-            return None
-        return self.program_config('rustfmt')
-
     def program_config(self, program):
         """Return config path for the given program at the given stage
 
@@ -1082,8 +1042,6 @@ def bootstrap(help_triggered):
         data = json.load(f)
     build.checksums_sha256 = data["checksums_sha256"]
     build.stage0_compiler = Stage0Toolchain(data["compiler"])
-    if data.get("rustfmt") is not None:
-        build.stage0_rustfmt = Stage0Toolchain(data["rustfmt"])
 
     build.set_dist_environment(data["dist_server"])
 

src/bootstrap/builder.rs

@@ -728,7 +728,8 @@ impl<'a> Builder<'a> {
             Subcommand::Dist { ref paths } => (Kind::Dist, &paths[..]),
             Subcommand::Install { ref paths } => (Kind::Install, &paths[..]),
             Subcommand::Run { ref paths } => (Kind::Run, &paths[..]),
-            Subcommand::Format { .. } | Subcommand::Clean { .. } | Subcommand::Setup { .. } => {
+            Subcommand::Format { .. } => (Kind::Format, &[][..]),
+            Subcommand::Clean { .. } | Subcommand::Setup { .. } => {
                 panic!()
             }
         };
@@ -1192,6 +1193,10 @@ impl<'a> Builder<'a> {
         Config::download_rustc(self)
     }
 
+    pub(crate) fn initial_rustfmt(&self) -> Option<PathBuf> {
+        Config::initial_rustfmt(self)
+    }
+
     /// Prepares an invocation of `cargo` to be run.
     ///
     /// This will create a `Command` that represents a pending execution of

src/bootstrap/config.rs

@@ -3,7 +3,7 @@
 //! This module implements parsing `config.toml` configuration files to tweak
 //! how the build runs.
 
-use std::cell::Cell;
+use std::cell::{Cell, RefCell};
 use std::cmp;
 use std::collections::{HashMap, HashSet};
 use std::env;
@@ -204,10 +204,27 @@ pub struct Config {
     // These are either the stage0 downloaded binaries or the locally installed ones.
     pub initial_cargo: PathBuf,
     pub initial_rustc: PathBuf,
-    pub initial_rustfmt: Option<PathBuf>,
+    #[cfg(not(test))]
+    initial_rustfmt: RefCell<RustfmtState>,
+    #[cfg(test)]
+    pub initial_rustfmt: RefCell<RustfmtState>,
     pub out: PathBuf,
 }
 
+#[derive(Clone, Debug)]
+pub enum RustfmtState {
+    SystemToolchain(PathBuf),
+    Downloaded(PathBuf),
+    Unavailable,
+    LazyEvaluated,
+}
+
+impl Default for RustfmtState {
+    fn default() -> Self {
+        RustfmtState::LazyEvaluated
+    }
+}
+
 #[derive(Debug, Clone, Copy, PartialEq)]
 pub enum LlvmLibunwind {
     No,
@@ -1151,13 +1168,22 @@ impl Config {
             set(&mut config.missing_tools, t.missing_tools);
         }
 
-        config.initial_rustfmt = build.rustfmt.or_else(|| {
-            // Cargo does not provide a RUSTFMT environment variable, so we
-            // synthesize it manually.
-            let rustfmt = config.initial_rustc.with_file_name(exe("rustfmt", config.build));
-
-            if rustfmt.exists() { Some(rustfmt) } else { None }
-        });
+        if let Some(r) = build.rustfmt {
+            *config.initial_rustfmt.borrow_mut() = if r.exists() {
+                RustfmtState::SystemToolchain(r)
+            } else {
+                RustfmtState::Unavailable
+            };
+        } else {
+            // If using a system toolchain for bootstrapping, see if that has rustfmt available.
+            let host = config.build;
+            let rustfmt_path = config.initial_rustc.with_file_name(exe("rustfmt", host));
+            let bin_root = config.out.join(host.triple).join("stage0");
+            if !rustfmt_path.starts_with(&bin_root) {
+                // Using a system-provided toolchain; we shouldn't download rustfmt.
+                *config.initial_rustfmt.borrow_mut() = RustfmtState::SystemToolchain(rustfmt_path);
+            }
+        }
 
         // Now that we've reached the end of our configuration, infer the
         // default values for all options that we haven't otherwise stored yet.
@@ -1327,6 +1353,25 @@ impl Config {
         })
     }
 
+    pub(crate) fn initial_rustfmt(builder: &Builder<'_>) -> Option<PathBuf> {
+        match &mut *builder.config.initial_rustfmt.borrow_mut() {
+            RustfmtState::SystemToolchain(p) | RustfmtState::Downloaded(p) => Some(p.clone()),
+            RustfmtState::Unavailable => None,
+            r @ RustfmtState::LazyEvaluated => {
+                if builder.config.dry_run {
+                    return Some(PathBuf::new());
+                }
+                let path = maybe_download_rustfmt(builder);
+                *r = if let Some(p) = &path {
+                    RustfmtState::Downloaded(p.clone())
+                } else {
+                    RustfmtState::Unavailable
+                };
+                path
+            }
+        }
+    }
+
     pub fn verbose(&self) -> bool {
         self.verbose > 0
     }
@@ -1437,6 +1482,44 @@ fn download_ci_rustc_commit(download_rustc: Option<StringOrBool>, verbose: bool)
     Some(commit.to_string())
 }
 
+fn maybe_download_rustfmt(builder: &Builder<'_>) -> Option<PathBuf> {
+    #[derive(Deserialize)]
+    struct Stage0Metadata {
+        dist_server: String,
+        rustfmt: Option<RustfmtMetadata>,
+    }
+    #[derive(Deserialize)]
+    struct RustfmtMetadata {
+        date: String,
+        version: String,
+    }
+
+    let stage0_json = builder.read(&builder.src.join("src").join("stage0.json"));
+    let metadata = t!(serde_json::from_str::<Stage0Metadata>(&stage0_json));
+    let RustfmtMetadata { date, version } = metadata.rustfmt?;
+    let channel = format!("{version}-{date}");
+    let mut dist_server = env::var("RUSTUP_DIST_SERVER").unwrap_or(metadata.dist_server);
+    dist_server.push_str("/dist");
+
+    let host = builder.config.build;
+    let rustfmt_path = builder.config.initial_rustc.with_file_name(exe("rustfmt", host));
+    let bin_root = builder.config.out.join(host.triple).join("stage0");
+    let rustfmt_stamp = bin_root.join(".rustfmt-stamp");
+    if rustfmt_path.exists() && !program_out_of_date(&rustfmt_stamp, &channel) {
+        return Some(rustfmt_path);
+    }
+
+    let filename = format!("rustfmt-{version}-{build}.tar.xz", build = host.triple);
+    download_component(builder, &dist_server, filename, "rustfmt-preview", &date, "stage0");
+    assert!(rustfmt_path.exists());
+
+    builder.fix_bin_or_dylib(&bin_root.join("bin").join("rustfmt"));
+    builder.fix_bin_or_dylib(&bin_root.join("bin").join("cargo-fmt"));
+
+    builder.create(&rustfmt_stamp, &channel);
+    Some(rustfmt_path)
+}
+
 fn download_ci_rustc(builder: &Builder<'_>, commit: &str) {
     builder.verbose(&format!("using downloaded stage2 artifacts from CI (commit {commit})"));
     // FIXME: support downloading artifacts from the beta channel
@@ -1474,18 +1557,34 @@ fn download_ci_rustc(builder: &Builder<'_>, commit: &str) {
 /// Download a single component of a CI-built toolchain (not necessarily a published nightly).
 // NOTE: intentionally takes an owned string to avoid downloading multiple times by accident
 fn download_ci_component(builder: &Builder<'_>, filename: String, prefix: &str, commit: &str) {
+    download_component(
+        builder,
+        "https://ci-artifacts.rust-lang.org/rustc-builds",
+        filename,
+        prefix,
+        commit,
+        "ci-rustc",
+    )
+}
+
+fn download_component(
+    builder: &Builder<'_>,
+    base_url: &str,
+    filename: String,
+    prefix: &str,
+    key: &str,
+    destination: &str,
+) {
     let cache_dst = builder.out.join("cache");
-    let rustc_cache = cache_dst.join(commit);
-    if !rustc_cache.exists() {
-        t!(fs::create_dir_all(&rustc_cache));
+    let cache_dir = cache_dst.join(key);
+    if !cache_dir.exists() {
+        t!(fs::create_dir_all(&cache_dir));
     }
 
-    let base = "https://ci-artifacts.rust-lang.org";
-    let url = format!("rustc-builds/{commit}");
-    let tarball = rustc_cache.join(&filename);
+    let tarball = cache_dir.join(&filename);
     if !tarball.exists() {
-        builder.download_component(base, &format!("{url}/{filename}"), &tarball, "");
+        builder.download_component(base_url, &format!("{key}/{filename}"), &tarball, "");
     }
-    let bin_root = builder.out.join(builder.config.build.triple).join("ci-rustc");
+    let bin_root = builder.out.join(builder.config.build.triple).join(destination);
     builder.unpack(&tarball, &bin_root, prefix)
 }

src/bootstrap/format.rs

@@ -1,7 +1,7 @@
 //! Runs rustfmt on the repository.
 
+use crate::builder::Builder;
 use crate::util::{output, t};
-use crate::Build;
 use ignore::WalkBuilder;
 use std::collections::VecDeque;
 use std::path::{Path, PathBuf};
@@ -42,7 +42,7 @@ struct RustfmtConfig {
     ignore: Vec<String>,
 }
 
-pub fn format(build: &Build, check: bool, paths: &[PathBuf]) {
+pub fn format(build: &Builder<'_>, check: bool, paths: &[PathBuf]) {
     if build.config.dry_run {
         return;
     }
@@ -112,15 +112,11 @@ pub fn format(build: &Build, check: bool, paths: &[PathBuf]) {
     }
     let ignore_fmt = ignore_fmt.build().unwrap();
 
-    let rustfmt_path = build
-        .config
-        .initial_rustfmt
-        .as_ref()
-        .unwrap_or_else(|| {
-            eprintln!("./x.py fmt is not supported on this channel");
-            std::process::exit(1);
-        })
-        .to_path_buf();
+    let rustfmt_path = build.initial_rustfmt().unwrap_or_else(|| {
+        eprintln!("./x.py fmt is not supported on this channel");
+        std::process::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);
     let walker = match paths.get(0) {

src/bootstrap/lib.rs

@@ -661,7 +661,7 @@ impl Build {
         self.maybe_update_submodules();
 
         if let Subcommand::Format { check, paths } = &self.config.cmd {
-            return format::format(self, *check, &paths);
+            return format::format(&builder::Builder::new(&self), *check, &paths);
         }
 
         if let Subcommand::Clean { all } = self.config.cmd {