diff --git a/Cargo.lock b/Cargo.lock index 0bb395cdcb452..2d00f31e3159a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5308,6 +5308,7 @@ name = "tidy" version = "0.1.0" dependencies = [ "cargo_metadata 0.11.1", + "crossbeam-utils 0.8.0", "lazy_static", "regex", "walkdir", diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index 69d39f5e544c1..117201ab3cd86 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -854,6 +854,7 @@ impl Step for Tidy { cmd.arg(&builder.src); cmd.arg(&builder.initial_cargo); cmd.arg(&builder.out); + cmd.arg(builder.jobs().to_string()); if builder.is_verbose() { cmd.arg("--verbose"); } diff --git a/src/tools/tidy/Cargo.toml b/src/tools/tidy/Cargo.toml index 777d7be8fdc43..58c32993cb6ef 100644 --- a/src/tools/tidy/Cargo.toml +++ b/src/tools/tidy/Cargo.toml @@ -10,6 +10,7 @@ cargo_metadata = "0.11" regex = "1" lazy_static = "1" walkdir = "2" +crossbeam-utils = "0.8.0" [[bin]] name = "rust-tidy" diff --git a/src/tools/tidy/src/bins.rs b/src/tools/tidy/src/bins.rs index 62cfa85577f98..1d5ec5c31c67f 100644 --- a/src/tools/tidy/src/bins.rs +++ b/src/tools/tidy/src/bins.rs @@ -5,91 +5,126 @@ //! huge amount of bloat to the Git history, so it's good to just ensure we //! don't do that again. -use std::path::Path; +pub use os_impl::*; // All files are executable on Windows, so just check on Unix. #[cfg(windows)] -pub fn check(_path: &Path, _output: &Path, _bad: &mut bool) {} +mod os_impl { + use std::path::Path; + + pub fn check_filesystem_support(_sources: &[&Path], _output: &Path) -> bool { + return false; + } + + pub fn check(_path: &Path, _bad: &mut bool) {} +} #[cfg(unix)] -pub fn check(path: &Path, output: &Path, bad: &mut bool) { +mod os_impl { use std::fs; use std::os::unix::prelude::*; + use std::path::Path; use std::process::{Command, Stdio}; + enum FilesystemSupport { + Supported, + Unsupported, + ReadOnlyFs, + } + + use FilesystemSupport::*; + fn is_executable(path: &Path) -> std::io::Result { Ok(path.metadata()?.mode() & 0o111 != 0) } - // We want to avoid false positives on filesystems that do not support the - // executable bit. This occurs on some versions of Window's linux subsystem, - // for example. - // - // We try to create the temporary file first in the src directory, which is - // the preferred location as it's most likely to be on the same filesystem, - // and then in the output (`build`) directory if that fails. Sometimes we - // see the source directory mounted as read-only which means we can't - // readily create a file there to test. - // - // See #36706 and #74753 for context. - let mut temp_path = path.join("tidy-test-file"); - match fs::File::create(&temp_path).or_else(|_| { - temp_path = output.join("tidy-test-file"); - fs::File::create(&temp_path) - }) { - Ok(file) => { - let exec = is_executable(&temp_path).unwrap_or(false); - std::mem::drop(file); - std::fs::remove_file(&temp_path).expect("Deleted temp file"); - if exec { - // If the file is executable, then we assume that this - // filesystem does not track executability, so skip this check. - return; - } + pub fn check_filesystem_support(sources: &[&Path], output: &Path) -> bool { + // We want to avoid false positives on filesystems that do not support the + // executable bit. This occurs on some versions of Window's linux subsystem, + // for example. + // + // We try to create the temporary file first in the src directory, which is + // the preferred location as it's most likely to be on the same filesystem, + // and then in the output (`build`) directory if that fails. Sometimes we + // see the source directory mounted as read-only which means we can't + // readily create a file there to test. + // + // See #36706 and #74753 for context. + + fn check_dir(dir: &Path) -> FilesystemSupport { + let path = dir.join("tidy-test-file"); + match fs::File::create(&path) { + Ok(file) => { + let exec = is_executable(&path).unwrap_or(false); + std::mem::drop(file); + std::fs::remove_file(&path).expect("Deleted temp file"); + // If the file is executable, then we assume that this + // filesystem does not track executability, so skip this check. + return if exec { Unsupported } else { Supported }; + } + Err(e) => { + // If the directory is read-only or we otherwise don't have rights, + // just don't run this check. + // + // 30 is the "Read-only filesystem" code at least in one CI + // environment. + if e.raw_os_error() == Some(30) { + eprintln!("tidy: Skipping binary file check, read-only filesystem"); + return ReadOnlyFs; + } + + panic!("unable to create temporary file `{:?}`: {:?}", path, e); + } + }; } - Err(e) => { - // If the directory is read-only or we otherwise don't have rights, - // just don't run this check. - // - // 30 is the "Read-only filesystem" code at least in one CI - // environment. - if e.raw_os_error() == Some(30) { - eprintln!("tidy: Skipping binary file check, read-only filesystem"); - return; - } - panic!("unable to create temporary file `{:?}`: {:?}", temp_path, e); + for &source_dir in sources { + match check_dir(source_dir) { + Unsupported => return false, + ReadOnlyFs => { + return match check_dir(output) { + Supported => true, + _ => false, + }; + } + _ => {} + } } + + return true; } - super::walk_no_read( - path, - &mut |path| super::filter_dirs(path) || path.ends_with("src/etc"), - &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; - } + #[cfg(unix)] + pub fn check(path: &Path, bad: &mut bool) { + crate::walk_no_read( + path, + &mut |path| crate::filter_dirs(path) || path.ends_with("src/etc"), + &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; + } - if t!(is_executable(&file), file) { - let rel_path = file.strip_prefix(path).unwrap(); - let git_friendly_path = rel_path.to_str().unwrap().replace("\\", "/"); - let output = Command::new("git") - .arg("ls-files") - .arg(&git_friendly_path) - .current_dir(path) - .stderr(Stdio::null()) - .output() - .unwrap_or_else(|e| { - panic!("could not run git ls-files: {}", e); - }); - let path_bytes = rel_path.as_os_str().as_bytes(); - if output.status.success() && output.stdout.starts_with(path_bytes) { - tidy_error!(bad, "binary checked into source: {}", file.display()); + if t!(is_executable(&file), file) { + let rel_path = file.strip_prefix(path).unwrap(); + let git_friendly_path = rel_path.to_str().unwrap().replace("\\", "/"); + let output = Command::new("git") + .arg("ls-files") + .arg(&git_friendly_path) + .current_dir(path) + .stderr(Stdio::null()) + .output() + .unwrap_or_else(|e| { + panic!("could not run git ls-files: {}", e); + }); + let path_bytes = rel_path.as_os_str().as_bytes(); + if output.status.success() && output.stdout.starts_with(path_bytes) { + tidy_error!(bad, "binary checked into source: {}", file.display()); + } } - } - }, - ) + }, + ) + } } diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index 2ac96e404acb9..f190a9e57cecb 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -6,15 +6,23 @@ use tidy::*; +use crossbeam_utils::thread::{scope, ScopedJoinHandle}; +use std::collections::VecDeque; use std::env; +use std::num::NonZeroUsize; use std::path::PathBuf; use std::process; +use std::str::FromStr; +use std::sync::atomic::{AtomicBool, Ordering}; fn main() { let root_path: PathBuf = env::args_os().nth(1).expect("need path to root of repo").into(); let cargo: PathBuf = env::args_os().nth(2).expect("need path to cargo").into(); let output_directory: PathBuf = env::args_os().nth(3).expect("need path to output directory").into(); + let concurrency: NonZeroUsize = + FromStr::from_str(&env::args().nth(4).expect("need concurrency")) + .expect("concurrency must be a number"); let src_path = root_path.join("src"); let library_path = root_path.join("library"); @@ -22,45 +30,84 @@ fn main() { let args: Vec = env::args().skip(1).collect(); - let mut bad = false; let verbose = args.iter().any(|s| *s == "--verbose"); - // Checks over tests. - debug_artifacts::check(&src_path, &mut bad); - ui_tests::check(&src_path, &mut bad); - - // Checks that only make sense for the compiler. - errors::check(&compiler_path, &mut bad); - error_codes_check::check(&src_path, &mut bad); - - // Checks that only make sense for the std libs. - pal::check(&library_path, &mut bad); - - // Checks that need to be done for both the compiler and std libraries. - unit_tests::check(&src_path, &mut bad); - unit_tests::check(&compiler_path, &mut bad); - unit_tests::check(&library_path, &mut bad); - - bins::check(&src_path, &output_directory, &mut bad); - bins::check(&compiler_path, &output_directory, &mut bad); - bins::check(&library_path, &output_directory, &mut bad); - - style::check(&src_path, &mut bad); - style::check(&compiler_path, &mut bad); - style::check(&library_path, &mut bad); - - edition::check(&src_path, &mut bad); - edition::check(&compiler_path, &mut bad); - edition::check(&library_path, &mut bad); - - let collected = features::check(&src_path, &compiler_path, &library_path, &mut bad, verbose); - unstable_book::check(&src_path, collected, &mut bad); - - // Checks that are done on the cargo workspace. - deps::check(&root_path, &cargo, &mut bad); - extdeps::check(&root_path, &mut bad); - - if bad { + let bad = std::sync::Arc::new(AtomicBool::new(false)); + + scope(|s| { + let mut handles: VecDeque> = + VecDeque::with_capacity(concurrency.get()); + + macro_rules! check { + ($p:ident $(, $args:expr)* ) => { + while handles.len() >= concurrency.get() { + handles.pop_front().unwrap().join().unwrap(); + } + + let handle = s.spawn(|_| { + let mut flag = false; + $p::check($($args),* , &mut flag); + if (flag) { + bad.store(true, Ordering::Relaxed); + } + }); + handles.push_back(handle); + } + } + + // Checks that are done on the cargo workspace. + check!(deps, &root_path, &cargo); + check!(extdeps, &root_path); + + // Checks over tests. + check!(debug_artifacts, &src_path); + check!(ui_tests, &src_path); + + // Checks that only make sense for the compiler. + check!(errors, &compiler_path); + check!(error_codes_check, &src_path); + + // Checks that only make sense for the std libs. + check!(pal, &library_path); + + // Checks that need to be done for both the compiler and std libraries. + check!(unit_tests, &src_path); + check!(unit_tests, &compiler_path); + check!(unit_tests, &library_path); + + if bins::check_filesystem_support( + &[&src_path, &compiler_path, &library_path], + &output_directory, + ) { + check!(bins, &src_path); + check!(bins, &compiler_path); + check!(bins, &library_path); + } + + check!(style, &src_path); + check!(style, &compiler_path); + check!(style, &library_path); + + check!(edition, &src_path); + check!(edition, &compiler_path); + check!(edition, &library_path); + + let collected = { + while handles.len() >= concurrency.get() { + handles.pop_front().unwrap().join().unwrap(); + } + let mut flag = false; + let r = features::check(&src_path, &compiler_path, &library_path, &mut flag, verbose); + if flag { + bad.store(true, Ordering::Relaxed); + } + r + }; + check!(unstable_book, &src_path, collected); + }) + .unwrap(); + + if bad.load(Ordering::Relaxed) { eprintln!("some tidy checks failed"); process::exit(1); }