From 05c9c0ee5dcd935f518db151bee2dc88380fb92f Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Fri, 11 Sep 2020 13:10:15 -0400 Subject: [PATCH] Modify executable checking to be more universal This uses a dummy file to check if the filesystem being used supports the executable bit in general. --- src/bootstrap/test.rs | 1 + src/tools/tidy/src/bins.rs | 54 +++++++++++++++++++++++++++++++------- src/tools/tidy/src/main.rs | 8 +++--- 3 files changed, 51 insertions(+), 12 deletions(-) diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index 045dda2d4cb4c..82a4e415b8d86 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -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"); } diff --git a/src/tools/tidy/src/bins.rs b/src/tools/tidy/src/bins.rs index 589be26dc27ed..62cfa85577f98 100644 --- a/src/tools/tidy/src/bins.rs +++ b/src/tools/tidy/src/bins.rs @@ -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 { + 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); } } @@ -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") diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index 36c9e58eb9a87..e1525f8e1bf23 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -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"); @@ -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);