Skip to content

Make tidy not traverse untracked directories #112921

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Make tidy not traverse untracked directories #112921

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/tools/tidy/src/walk.rs
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use ignore::DirEntry;

use std::process::Command;
use std::{ffi::OsStr, fs::File, io::Read, path::Path};

/// The default directory filter.
pub fn filter_dirs(path: &Path) -> bool {
let skip = [
let skip = vec![
"tidy-test-file",
"compiler/rustc_codegen_cranelift",
"compiler/rustc_codegen_gcc",
@@ -31,6 +32,17 @@ pub fn filter_dirs(path: &Path) -> bool {
"src/bootstrap/target",
"vendor",
];
let command =
Command::new("git").args(["ls-files", ".", "--exclude-standard", "--others"]).output();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work properly if tidy is run from a subdirectory. Maybe this could be combined with git rev-parse --show-toplevel?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, ls-files will return each untracked file in a directory rather than the directory itself. I think --directory is the flag you want to prevent recursing into untracked dirs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the command that was posted in the linked issue, so I wasn't sure. I'll look into it.

if let Ok(output) = command {
let stdout: Vec<&str> =
String::from_utf8_lossy(&output.stdout).split('\n').into_iter().collect();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: no need to collect when you iterate over it below

for line in stdout {
if skip.contains(&line) {
skip.remove(skip.iter().position(|x| **x = line))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this just add each line of stdout to skip?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue (unless I'm interpreting it wrong) says that you need to remove these elements from the skip array. But, that could be wrong.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix for this is probably to run git ls-files . --exclude-standard --others and skip files that it outputs in the default filter_dirs function

The existing skip array defines directories that we wish to skip. We want to extend this with any untracked directories - all directories in skip are tracked, so right now this wouldn't do anything even without the compile errors

}
}
}
skip.iter().any(|p| path.ends_with(p))
}