Skip to content

Commit

Permalink
perform filesystem probe once before running bins checks concurrently
Browse files Browse the repository at this point in the history
this avoids concurrent write attempts to the output directory
  • Loading branch information
the8472 committed Apr 4, 2021
1 parent 2071040 commit 0513ba4
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 80 deletions.
172 changes: 102 additions & 70 deletions src/tools/tidy/src/bins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
}
},
)
},
)
}
}
7 changes: 0 additions & 7 deletions src/tools/tidy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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",
Expand Down
11 changes: 8 additions & 3 deletions src/tools/tidy/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 0513ba4

Please sign in to comment.