Rollup merge of #65939 - anp:incremental-rustfmt-rollout, r=Mark-Simulacrum

Enable incremental rustfmt adoption

Enables an incremental rollout of rustfmt usage within the compiler via a granular ignore configuration and automated enforcement. The decision to format the repository was approved in rust-lang/compiler-team#80 (comment).

This PR includes:

* an `[ignore]` section to `rustfmt.toml` including most of the repository
* `./x.py` downloads rustfmt from a specific nightly (we do not pin to beta or stable as we need unstable features)
* an `./x.py fmt [--check]` command which runs cargo-fmt
* `./x.py fmt --check` runs during the same test step as `src/tools/tidy`, on master, but not on beta or stable as we don't want to depend on nightly rustfmt there.
* a commit to format `src/librustc_fs_util` as an initial target and to ensure enforcement is working from the start

Author: Mark-Simulacrum

rustfmt.toml

@@ -4,3 +4,85 @@
 # be picked up automatically).
 version = "Two"
 use_small_heuristics = "Max"
+
+# by default we ignore everything in the repository
+# tidy only checks files which are not ignored, each entry follows gitignore style
+ignore = [
+    # remove directories below, or opt out their subdirectories, as they are formatted
+    "src/bootstrap/",
+    "src/build_helper/",
+    "src/liballoc/",
+    "src/libarena/",
+    "src/libcore/",
+    "src/libfmt_macros/",
+    "src/libgraphviz/",
+    "src/libpanic_abort/",
+    "src/libpanic_unwind/",
+    "src/libproc_macro/",
+    "src/libprofiler_builtins/",
+    "src/librustc/",
+    "src/librustc_apfloat/",
+    "src/librustc_asan/",
+    "src/librustc_codegen_llvm/",
+    "src/librustc_codegen_ssa/",
+    "src/librustc_codegen_utils/",
+    "src/librustc_data_structures/",
+    "src/librustc_driver/",
+    "src/librustc_errors/",
+    "src/librustc_feature/",
+    "src/librustc_incremental/",
+    "src/librustc_index/",
+    "src/librustc_interface/",
+    "src/librustc_lexer/",
+    "src/librustc_lint/",
+    "src/librustc_llvm/",
+    "src/librustc_lsan/",
+    "src/librustc_macros/",
+    "src/librustc_metadata/",
+    "src/librustc_mir/",
+    "src/librustc_msan/",
+    "src/librustc_parse/",
+    "src/librustc_passes/",
+    "src/librustc_plugin/",
+    "src/librustc_plugin_impl/",
+    "src/librustc_privacy/",
+    "src/librustc_resolve/",
+    "src/librustc_save_analysis/",
+    "src/librustc_session/",
+    "src/librustc_target/",
+    "src/librustc_traits/",
+    "src/librustc_tsan/",
+    "src/librustc_typeck/",
+    "src/librustdoc/",
+    "src/libserialize/",
+    "src/libstd/",
+    "src/libsyntax/",
+    "src/libsyntax_expand/",
+    "src/libsyntax_ext/",
+    "src/libsyntax_pos/",
+    "src/libterm/",
+    "src/libtest/",
+    "src/libunwind/",
+    "src/rtstartup/",
+    "src/rustc/",
+    "src/rustllvm/",
+    "src/test/",
+    "src/tools/",
+
+    # do not format submodules
+    "src/doc/book",
+    "src/doc/edition-guide",
+    "src/doc/embedded-book",
+    "src/doc/nomicon",
+    "src/doc/reference",
+    "src/doc/rust-by-example",
+    "src/doc/rustc-guide",
+    "src/llvm-project",
+    "src/stdarch",
+    "src/tools/cargo",
+    "src/tools/clippy",
+    "src/tools/miri",
+    "src/tools/rls",
+    "src/tools/rust-installer",
+    "src/tools/rustfmt",
+]

src/bootstrap/bootstrap.py

@@ -322,6 +322,7 @@ def __init__(self):
         self.date = ''
         self._download_url = ''
         self.rustc_channel = ''
+        self.rustfmt_channel = ''
         self.build = ''
         self.build_dir = os.path.join(os.getcwd(), "build")
         self.clean = False
