Skip to content

Commit d998000

Browse files
jyn514jyn514
committed
Ignore things in .gitignore in tidy
- Switch from `walkdir` to `ignore`. This required various changes to make `skip` thread-safe. - Ignore `build` anywhere in the source tree, not just at the top-level. We support this in bootstrap, we should support it in tidy too. As a nice side benefit, this also makes tidy a bit faster. Before: ``` ; hyperfine -i '"/home/gh-jyn514/rust2/build/aarch64-unknown-linux-gnu/stage0-tools-bin/rust-tidy" "/home/gh-jyn514/rust2" "/home/gh-jyn514/rust2/build/aarch64-unknown-linux-gnu/stage0/bin/cargo" "/home/gh-jyn514/rust2/build" "32"' Benchmark 1: "/home/gh-jyn514/rust2/build/aarch64-unknown-linux-gnu/stage0-tools-bin/rust-tidy" "/home/gh-jyn514/rust2" "/home/gh-jyn514/rust2/build/aarch64-unknown-linux-gnu/stage0/bin/cargo" "/home/gh-jyn514/rust2/build" "32" Time (mean ± σ): 1.080 s ± 0.008 s [User: 2.616 s, System: 3.243 s] Range (min … max): 1.069 s … 1.099 s 10 runs ``` After: ``` ; hyperfine '"/home/gh-jyn514/rust2/build/aarch64-unknown-linux-gnu/stage0-tools-bin/rust-tidy" "/home/gh-jyn514/rust2" "/home/gh-jyn514/rust2/build/aarch64-unknown-linux-gnu/stage0/bin/cargo" "/home/gh-jyn514/rust2/build" "32"' Benchmark 1: "/home/gh-jyn514/rust2/build/aarch64-unknown-linux-gnu/stage0-tools-bin/rust-tidy" "/home/gh-jyn514/rust2" "/home/gh-jyn514/rust2/build/aarch64-unknown-linux-gnu/stage0/bin/cargo" "/home/gh-jyn514/rust2/build" "32" Time (mean ± σ): 705.0 ms ± 1.4 ms [User: 3179.1 ms, System: 1517.5 ms] Range (min … max): 702.3 ms … 706.9 ms 10 runs ```
1 parent 312c9a3 commit d998000

14 files changed

+96
-98
lines changed

‎.gitignore

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ no_llvm_build
4141
/inst/
4242
/llvm/
4343
/mingw-build/
44-
/build/
44+
build/
4545
/build-rust-analyzer/
4646
/dist/
4747
/unicode-downloads

‎src/tools/tidy/src/alphabetical.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ fn check_section<'a>(
9595
}
9696

