Skip to content

Commit 0d2817a

Browse files
committed
Auto 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
2 parents 26286c7 + b9e4174 commit 0d2817a

File tree

13 files changed

+267
-32
lines changed

13 files changed

+267
-32
lines changed

Cargo.lock

+3-2
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ dependencies = [
192192
"cmake",
193193
"filetime",
194194
"getopts",
195+
"ignore",
195196
"lazy_static 1.3.0",
196197
"libc",
197198
"num_cpus",
@@ -1525,9 +1526,9 @@ checksum = "c3360c7b59e5ffa2653671fb74b4741a5d343c03f331c0a4aeda42b5c2b0ec7d"
15251526

15261527
[[package]]
15271528
name = "ignore"
1528-
version = "0.4.7"
1529+
version = "0.4.10"
15291530
source = "registry+https://github.com/rust-lang/crates.io-index"
1530-
checksum = "8dc57fa12805f367736a38541ac1a9fc6a52812a0ca959b1d4d4b640a89eb002"
1531+
checksum = "0ec16832258409d571aaef8273f3c3cc5b060d784e159d1a0f3b0017308f84a7"
15311532
dependencies = [
15321533
"crossbeam-channel",
15331534
"globset",

rustfmt.toml

+83
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,86 @@
44
# be picked up automatically).
55
version = "Two"
66
use_small_heuristics = "Max"
7+
8+
# by default we ignore everything in the repository
9+
# tidy only checks files which are not ignored, each entry follows gitignore style
10+
ignore = [
11+
# remove directories below, or opt out their subdirectories, as they are formatted
12+
"src/bootstrap/",
13+
"src/build_helper/",
14+
"src/liballoc/",
15+
"src/libarena/",
16+
"src/libcore/",
17+
"src/libfmt_macros/",
18+
"src/libgraphviz/",
19+
"src/libpanic_abort/",
20+
"src/libpanic_unwind/",
21+
"src/libproc_macro/",
22+
"src/libprofiler_builtins/",
23+
"src/librustc/",
24+
"src/librustc_apfloat/",
25+
"src/librustc_asan/",
26+
"src/librustc_codegen_llvm/",
27+
"src/librustc_codegen_ssa/",
28+
"src/librustc_codegen_utils/",
29+
"src/librustc_data_structures/",
30+
"src/librustc_driver/",
31+
"src/librustc_errors/",
32+
"src/librustc_feature/",
33+
"src/librustc_incremental/",
34+
"src/librustc_index/",
35+
"src/librustc_interface/",
36+
"src/librustc_lexer/",
37+
"src/librustc_lint/",
38+
"src/librustc_llvm/",
39+
"src/librustc_lsan/",
40+
"src/librustc_macros/",
41+
"src/librustc_metadata/",
42+
"src/librustc_mir/",
43+
"src/librustc_msan/",
44+
"src/librustc_parse/",
45+
"src/librustc_passes/",
46+
"src/librustc_plugin/",
47+
"src/librustc_plugin_impl/",
48+
"src/librustc_privacy/",
49+
"src/librustc_resolve/",
50+
"src/librustc_save_analysis/",
51+
"src/librustc_session/",
52+
"src/librustc_target/",
53+
"src/librustc_traits/",
54+
"src/librustc_tsan/",
55+
"src/librustc_typeck/",
56+
"src/librustdoc/",
57+
"src/libserialize/",
58+
"src/libstd/",
59+
"src/libsyntax/",
60+
"src/libsyntax_expand/",
61+
"src/libsyntax_ext/",
62+
"src/libsyntax_pos/",
63+
"src/libterm/",
64+
"src/libtest/",
65+
"src/libunwind/",
66+
"src/rtstartup/",
67+
"src/rustc/",
68+
"src/rustllvm/",
69+
"src/test/",
70+
"src/tools/",
71+
"src/etc",
72+
73+
# do not format submodules
74+
"src/doc/book",
75+
"src/doc/edition-guide",
76+
"src/doc/embedded-book",
77+
"src/doc/nomicon",
78+
"src/doc/reference",
79+
"src/doc/rust-by-example",
80+
"src/doc/rustc-guide",
81+
"src/llvm-project",
82+
"src/stdarch",
83+
"src/tools/cargo",
84+
"src/tools/clippy",
85+
"src/tools/miri",
86+
"src/tools/rls",
87+
"src/tools/rust-installer",
88+
"src/tools/rustfmt",
89+
]

src/bootstrap/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ serde_json = "1.0.2"
4747
toml = "0.5"
4848
lazy_static = "1.3.0"
4949
time = "0.1"
50+
ignore = "0.4.10"
5051

5152
[dev-dependencies]
5253
pretty_assertions = "0.5"

src/bootstrap/bootstrap.py

+42-3
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,7 @@ def __init__(self):
322322
self.date = ''
323323
self._download_url = ''
324324
self.rustc_channel = ''
325+
self.rustfmt_channel = ''
325326
self.build = ''
326327
self.build_dir = os.path.join(os.getcwd(), "build")
327328
self.clean = False
@@ -344,6 +345,7 @@ def download_stage0(self):
344345
"""
345346
rustc_channel = self.rustc_channel
346347
cargo_channel = self.cargo_channel
348+
rustfmt_channel = self.rustfmt_channel
347349

348350
def support_xz():
349351
try:
@@ -393,13 +395,29 @@ def support_xz():
393395
with output(self.cargo_stamp()) as cargo_stamp:
394396
cargo_stamp.write(self.date)
395397

396-
def _download_stage0_helper(self, filename, pattern, tarball_suffix):
398+
if self.rustfmt() and self.rustfmt().startswith(self.bin_root()) and (
399+
not os.path.exists(self.rustfmt())
400+
or self.program_out_of_date(self.rustfmt_stamp())
401+
):
402+
if rustfmt_channel:
403+
tarball_suffix = '.tar.xz' if support_xz() else '.tar.gz'
404+
[channel, date] = rustfmt_channel.split('-', 1)
405+
filename = "rustfmt-{}-{}{}".format(channel, self.build, tarball_suffix)
406+
self._download_stage0_helper(filename, "rustfmt-preview", tarball_suffix, date)
407+
self.fix_executable("{}/bin/rustfmt".format(self.bin_root()))
408+
self.fix_executable("{}/bin/cargo-fmt".format(self.bin_root()))
409+
with output(self.rustfmt_stamp()) as rustfmt_stamp:
410+
rustfmt_stamp.write(self.date)
411+
412+
def _download_stage0_helper(self, filename, pattern, tarball_suffix, date=None):
413+
if date is None:
414+
date = self.date
397415
cache_dst = os.path.join(self.build_dir, "cache")
398-
rustc_cache = os.path.join(cache_dst, self.date)
416+
rustc_cache = os.path.join(cache_dst, date)
399417
if not os.path.exists(rustc_cache):
400418
os.makedirs(rustc_cache)
401419

402-
url = "{}/dist/{}".format(self._download_url, self.date)
420+
url = "{}/dist/{}".format(self._download_url, date)
403421
tarball = os.path.join(rustc_cache, filename)
404422
if not os.path.exists(tarball):
405423
get("{}/{}".format(url, filename), tarball, verbose=self.verbose)
@@ -493,6 +511,16 @@ def cargo_stamp(self):
493511
"""
494512
return os.path.join(self.bin_root(), '.cargo-stamp')
495513

514+
def rustfmt_stamp(self):
515+
"""Return the path for .rustfmt-stamp
516+
517+
>>> rb = RustBuild()
518+
>>> rb.build_dir = "build"
519+
>>> rb.rustfmt_stamp() == os.path.join("build", "stage0", ".rustfmt-stamp")
520+
True
521+
"""
522+
return os.path.join(self.bin_root(), '.rustfmt-stamp')
523+
496524
def program_out_of_date(self, stamp_path):
497525
"""Check if the given program stamp is out of date"""
498526
if not os.path.exists(stamp_path) or self.clean:
@@ -565,6 +593,12 @@ def rustc(self):
565593
"""Return config path for rustc"""
566594
return self.program_config('rustc')
567595

596+
def rustfmt(self):
597+
"""Return config path for rustfmt"""
598+
if not self.rustfmt_channel:
599+
return None
600+
return self.program_config('rustfmt')
601+
568602
def program_config(self, program):
569603
"""Return config path for the given program
570604
@@ -868,6 +902,9 @@ def bootstrap(help_triggered):
868902
build.rustc_channel = data['rustc']
869903
build.cargo_channel = data['cargo']
870904

905+
if "rustfmt" in data:
906+
build.rustfmt_channel = data['rustfmt']
907+
871908
if 'dev' in data:
872909
build.set_dev_environment()
873910
else:
@@ -895,6 +932,8 @@ def bootstrap(help_triggered):
895932
env["RUSTC_BOOTSTRAP"] = '1'
896933
env["CARGO"] = build.cargo()
897934
env["RUSTC"] = build.rustc()
935+
if build.rustfmt():
936+
env["RUSTFMT"] = build.rustfmt()
898937
run(args, env=env, verbose=build.verbose)
899938

900939

src/bootstrap/bootstrap_test.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,14 @@ def setUp(self):
2020
os.mkdir(os.path.join(self.rust_root, "src"))
2121
with open(os.path.join(self.rust_root, "src",
2222
"stage0.txt"), "w") as stage0:
23-
stage0.write("#ignore\n\ndate: 2017-06-15\nrustc: beta\ncargo: beta")
23+
stage0.write("#ignore\n\ndate: 2017-06-15\nrustc: beta\ncargo: beta\nrustfmt: beta")
2424

2525
def tearDown(self):
2626
rmtree(self.rust_root)
2727

2828
def test_stage0_data(self):
2929
"""Extract data from stage0.txt"""
30-
expected = {"date": "2017-06-15", "rustc": "beta", "cargo": "beta"}
30+
expected = {"date": "2017-06-15", "rustc": "beta", "cargo": "beta", "rustfmt": "beta"}
3131
data = bootstrap.stage0_data(self.rust_root)
3232
self.assertDictEqual(data, expected)
3333

src/bootstrap/builder.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,7 @@ pub enum Kind {
314314
Check,
315315
Clippy,
316316
Fix,
317+
Format,
317318
Test,
318319
Bench,
319320
Dist,
@@ -353,7 +354,7 @@ impl<'a> Builder<'a> {
353354
tool::Miri,
354355
native::Lld
355356
),
356-
Kind::Check | Kind::Clippy | Kind::Fix => describe!(
357+
Kind::Check | Kind::Clippy | Kind::Fix | Kind::Format => describe!(
357358
check::Std,
358359
check::Rustc,
359360
check::Rustdoc
@@ -514,7 +515,7 @@ impl<'a> Builder<'a> {
514515
Subcommand::Bench { ref paths, .. } => (Kind::Bench, &paths[..]),
515516
Subcommand::Dist { ref paths } => (Kind::Dist, &paths[..]),
516517
Subcommand::Install { ref paths } => (Kind::Install, &paths[..]),
517-
Subcommand::Clean { .. } => panic!(),
518+
Subcommand::Format { .. } | Subcommand::Clean { .. } => panic!(),
518519
};
519520

520521
let builder = Builder {

src/bootstrap/config.rs

+9-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
66
use std::collections::{HashMap, HashSet};
77
use std::env;
8+
use std::ffi::OsString;
89
use std::fs;
910
use std::path::{Path, PathBuf};
1011
use std::process;
@@ -149,6 +150,7 @@ pub struct Config {
149150
// These are either the stage0 downloaded binaries or the locally installed ones.
150151
pub initial_cargo: PathBuf,
151152
pub initial_rustc: PathBuf,
153+
pub initial_rustfmt: Option<PathBuf>,
152154
pub out: PathBuf,
153155
}
154156

@@ -348,12 +350,16 @@ struct TomlTarget {
348350
impl Config {
349351
fn path_from_python(var_key: &str) -> PathBuf {
350352
match env::var_os(var_key) {
351-
// Do not trust paths from Python and normalize them slightly (#49785).
352-
Some(var_val) => Path::new(&var_val).components().collect(),
353+
Some(var_val) => Self::normalize_python_path(var_val),
353354
_ => panic!("expected '{}' to be set", var_key),
354355
}
355356
}
356357

358+
/// Normalizes paths from Python slightly. We don't trust paths from Python (#49785).
359+
fn normalize_python_path(path: OsString) -> PathBuf {
360+
Path::new(&path).components().collect()
361+
}
362+
357363
pub fn default_opts() -> Config {
358364
let mut config = Config::default();
359365
config.llvm_optimize = true;
@@ -380,6 +386,7 @@ impl Config {
380386

381387
config.initial_rustc = Config::path_from_python("RUSTC");
382388
config.initial_cargo = Config::path_from_python("CARGO");
389+
config.initial_rustfmt = env::var_os("RUSTFMT").map(Config::normalize_python_path);
383390

384391
config
385392
}

src/bootstrap/flags.rs

+25-1
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ pub enum Subcommand {
5353
Fix {
5454
paths: Vec<PathBuf>,
5555
},
56+
Format {
57+
check: bool,
58+
},
5659
Doc {
5760
paths: Vec<PathBuf>,
5861
},
@@ -102,6 +105,7 @@ Subcommands:
102105
check Compile either the compiler or libraries, using cargo check
103106
clippy Run clippy
104107
fix Run cargo fix
108+
fmt Run rustfmt
105109
test Build and run some test suites
106110
bench Build and run some benchmarks
107111
doc Build documentation
@@ -160,6 +164,7 @@ To learn more about a subcommand, run `./x.py <subcommand> -h`"
160164
|| (s == "check")
161165
|| (s == "clippy")
162166
|| (s == "fix")
167+
|| (s == "fmt")
163168
|| (s == "test")
164169
|| (s == "bench")
165170
|| (s == "doc")
@@ -222,6 +227,9 @@ To learn more about a subcommand, run `./x.py <subcommand> -h`"
222227
"clean" => {
223228
opts.optflag("", "all", "clean all build artifacts");
224229
}
230+
"fmt" => {
231+
opts.optflag("", "check", "check formatting instead of applying.");
232+
}
225233
_ => {}
226234
};
227235

@@ -323,6 +331,17 @@ Arguments:
323331
./x.py fix src/libcore src/libproc_macro",
324332
);
325333
}
334+
"fmt" => {
335+
subcommand_help.push_str(
336+
"\n
337+
Arguments:
338+
This subcommand optionally accepts a `--check` flag which succeeds if formatting is correct and
339+
fails if it is not. For example:
340+
341+
./x.py fmt
342+
./x.py fmt --check",
343+
);
344+
}
326345
"test" => {
327346
subcommand_help.push_str(
328347
"\n
@@ -388,7 +407,7 @@ Arguments:
388407

389408
let maybe_rules_help = Builder::get_help(&build, subcommand.as_str());
390409
extra_help.push_str(maybe_rules_help.unwrap_or_default().as_str());
391-
} else if subcommand.as_str() != "clean" {
410+
} else if !(subcommand.as_str() == "clean" || subcommand.as_str() == "fmt") {
392411
extra_help.push_str(
393412
format!(
394413
"Run `./x.py {} -h -v` to see a list of available paths.",
@@ -439,6 +458,11 @@ Arguments:
439458
all: matches.opt_present("all"),
440459
}
441460
}
461+
"fmt" => {
462+
Subcommand::Format {
463+
check: matches.opt_present("check"),
464+
}
465+
}
442466
"dist" => Subcommand::Dist { paths },
443467
"install" => Subcommand::Install { paths },
444468
_ => {

0 commit comments

Comments
 (0)