diff options
| -rw-r--r-- | src/tools/tidy/src/bins.rs | 172 | ||||
| -rw-r--r-- | src/tools/tidy/src/lib.rs | 7 | ||||
| -rw-r--r-- | src/tools/tidy/src/main.rs | 11 |
3 files changed, 110 insertions, 80 deletions
diff --git a/src/tools/tidy/src/bins.rs b/src/tools/tidy/src/bins.rs index f4e203cac40..1d5ec5c31c6 100644 --- a/src/tools/tidy/src/bins.rs +++ b/src/tools/tidy/src/bins.rs @@ -5,94 +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<bool> { 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. - // - // We also add the thread ID to avoid threads trampling on each others files. - let file_name = format!("t{}.tidy-test-file", std::thread::current().id().as_u64()); - let mut temp_path = path.join(&file_name); - match fs::File::create(&temp_path).or_else(|_| { - temp_path = output.join(&file_name); - 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/lib.rs b/src/tools/tidy/src/lib.rs index 45cc169470b..11d36751f67 100644 --- a/src/tools/tidy/src/lib.rs +++ b/src/tools/tidy/src/lib.rs @@ -4,7 +4,6 @@ //! to be used by tools. #![cfg_attr(bootstrap, feature(str_split_once))] -#![feature(thread_id_value)] use std::fs::File; use std::io::Read; @@ -55,12 +54,6 @@ pub mod unit_tests; pub mod unstable_book; fn filter_dirs(path: &Path) -> bool { - // Filter out temporary files used by the bins module to probe the filesystem - match path.extension() { - Some(ext) if ext == "tidy-test-file" => return true, - _ => {} - } - let skip = [ "compiler/rustc_codegen_cranelift", "src/llvm-project", diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index 2d19db38799..f190a9e57ce 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -75,9 +75,14 @@ fn main() { check!(unit_tests, &compiler_path); check!(unit_tests, &library_path); - check!(bins, &src_path, &output_directory); - check!(bins, &compiler_path, &output_directory); - check!(bins, &library_path, &output_directory); + 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); |