@@ -344,6 +345,7 @@ def download_stage0(self):
         """
         rustc_channel = self.rustc_channel
         cargo_channel = self.cargo_channel
+        rustfmt_channel = self.rustfmt_channel
 
         def support_xz():
             try:
@@ -393,13 +395,29 @@ def support_xz():
             with output(self.cargo_stamp()) as cargo_stamp:
                 cargo_stamp.write(self.date)
 
-    def _download_stage0_helper(self, filename, pattern, tarball_suffix):
+        if self.rustfmt() and self.rustfmt().startswith(self.bin_root()) and (
+            not os.path.exists(self.rustfmt())
+            or self.program_out_of_date(self.rustfmt_stamp())
+        ):
+            if rustfmt_channel:
+                tarball_suffix = '.tar.xz' if support_xz() else '.tar.gz'
+                [channel, date] = rustfmt_channel.split('-', 1)
+                filename = "rustfmt-{}-{}{}".format(channel, self.build, tarball_suffix)
+                self._download_stage0_helper(filename, "rustfmt-preview", tarball_suffix, date)
+                self.fix_executable("{}/bin/rustfmt".format(self.bin_root()))
+                self.fix_executable("{}/bin/cargo-fmt".format(self.bin_root()))
+                with output(self.rustfmt_stamp()) as rustfmt_stamp:
+                    rustfmt_stamp.write(self.date)
+
+    def _download_stage0_helper(self, filename, pattern, tarball_suffix, date=None):
+        if date is None:
+            date = self.date
         cache_dst = os.path.join(self.build_dir, "cache")
-        rustc_cache = os.path.join(cache_dst, self.date)
+        rustc_cache = os.path.join(cache_dst, date)
         if not os.path.exists(rustc_cache):
             os.makedirs(rustc_cache)
 
-        url = "{}/dist/{}".format(self._download_url, self.date)
+        url = "{}/dist/{}".format(self._download_url, date)
         tarball = os.path.join(rustc_cache, filename)
         if not os.path.exists(tarball):
             get("{}/{}".format(url, filename), tarball, verbose=self.verbose)
@@ -493,6 +511,16 @@ def cargo_stamp(self):
         """
         return os.path.join(self.bin_root(), '.cargo-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):
         """Check if the given program stamp is out of date"""
         if not os.path.exists(stamp_path) or self.clean:
@@ -565,6 +593,12 @@ def rustc(self):
         """Return config path for rustc"""
         return self.program_config('rustc')
 
+    def rustfmt(self):
+        """Return config path for rustfmt"""
+        if not self.rustfmt_channel:
+            return None
+        return self.program_config('rustfmt')
+
     def program_config(self, program):
         """Return config path for the given program
 
@@ -868,6 +902,9 @@ def bootstrap(help_triggered):
     build.rustc_channel = data['rustc']
     build.cargo_channel = data['cargo']
 
+    if "rustfmt" in data:
+        build.rustfmt_channel = data['rustfmt']
+
     if 'dev' in data:
         build.set_dev_environment()
     else:
@@ -895,6 +932,8 @@ def bootstrap(help_triggered):
     env["RUSTC_BOOTSTRAP"] = '1'
     env["CARGO"] = build.cargo()
     env["RUSTC"] = build.rustc()
+    if build.rustfmt():
+        env["RUSTFMT"] = build.rustfmt()
     run(args, env=env, verbose=build.verbose)
 
 

src/bootstrap/bootstrap_test.py

@@ -20,14 +20,14 @@ def setUp(self):
         os.mkdir(os.path.join(self.rust_root, "src"))
         with open(os.path.join(self.rust_root, "src",
                                "stage0.txt"), "w") as stage0:
-            stage0.write("#ignore\n\ndate: 2017-06-15\nrustc: beta\ncargo: beta")
+            stage0.write("#ignore\n\ndate: 2017-06-15\nrustc: beta\ncargo: beta\nrustfmt: beta")
 
     def tearDown(self):
         rmtree(self.rust_root)
 
     def test_stage0_data(self):
         """Extract data from stage0.txt"""
-        expected = {"date": "2017-06-15", "rustc": "beta", "cargo": "beta"}
+        expected = {"date": "2017-06-15", "rustc": "beta", "cargo": "beta", "rustfmt": "beta"}
         data = bootstrap.stage0_data(self.rust_root)
         self.assertDictEqual(data, expected)
 

src/bootstrap/builder.rs

@@ -321,6 +321,7 @@ pub enum Kind {
     Check,
     Clippy,
     Fix,
+    Format,
     Test,
     Bench,
     Dist,
@@ -360,7 +361,7 @@ impl<'a> Builder<'a> {
                 tool::Miri,
                 native::Lld
             ),
-            Kind::Check | Kind::Clippy | Kind::Fix => describe!(
+            Kind::Check | Kind::Clippy | Kind::Fix | Kind::Format => describe!(
                 check::Std,
                 check::Rustc,
                 check::Rustdoc
@@ -524,7 +525,7 @@ impl<'a> Builder<'a> {
             Subcommand::Bench { ref paths, .. } => (Kind::Bench, &paths[..]),
             Subcommand::Dist { ref paths } => (Kind::Dist, &paths[..]),
             Subcommand::Install { ref paths } => (Kind::Install, &paths[..]),
-            Subcommand::Clean { .. } => panic!(),
+            Subcommand::Format { .. } | Subcommand::Clean { .. } => panic!(),
         };
 
         let builder = Builder {

src/bootstrap/config.rs

@@ -5,6 +5,7 @@
 
 use std::collections::{HashMap, HashSet};
 use std::env;
+use std::ffi::OsString;
 use std::fs;
 use std::path::{Path, PathBuf};
 use std::process;
@@ -149,6 +150,7 @@ 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>,
     pub out: PathBuf,
 }
 
@@ -348,12 +350,16 @@ struct TomlTarget {
 impl Config {
     fn path_from_python(var_key: &str) -> PathBuf {
         match env::var_os(var_key) {
-            // Do not trust paths from Python and normalize them slightly (#49785).
-            Some(var_val) => Path::new(&var_val).components().collect(),
+            Some(var_val) => Self::normalize_python_path(var_val),
             _ => panic!("expected '{}' to be set", var_key),
         }
     }
 
+    /// Normalizes paths from Python slightly. We don't trust paths from Python (#49785).
+    fn normalize_python_path(path: OsString) -> PathBuf {
+        Path::new(&path).components().collect()
+    }
+
     pub fn default_opts() -> Config {
         let mut config = Config::default();
         config.llvm_optimize = true;
@@ -380,6 +386,7 @@ impl Config {
 
         config.initial_rustc = Config::path_from_python("RUSTC");
         config.initial_cargo = Config::path_from_python("CARGO");
+        config.initial_rustfmt = env::var_os("RUSTFMT").map(Config::normalize_python_path);
 
         config
     }

src/bootstrap/flags.rs

@@ -53,6 +53,9 @@ pub enum Subcommand {
     Fix {
         paths: Vec<PathBuf>,
     },
+    Format {
+        check: bool,
+    },
     Doc {
         paths: Vec<PathBuf>,
     },
@@ -102,6 +105,7 @@ Subcommands:
     check       Compile either the compiler or libraries, using cargo check
     clippy      Run clippy
     fix         Run cargo fix
+    fmt         Run rustfmt
     test        Build and run some test suites
     bench       Build and run some benchmarks
     doc         Build documentation
@@ -160,6 +164,7 @@ To learn more about a subcommand, run `./x.py <subcommand> -h`"
                 || (s == "check")
                 || (s == "clippy")
                 || (s == "fix")
+                || (s == "fmt")
                 || (s == "test")
                 || (s == "bench")
                 || (s == "doc")
@@ -222,6 +227,9 @@ To learn more about a subcommand, run `./x.py <subcommand> -h`"
             "clean" => {
                 opts.optflag("", "all", "clean all build artifacts");
             }
+            "fmt" => {
+                opts.optflag("", "check", "check formatting instead of applying.");
+            }
             _ => {}
         };
 
@@ -323,6 +331,17 @@ Arguments:
         ./x.py fix src/libcore src/libproc_macro",
                 );
             }
+            "fmt" => {
+                subcommand_help.push_str(
+                    "\n
+Arguments:
+    This subcommand optionally accepts a `--check` flag which succeeds if formatting is correct and
+    fails if it is not. For example:
+
+        ./x.py fmt
+        ./x.py fmt --check",
+                );
+            }
             "test" => {
                 subcommand_help.push_str(
                     "\n
@@ -388,7 +407,7 @@ Arguments:
 
             let maybe_rules_help = Builder::get_help(&build, subcommand.as_str());
             extra_help.push_str(maybe_rules_help.unwrap_or_default().as_str());
-        } else if subcommand.as_str() != "clean" {
+        } else if !(subcommand.as_str() == "clean" || subcommand.as_str() == "fmt") {
             extra_help.push_str(
                 format!(
                     "Run `./x.py {} -h -v` to see a list of available paths.",
@@ -439,6 +458,11 @@ Arguments:
                     all: matches.opt_present("all"),
                 }
             }
+            "fmt" => {
+                Subcommand::Format {
+                    check: matches.opt_present("check"),
+                }
+            }
             "dist" => Subcommand::Dist { paths },
             "install" => Subcommand::Install { paths },
             _ => {

src/bootstrap/format.rs

@@ -0,0 +1,28 @@
+//! Runs rustfmt on the repository.
+
+use crate::{util, Build};
+use std::process::Command;
+
+pub fn format(build: &Build, check: bool) {
+    let target = &build.build;
+    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);
+    }).clone();
+    let cargo_fmt_path = rustfmt_path.with_file_name(util::exe("cargo-fmt", &target));
+    assert!(cargo_fmt_path.is_file(), "{} not a file", cargo_fmt_path.display());
+
+    let mut cmd = Command::new(&cargo_fmt_path);
+    // cargo-fmt calls rustfmt as a bare command, so we need it to only find the correct one
+    cmd.env("PATH", cargo_fmt_path.parent().unwrap());
+    cmd.current_dir(&build.src);
+    cmd.arg("fmt");
+
+    if check {
+        cmd.arg("--");
+        cmd.arg("--check");
+    }
+
+    let status = cmd.status().expect("executing cargo-fmt");
+    assert!(status.success(), "cargo-fmt errored with status {:?}", status);
+}

src/bootstrap/lib.rs

@@ -147,6 +147,7 @@ mod builder;
 mod cache;
 mod tool;
 mod toolstate;
+mod format;
 
 #[cfg(windows)]
 mod job;
@@ -421,6 +422,10 @@ impl Build {
             job::setup(self);
         }
 
+        if let Subcommand::Format { check } = self.config.cmd {
+            return format::format(self, check);
+        }
+
         if let Subcommand::Clean { all } = self.config.cmd {
             return clean::clean(self, all);
         }