Skip to content
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.

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

Speed up tidy #62037

Merged
merged 11 commits into from
Jun 23, 2019
2 changes: 2 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3801,9 +3801,11 @@ dependencies = [
name = "tidy"
version = "0.1.0"
dependencies = [
"lazy_static 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
"regex 1.1.6 (registry+https://github.com/rust-lang/crates.io-index)",
"serde 1.0.92 (registry+https://github.com/rust-lang/crates.io-index)",
"serde_json 1.0.33 (registry+https://github.com/rust-lang/crates.io-index)",
"walkdir 2.2.7 (registry+https://github.com/rust-lang/crates.io-index)",
]

[[package]]
Expand Down
4 changes: 2 additions & 2 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -709,8 +709,8 @@ impl Step for Tidy {
if !builder.config.vendor {
cmd.arg("--no-vendor");
}
if !builder.config.verbose_tests {
cmd.arg("--quiet");
if builder.is_verbose() {
cmd.arg("--verbose");
}

let _folder = builder.fold_output(|| "tidy");
Expand Down
2 changes: 2 additions & 0 deletions src/tools/tidy/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,5 @@ edition = "2018"
regex = "1"
serde = { version = "1.0.8", features = ["derive"] }
serde_json = "1.0.2"
lazy_static = "1"
walkdir = "2"
7 changes: 4 additions & 3 deletions src/tools/tidy/src/bins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,17 @@ pub fn check(path: &Path, bad: &mut bool) {
}
}

super::walk(path,
super::walk_no_read(path,
&mut |path| super::filter_dirs(path) || path.ends_with("src/etc"),
&mut |file| {
&mut |entry| {
let file = entry.path();
let filename = file.file_name().unwrap().to_string_lossy();
let extensions = [".py", ".sh"];
if extensions.iter().any(|e| filename.ends_with(e)) {
return;
}

let metadata = t!(fs::symlink_metadata(&file), &file);
let metadata = t!(entry.metadata(), file);
if metadata.mode() & 0o111 != 0 {
let rel_path = file.strip_prefix(path).unwrap();
let git_friendly_path = rel_path.to_str().unwrap().replace("\\", "/");
Expand Down
9 changes: 2 additions & 7 deletions src/tools/tidy/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,19 @@
//! statistics about the error codes.

use std::collections::HashMap;
use std::fs::File;
use std::io::prelude::*;
use std::path::Path;

pub fn check(path: &Path, bad: &mut bool) {
let mut contents = String::new();
let mut map: HashMap<_, Vec<_>> = HashMap::new();
super::walk(path,
&mut |path| super::filter_dirs(path) || path.ends_with("src/test"),
&mut |file| {
&mut |entry, contents| {
let file = entry.path();
let filename = file.file_name().unwrap().to_string_lossy();
if filename != "error_codes.rs" {
return
}

contents.truncate(0);
t!(t!(File::open(file)).read_to_string(&mut contents));

// In the `register_long_diagnostics!` macro, entries look like this:
//
// ```
Expand Down
132 changes: 75 additions & 57 deletions src/tools/tidy/src/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,10 @@

use std::collections::HashMap;
use std::fmt;
use std::fs::{self, File};
use std::io::prelude::*;
use std::fs;
use std::path::Path;

use regex::{Regex, escape};
use regex::Regex;

mod version;
use version::Version;
Expand Down Expand Up @@ -51,20 +50,48 @@ pub struct Feature {

pub type Features = HashMap<String, Feature>;

pub fn check(path: &Path, bad: &mut bool, quiet: bool) {
pub struct CollectedFeatures {
pub lib: Features,
pub lang: Features,
}

// Currently only used for unstable book generation
pub fn collect_lib_features(base_src_path: &Path) -> Features {
let mut lib_features = Features::new();

// This library feature is defined in the `compiler_builtins` crate, which
// has been moved out-of-tree. Now it can no longer be auto-discovered by
// `tidy`, because we need to filter out its (submodule) directory. Manually
// add it to the set of known library features so we can still generate docs.
lib_features.insert("compiler_builtins_lib".to_owned(), Feature {
level: Status::Unstable,
since: None,
has_gate_test: false,
tracking_issue: None,
});

map_lib_features(base_src_path,
&mut |res, _, _| {
if let Ok((name, feature)) = res {
lib_features.insert(name.to_owned(), feature);
}
});
lib_features
}

pub fn check(path: &Path, bad: &mut bool, verbose: bool) -> CollectedFeatures {
let mut features = collect_lang_features(path, bad);
assert!(!features.is_empty());

let lib_features = get_and_check_lib_features(path, bad, &features);
assert!(!lib_features.is_empty());

let mut contents = String::new();

super::walk_many(&[&path.join("test/ui"),
&path.join("test/ui-fulldeps"),
&path.join("test/compile-fail")],
&mut |path| super::filter_dirs(path),
&mut |file| {
&mut |entry, contents| {
let file = entry.path();
let filename = file.file_name().unwrap().to_string_lossy();
if !filename.ends_with(".rs") || filename == "features.rs" ||
filename == "diagnostic_list.rs" {
Expand All @@ -74,9 +101,6 @@ pub fn check(path: &Path, bad: &mut bool, quiet: bool) {
let filen_underscore = filename.replace('-',"_").replace(".rs","");
let filename_is_gate_test = test_filen_gate(&filen_underscore, &mut features);

contents.truncate(0);
t!(t!(File::open(&file), &file).read_to_string(&mut contents));

for (i, line) in contents.lines().enumerate() {
let mut err = |msg: &str| {
tidy_error!(bad, "{}:{}: {}", file.display(), i + 1, msg);
Expand Down Expand Up @@ -130,21 +154,23 @@ pub fn check(path: &Path, bad: &mut bool, quiet: bool) {
}

if *bad {
return;
}
if quiet {
println!("* {} features", features.len());
return;
return CollectedFeatures { lib: lib_features, lang: features };
}

let mut lines = Vec::new();
lines.extend(format_features(&features, "lang"));
lines.extend(format_features(&lib_features, "lib"));
if verbose {
let mut lines = Vec::new();
lines.extend(format_features(&features, "lang"));
lines.extend(format_features(&lib_features, "lib"));

lines.sort();
for line in lines {
println!("* {}", line);
lines.sort();
for line in lines {
println!("* {}", line);
}
} else {
println!("* {} features", features.len());
}

CollectedFeatures { lib: lib_features, lang: features }
}

fn format_features<'a>(features: &'a Features, family: &'a str) -> impl Iterator<Item = String> + 'a {
Expand All @@ -159,8 +185,19 @@ fn format_features<'a>(features: &'a Features, family: &'a str) -> impl Iterator
}

fn find_attr_val<'a>(line: &'a str, attr: &str) -> Option<&'a str> {
let r = Regex::new(&format!(r#"{}\s*=\s*"([^"]*)""#, escape(attr)))
.expect("malformed regex for find_attr_val");
lazy_static::lazy_static! {
static ref ISSUE: Regex = Regex::new(r#"issue\s*=\s*"([^"]*)""#).unwrap();
static ref FEATURE: Regex = Regex::new(r#"feature\s*=\s*"([^"]*)""#).unwrap();
static ref SINCE: Regex = Regex::new(r#"since\s*=\s*"([^"]*)""#).unwrap();
}

let r = match attr {
"issue" => &*ISSUE,
"feature" => &*FEATURE,
"since" => &*SINCE,
_ => unimplemented!("{} not handled", attr),
};

r.captures(line)
.and_then(|c| c.get(1))
.map(|m| m.as_str())
Expand All @@ -175,9 +212,11 @@ fn test_find_attr_val() {
}

fn test_filen_gate(filen_underscore: &str, features: &mut Features) -> bool {
if filen_underscore.starts_with("feature_gate") {
let prefix = "feature_gate_";
if filen_underscore.starts_with(prefix) {
for (n, f) in features.iter_mut() {
if filen_underscore == format!("feature_gate_{}", n) {
// Equivalent to filen_underscore == format!("feature_gate_{}", n)
if &filen_underscore[prefix.len()..] == n {
f.has_gate_test = true;
return true;
}
Expand Down Expand Up @@ -295,32 +334,6 @@ pub fn collect_lang_features(base_src_path: &Path, bad: &mut bool) -> Features {
.collect()
}

pub fn collect_lib_features(base_src_path: &Path) -> Features {
let mut lib_features = Features::new();

// This library feature is defined in the `compiler_builtins` crate, which
// has been moved out-of-tree. Now it can no longer be auto-discovered by
// `tidy`, because we need to filter out its (submodule) directory. Manually
// add it to the set of known library features so we can still generate docs.
lib_features.insert("compiler_builtins_lib".to_owned(), Feature {
level: Status::Unstable,
since: None,
has_gate_test: false,
tracking_issue: None,
});

map_lib_features(base_src_path,
&mut |res, _, _| {
if let Ok((name, feature)) = res {
if lib_features.contains_key(name) {
return;
}
lib_features.insert(name.to_owned(), feature);
}
});
lib_features
}

fn get_and_check_lib_features(base_src_path: &Path,
bad: &mut bool,
lang_features: &Features) -> Features {
Expand Down Expand Up @@ -355,20 +368,25 @@ fn get_and_check_lib_features(base_src_path: &Path,

fn map_lib_features(base_src_path: &Path,
mf: &mut dyn FnMut(Result<(&str, Feature), &str>, &Path, usize)) {
let mut contents = String::new();
super::walk(base_src_path,
&mut |path| super::filter_dirs(path) || path.ends_with("src/test"),
&mut |file| {
&mut |entry, contents| {
let file = entry.path();
let filename = file.file_name().unwrap().to_string_lossy();
if !filename.ends_with(".rs") || filename == "features.rs" ||
filename == "diagnostic_list.rs" {
return;
}

contents.truncate(0);
t!(t!(File::open(&file), &file).read_to_string(&mut contents));
// This is an early exit -- all the attributes we're concerned with must contain this:
// * rustc_const_unstable(
// * unstable(
// * stable(
if !contents.contains("stable(") {
return;
}

let mut becoming_feature: Option<(String, Feature)> = None;
let mut becoming_feature: Option<(&str, Feature)> = None;
for (i, line) in contents.lines().enumerate() {
macro_rules! err {
($msg:expr) => {{
Expand Down Expand Up @@ -447,7 +465,7 @@ fn map_lib_features(base_src_path: &Path,
if line.contains(']') {
mf(Ok((feature_name, feature)), file, i + 1);
} else {
becoming_feature = Some((feature_name.to_owned(), feature));
becoming_feature = Some((feature_name, feature));
}
}
});
Expand Down
40 changes: 26 additions & 14 deletions src/tools/tidy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
//! This library contains the tidy lints and exposes it
//! to be used by tools.

use std::fs;
use walkdir::{DirEntry, WalkDir};
use std::fs::File;
use std::io::Read;

use std::path::Path;

Expand Down Expand Up @@ -65,25 +67,35 @@ fn filter_dirs(path: &Path) -> bool {
skip.iter().any(|p| path.ends_with(p))
}

fn walk_many(paths: &[&Path], skip: &mut dyn FnMut(&Path) -> bool, f: &mut dyn FnMut(&Path)) {

fn walk_many(
paths: &[&Path], skip: &mut dyn FnMut(&Path) -> bool, f: &mut dyn FnMut(&DirEntry, &str)
) {
for path in paths {
walk(path, skip, f);
}
}

fn walk(path: &Path, skip: &mut dyn FnMut(&Path) -> bool, f: &mut dyn FnMut(&Path)) {
if let Ok(dir) = fs::read_dir(path) {
for entry in dir {
let entry = t!(entry);
let kind = t!(entry.file_type());
let path = entry.path();
if kind.is_dir() {
if !skip(&path) {
walk(&path, skip, f);
}
} else {
f(&path);
fn walk(path: &Path, skip: &mut dyn FnMut(&Path) -> bool, f: &mut dyn FnMut(&DirEntry, &str)) {
let mut contents = String::new();
walk_no_read(path, skip, &mut |entry| {
contents.clear();
if t!(File::open(entry.path()), entry.path()).read_to_string(&mut contents).is_err() {
contents.clear();
}
f(&entry, &contents);
});
}

fn walk_no_read(path: &Path, skip: &mut dyn FnMut(&Path) -> bool, f: &mut dyn FnMut(&DirEntry)) {
let walker = WalkDir::new(path).into_iter()
.filter_entry(|e| !skip(e.path()));
for entry in walker {
if let Ok(entry) = entry {
if entry.file_type().is_dir() {
continue;
}
f(&entry);
}
}
}
25 changes: 9 additions & 16 deletions src/tools/tidy/src/libcoretest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,22 @@
//! item. All tests must be written externally in `libcore/tests`.

use std::path::Path;
use std::fs::read_to_string;

pub fn check(path: &Path, bad: &mut bool) {
let libcore_path = path.join("libcore");
super::walk(
&libcore_path,
&mut |subpath| t!(subpath.strip_prefix(&libcore_path)).starts_with("tests"),
&mut |subpath| {
&mut |entry, contents| {
let subpath = entry.path();
if let Some("rs") = subpath.extension().and_then(|e| e.to_str()) {
match read_to_string(subpath) {
Ok(contents) => {
if contents.contains("#[test]") {
tidy_error!(
bad,
"{} contains #[test]; libcore tests must be placed inside \
`src/libcore/tests/`",
subpath.display()
);
}
}
Err(err) => {
panic!("failed to read file {:?}: {}", subpath, err);
}
if contents.contains("#[test]") {
tidy_error!(
bad,
"{} contains #[test]; libcore tests must be placed inside \
`src/libcore/tests/`",
subpath.display()
);
}
}
},
Expand Down
Loading