9797
pub fn check(path: &Path, bad: &mut bool) {
98-
walk(path, &mut filter_dirs, &mut |entry, contents| {
98+
walk(path, filter_dirs, &mut |entry, contents| {
9999
let file = &entry.path().display();
100100

101101
let mut lines = contents.lines().enumerate().peekable();

‎src/tools/tidy/src/bins.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ mod os_impl {
103103

104104
walk_no_read(
105105
path,
106-
&mut |path| {
106+
|path| {
107107
filter_dirs(path)
108108
|| path.ends_with("src/etc")
109109
// This is a list of directories that we almost certainly

‎src/tools/tidy/src/debug_artifacts.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const GRAPHVIZ_POSTFLOW_MSG: &str = "`borrowck_graphviz_postflow` attribute in t
88
pub fn check(path: &Path, bad: &mut bool) {
99
let test_dir: PathBuf = path.join("test");
1010

11-
walk(&test_dir, &mut filter_dirs, &mut |entry, contents| {
11+
walk(&test_dir, filter_dirs, &mut |entry, contents| {
1212
let filename = entry.path();
1313
let is_rust = filename.extension().map_or(false, |ext| ext == "rs");
1414
if !is_rust {

‎src/tools/tidy/src/edition.rs

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,24 +9,20 @@ fn is_edition_2021(mut line: &str) -> bool {
99
}
1010

1111
pub fn check(path: &Path, bad: &mut bool) {
12-
walk(
13-
path,
14-
&mut |path| filter_dirs(path) || path.ends_with("src/test"),
15-
&mut |entry, contents| {
16-
let file = entry.path();
17-
let filename = file.file_name().unwrap();
18-
if filename != "Cargo.toml" {
19-
return;
20-
}
12+
walk(path, |path| filter_dirs(path) || path.ends_with("src/test"), &mut |entry, contents| {
13+
let file = entry.path();
14+
let filename = file.file_name().unwrap();
15+
if filename != "Cargo.toml" {
16+
return;
17+
}
2118

22-
let is_2021 = contents.lines().any(is_edition_2021);
23-
if !is_2021 {
24-
tidy_error!(
25-
bad,
26-
"{} doesn't have `edition = \"2021\"` on a separate line",
27-
file.display()
28-
);
29-
}
30-
},
31-
);
19+
let is_2021 = contents.lines().any(is_edition_2021);
20+
if !is_2021 {
21+
tidy_error!(
22+
bad,
23+
"{} doesn't have `edition = \"2021\"` on a separate line",
24+
file.display()
25+
);
26+
}
27+
});
3228
}

‎src/tools/tidy/src/error_codes_check.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ pub fn check(paths: &[&Path], bad: &mut bool) {
213213
let regex = Regex::new(r#"[(,"\s](E\d{4})[,)"]"#).unwrap();
214214

215215
for path in paths {
216-
walk(path, &mut filter_dirs, &mut |entry, contents| {
216+
walk(path, filter_dirs, &mut |entry, contents| {
217217
let file_name = entry.file_name();
218218
let entry_path = entry.path();
219219

‎src/tools/tidy/src/errors.rs

Lines changed: 38 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -9,51 +9,47 @@ use std::path::Path;
99

1010
pub fn check(path: &Path, bad: &mut bool) {
1111
let mut map: HashMap<_, Vec<_>> = HashMap::new();
12-
walk(
13-
path,
14-
&mut |path| filter_dirs(path) || path.ends_with("src/test"),
15-
&mut |entry, contents| {
16-
let file = entry.path();
17-
let filename = file.file_name().unwrap().to_string_lossy();
18-
if filename != "error_codes.rs" {
19-
return;
20-
}
21-
22-
// In the `register_long_diagnostics!` macro, entries look like this:
23-
//
24-
// ```
25-
// EXXXX: r##"
26-
// <Long diagnostic message>
27-
// "##,
28-
// ```
29-
//
30-
// and these long messages often have error codes themselves inside
31-
// them, but we don't want to report duplicates in these cases. This
32-
// variable keeps track of whether we're currently inside one of these
33-
// long diagnostic messages.
34-
let mut inside_long_diag = false;
35-
for (num, line) in contents.lines().enumerate() {
36-
if inside_long_diag {
37-
inside_long_diag = !line.contains("\"##");
38-
continue;
39-
}
12+
walk(path, |path| filter_dirs(path) || path.ends_with("src/test"), &mut |entry, contents| {
13+
let file = entry.path();
14+
let filename = file.file_name().unwrap().to_string_lossy();
15+
if filename != "error_codes.rs" {
16+
return;
17+
}
4018

41-
let mut search = line;
42-
while let Some(i) = search.find('E') {
43-
search = &search[i + 1..];
44-
let code = if search.len() > 4 { search[..4].parse::<u32>() } else { continue };
45-
let code = match code {
46-
Ok(n) => n,
47-
Err(..) => continue,
48-
};
49-
map.entry(code).or_default().push((file.to_owned(), num + 1, line.to_owned()));
50-
break;
51-
}
19+
// In the `register_long_diagnostics!` macro, entries look like this:
20+
//
21+
// ```
22+
// EXXXX: r##"
23+
// <Long diagnostic message>
24+
// "##,
25+
// ```
26+
//
27+
// and these long messages often have error codes themselves inside
28+
// them, but we don't want to report duplicates in these cases. This
29+
// variable keeps track of whether we're currently inside one of these
30+
// long diagnostic messages.
31+
let mut inside_long_diag = false;
32+
for (num, line) in contents.lines().enumerate() {
33+
if inside_long_diag {
34+
inside_long_diag = !line.contains("\"##");
35+
continue;
36+
}
5237

53-
inside_long_diag = line.contains("r##\"");
38+
let mut search = line;
39+
while let Some(i) = search.find('E') {
40+
search = &search[i + 1..];
41+
let code = if search.len() > 4 { search[..4].parse::<u32>() } else { continue };
42+
let code = match code {
43+
Ok(n) => n,
44+
Err(..) => continue,
45+
};
46+
map.entry(code).or_default().push((file.to_owned(), num + 1, line.to_owned()));
47+
break;
5448
}
55-
},
56-
);
49+
50+
inside_long_diag = line.contains("r##\"");
51+
}
52+
});
5753

5854
let mut max = 0;
5955
for (&code, entries) in map.iter() {

‎src/tools/tidy/src/features.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ pub fn check(
100100
&src_path.join("test/rustdoc-ui"),
101101
&src_path.join("test/rustdoc"),
102102
],
103-
&mut filter_dirs,
103+
filter_dirs,
104104
&mut |entry, contents| {
105105
let file = entry.path();
106106
let filename = file.file_name().unwrap().to_string_lossy();
@@ -476,11 +476,11 @@ fn get_and_check_lib_features(
476476

477477
fn map_lib_features(
478478
base_src_path: &Path,
479-
mf: &mut dyn FnMut(Result<(&str, Feature), &str>, &Path, usize),
479+
mf: &mut (dyn Send + Sync + FnMut(Result<(&str, Feature), &str>, &Path, usize)),
480480
) {
481481
walk(
482482
base_src_path,
483-
&mut |path| filter_dirs(path) || path.ends_with("src/test"),
483+
|path| filter_dirs(path) || path.ends_with("src/test"),
484484
&mut |entry, contents| {
485485
let file = entry.path();
486486
let filename = file.file_name().unwrap().to_string_lossy();

‎src/tools/tidy/src/pal.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ pub fn check(path: &Path, bad: &mut bool) {
6868
// Sanity check that the complex parsing here works.
6969
let mut saw_target_arch = false;
7070
let mut saw_cfg_bang = false;
71-
walk(path, &mut filter_dirs, &mut |entry, contents| {
71+
walk(path, filter_dirs, &mut |entry, contents| {
7272
let file = entry.path();
7373
let filestr = file.to_string_lossy().replace("\\", "/");
7474
if !filestr.ends_with(".rs") {

‎src/tools/tidy/src/style.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ pub fn check(path: &Path, bad: &mut bool) {
226226
.chain(PROBLEMATIC_CONSTS.iter().map(|v| format!("{:X}", v)))
227227
.collect();
228228
let problematic_regex = RegexSet::new(problematic_consts_strings.as_slice()).unwrap();
229-
walk(path, &mut skip, &mut |entry, contents| {
229+
walk(path, skip, &mut |entry, contents| {
230230
let file = entry.path();
231231
let filename = file.file_name().unwrap().to_string_lossy();
232232
let extensions = [".rs", ".py", ".js", ".sh", ".c", ".cpp", ".h", ".md", ".css", ".ftl"];

‎src/tools/tidy/src/target_specific_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ pub fn check(path: &Path, bad: &mut bool) {
3838
let tests = path.join("test");
3939
crate::walk::walk(
4040
&tests,
41-
&mut |path| path.extension().map(|p| p == "rs") == Some(false),
41+
|path| path.extension().map(|p| p == "rs") == Some(false),
4242
&mut |entry, content| {
4343
let file = entry.path().display();
4444
let mut header_map = BTreeMap::new();

‎src/tools/tidy/src/ui_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ fn check_entries(path: &Path, bad: &mut bool) {
5454
pub fn check(path: &Path, bad: &mut bool) {
5555
check_entries(&path, bad);
5656
for path in &[&path.join("test/ui"), &path.join("test/ui-fulldeps")] {
57-
crate::walk::walk_no_read(path, &mut |_| false, &mut |entry| {
57+
crate::walk::walk_no_read(path, |_| false, &mut |entry| {
5858
let file_path = entry.path();
5959
if let Some(ext) = file_path.extension() {
6060
if ext == "stderr" || ext == "stdout" {

‎src/tools/tidy/src/unit_tests.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,16 @@ use crate::walk::{filter_dirs, walk};
1111
use std::path::Path;
1212

1313
pub fn check(root_path: &Path, bad: &mut bool) {
14-
let core = &root_path.join("core");
15-
let core_tests = &core.join("tests");
16-
let core_benches = &core.join("benches");
17-
let is_core = |path: &Path| {
18-
path.starts_with(core) && !(path.starts_with(core_tests) || path.starts_with(core_benches))
14+
let core = root_path.join("core");
15+
let core_copy = core.clone();
16+
let core_tests = core.join("tests");
17+
let core_benches = core.join("benches");
18+
let is_core = move |path: &Path| {
19+
path.starts_with(&core)
20+
&& !(path.starts_with(&core_tests) || path.starts_with(&core_benches))
1921
};
2022

21-
let mut skip = |path: &Path| {
23+
let skip = move |path: &Path| {
2224
let file_name = path.file_name().unwrap_or_default();
2325
if path.is_dir() {
2426
filter_dirs(path)
@@ -35,9 +37,9 @@ pub fn check(root_path: &Path, bad: &mut bool) {
3537
}
3638
};
3739

38-
walk(root_path, &mut skip, &mut |entry, contents| {
40+
walk(root_path, skip, &mut |entry, contents| {
3941
let path = entry.path();
40-
let is_core = path.starts_with(core);
42+
let is_core = path.starts_with(&core_copy);
4143
for (i, line) in contents.lines().enumerate() {
4244
let line = line.trim();
4345
let is_test = || line.contains("#[test]") && !line.contains("`#[test]");

‎src/tools/tidy/src/walk.rs

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
1-
use std::fs::File;
2-
use std::io::Read;
3-
use walkdir::{DirEntry, WalkDir};
1+
use ignore::DirEntry;
42

5-
use std::path::Path;
3+
use std::{fs, path::Path};
64

5+
/// The default directory filter.
76
pub fn filter_dirs(path: &Path) -> bool {
87
let skip = [
98
"tidy-test-file",
@@ -36,34 +35,39 @@ pub fn filter_dirs(path: &Path) -> bool {
3635

3736
pub fn walk_many(
3837
paths: &[&Path],
39-
skip: &mut dyn FnMut(&Path) -> bool,
38+
skip: impl Clone + Send + Sync + 'static + Fn(&Path) -> bool,
4039
f: &mut dyn FnMut(&DirEntry, &str),
4140
) {
4241
for path in paths {
43-
walk(path, skip, f);
42+
walk(path, skip.clone(), f);
4443
}
4544
}
4645

47-
pub fn walk(path: &Path, skip: &mut dyn FnMut(&Path) -> bool, f: &mut dyn FnMut(&DirEntry, &str)) {
48-
let mut contents = String::new();
46+
pub fn walk(
47+
path: &Path,
48+
skip: impl Send + Sync + 'static + Fn(&Path) -> bool,
49+
f: &mut dyn FnMut(&DirEntry, &str),
50+
) {
4951
walk_no_read(path, skip, &mut |entry| {
50-
contents.clear();
51-
if t!(File::open(entry.path()), entry.path()).read_to_string(&mut contents).is_err() {
52-
contents.clear();
53-
}
54-
f(&entry, &contents);
52+
let contents = t!(fs::read(entry.path()), entry.path());
53+
let contents_str = match String::from_utf8(contents) {
54+
Ok(s) => s,
55+
Err(_) => return, // skip this file
56+
};
57+
f(&entry, &contents_str);
5558
});
5659
}
5760

5861
pub(crate) fn walk_no_read(
5962
path: &Path,
60-
skip: &mut dyn FnMut(&Path) -> bool,
63+
skip: impl Send + Sync + 'static + Fn(&Path) -> bool,
6164
f: &mut dyn FnMut(&DirEntry),
6265
) {
63-
let walker = WalkDir::new(path).into_iter().filter_entry(|e| !skip(e.path()));
64-
for entry in walker {
66+
let mut walker = ignore::WalkBuilder::new(path);
67+
let walker = walker.filter_entry(move |e| !skip(e.path()));
68+
for entry in walker.build() {
6569
if let Ok(entry) = entry {
66-
if entry.file_type().is_dir() {
70+
if entry.file_type().map_or(true, |kind| kind.is_dir() || kind.is_symlink()) {
6771
continue;
6872
}
6973
f(&entry);

0 commit comments

Comments
 (0)