From 19b272a94b6f311affc86d721c34519299144681 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 5 Mar 2023 07:49:28 -0600 Subject: [PATCH] Use a single WalkBuilder for multiple paths --- src/tools/tidy/src/bins.rs | 2 +- src/tools/tidy/src/ui_tests.rs | 58 +++++++++++++++++----------------- src/tools/tidy/src/walk.rs | 29 +++++++++-------- 3 files changed, 45 insertions(+), 44 deletions(-) diff --git a/src/tools/tidy/src/bins.rs b/src/tools/tidy/src/bins.rs index 2d6abe59343..070ce93f97c 100644 --- a/src/tools/tidy/src/bins.rs +++ b/src/tools/tidy/src/bins.rs @@ -103,7 +103,7 @@ mod os_impl { // FIXME: we don't need to look at all binaries, only files that have been modified in this branch // (e.g. using `git ls-files`). - walk_no_read(path, |path| filter_dirs(path) || path.ends_with("src/etc"), &mut |entry| { + walk_no_read(&[path], |path| filter_dirs(path) || path.ends_with("src/etc"), &mut |entry| { let file = entry.path(); let extension = file.extension(); let scripts = ["py", "sh", "ps1"]; diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index ec8edf84ef3..66f5c932be2 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -49,37 +49,37 @@ fn check_entries(tests_path: &Path, bad: &mut bool) { pub fn check(path: &Path, bad: &mut bool) { check_entries(&path, bad); - for path in &[&path.join("ui"), &path.join("ui-fulldeps")] { - crate::walk::walk_no_read(path, |_| false, &mut |entry| { - let file_path = entry.path(); - if let Some(ext) = file_path.extension() { - if ext == "stderr" || ext == "stdout" { - // Test output filenames have one of the formats: - // ``` - // $testname.stderr - // $testname.$mode.stderr - // $testname.$revision.stderr - // $testname.$revision.$mode.stderr - // ``` - // - // For now, just make sure that there is a corresponding - // `$testname.rs` file. - // - // NB: We do not use file_stem() as some file names have multiple `.`s and we - // must strip all of them. - let testname = - file_path.file_name().unwrap().to_str().unwrap().split_once('.').unwrap().0; - if !file_path.with_file_name(testname).with_extension("rs").exists() { - tidy_error!(bad, "Stray file with UI testing output: {:?}", file_path); - } + let (ui, ui_fulldeps) = (path.join("ui"), path.join("ui-fulldeps")); + let paths = [ui.as_path(), ui_fulldeps.as_path()]; + crate::walk::walk_no_read(&paths, |_| false, &mut |entry| { + let file_path = entry.path(); + if let Some(ext) = file_path.extension() { + if ext == "stderr" || ext == "stdout" { + // Test output filenames have one of the formats: + // ``` + // $testname.stderr + // $testname.$mode.stderr + // $testname.$revision.stderr + // $testname.$revision.$mode.stderr + // ``` + // + // For now, just make sure that there is a corresponding + // `$testname.rs` file. + // + // NB: We do not use file_stem() as some file names have multiple `.`s and we + // must strip all of them. + let testname = + file_path.file_name().unwrap().to_str().unwrap().split_once('.').unwrap().0; + if !file_path.with_file_name(testname).with_extension("rs").exists() { + tidy_error!(bad, "Stray file with UI testing output: {:?}", file_path); + } - if let Ok(metadata) = fs::metadata(file_path) { - if metadata.len() == 0 { - tidy_error!(bad, "Empty file with UI testing output: {:?}", file_path); - } + if let Ok(metadata) = fs::metadata(file_path) { + if metadata.len() == 0 { + tidy_error!(bad, "Empty file with UI testing output: {:?}", file_path); } } } - }); - } + } + }); } diff --git a/src/tools/tidy/src/walk.rs b/src/tools/tidy/src/walk.rs index 90cd62531d0..2ade22c209f 100644 --- a/src/tools/tidy/src/walk.rs +++ b/src/tools/tidy/src/walk.rs @@ -35,26 +35,24 @@ pub fn filter_dirs(path: &Path) -> bool { /// Filter for only files that end in `.rs`. pub fn filter_not_rust(path: &Path) -> bool { - !path.is_dir() && path.extension() != Some(OsStr::new("rs")) + path.extension() != Some(OsStr::new("rs")) && !path.is_dir() +} + +pub fn walk( + path: &Path, + skip: impl Clone + Send + Sync + 'static + Fn(&Path) -> bool, + f: &mut dyn FnMut(&DirEntry, &str), +) { + walk_many(&[path], skip, f); } pub fn walk_many( paths: &[&Path], skip: impl Clone + Send + Sync + 'static + Fn(&Path) -> bool, f: &mut dyn FnMut(&DirEntry, &str), -) { - for path in paths { - walk(path, skip.clone(), f); - } -} - -pub fn walk( - path: &Path, - skip: impl Send + Sync + 'static + Fn(&Path) -> bool, - f: &mut dyn FnMut(&DirEntry, &str), ) { let mut contents = Vec::new(); - walk_no_read(path, skip, &mut |entry| { + walk_no_read(paths, skip, &mut |entry| { contents.clear(); let mut file = t!(File::open(entry.path()), entry.path()); t!(file.read_to_end(&mut contents), entry.path()); @@ -67,11 +65,14 @@ pub fn walk( } pub(crate) fn walk_no_read( - path: &Path, + paths: &[&Path], skip: impl Send + Sync + 'static + Fn(&Path) -> bool, f: &mut dyn FnMut(&DirEntry), ) { - let mut walker = ignore::WalkBuilder::new(path); + let mut walker = ignore::WalkBuilder::new(paths[0]); + for path in &paths[1..] { + walker.add(path); + } let walker = walker.filter_entry(move |e| !skip(e.path())); for entry in walker.build() { if let Ok(entry) = entry {