Skip to content

Commit

Permalink
Modify executable checking to be more universal
Browse files Browse the repository at this point in the history
This uses a dummy file to check if the filesystem being used supports the
executable bit in general.
  • Loading branch information
Mark-Simulacrum committed Sep 15, 2020
1 parent 90b1f5a commit 05c9c0e
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 12 deletions.
1 change: 1 addition & 0 deletions src/bootstrap/test.rs
Expand Up @@ -737,6 +737,7 @@ impl Step for Tidy {
let mut cmd = builder.tool_cmd(Tool::Tidy);
cmd.arg(&builder.src);
cmd.arg(&builder.initial_cargo);
cmd.arg(&builder.out);
if builder.is_verbose() {
cmd.arg("--verbose");
}
Expand Down
54 changes: 45 additions & 9 deletions src/tools/tidy/src/bins.rs
Expand Up @@ -9,19 +9,56 @@ use std::path::Path;

// All files are executable on Windows, so just check on Unix.
#[cfg(windows)]
pub fn check(_path: &Path, _bad: &mut bool) {}
pub fn check(_path: &Path, _output: &Path, _bad: &mut bool) {}

#[cfg(unix)]
pub fn check(path: &Path, bad: &mut bool) {
pub fn check(path: &Path, output: &Path, bad: &mut bool) {
use std::fs;
use std::os::unix::prelude::*;
use std::process::{Command, Stdio};

if let Ok(contents) = fs::read_to_string("/proc/version") {
// Probably on Windows Linux Subsystem or Docker via VirtualBox,
// all files will be marked as executable, so skip checking.
if contents.contains("Microsoft") || contents.contains("boot2docker") {
return;
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.
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;
}
}
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);
}
}

Expand All @@ -36,8 +73,7 @@ pub fn check(path: &Path, bad: &mut bool) {
return;
}

let metadata = t!(entry.metadata(), file);
if metadata.mode() & 0o111 != 0 {
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")
Expand Down
8 changes: 5 additions & 3 deletions src/tools/tidy/src/main.rs
Expand Up @@ -13,6 +13,8 @@ use std::process;
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 src_path = root_path.join("src");
let library_path = root_path.join("library");
Expand All @@ -36,9 +38,9 @@ fn main() {
unit_tests::check(&library_path, &mut bad);

// Checks that need to be done for both the compiler and std libraries.
bins::check(&src_path, &mut bad);
bins::check(&compiler_path, &mut bad);
bins::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);
Expand Down

0 comments on commit 05c9c0e

Please sign in to comment.