Skip to content

Commit

Permalink
Auto merge of #125736 - Oneirical:run-make-file-management, r=jieyouxu
Browse files Browse the repository at this point in the history
run-make-support: add wrapper for `fs` operations

Suggested by #125728.

The point of this wrapper is to stop silent fails caused by forgetting to `unwrap` `fs` functions. However, functions like `fs::read` which return something and get stored in a variable should cause a failure on their own if they are not unwrapped (as the `Result` will be stored in the variable, and something will be done on that `Result` that should have been done to its contents). Is it still pertinent to wrap `fs::read_to_string`, `fs::metadata` and so on?

Closes: #125728

try-job: x86_64-msvc
try-job: i686-mingw
  • Loading branch information
bors committed Jun 11, 2024
2 parents 0c96061 + c84afee commit 3ea5e23
Show file tree
Hide file tree
Showing 36 changed files with 211 additions and 108 deletions.
113 changes: 113 additions & 0 deletions src/tools/run-make-support/src/fs_wrapper.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
use std::fs;
use std::path::Path;

/// A wrapper around [`std::fs::remove_file`] which includes the file path in the panic message..
#[track_caller]
pub fn remove_file<P: AsRef<Path>>(path: P) {
fs::remove_file(path.as_ref())
.expect(&format!("the file in path \"{}\" could not be removed", path.as_ref().display()));
}

/// A wrapper around [`std::fs::copy`] which includes the file path in the panic message.
#[track_caller]
pub fn copy<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) {
fs::copy(from.as_ref(), to.as_ref()).expect(&format!(
"the file \"{}\" could not be copied over to \"{}\"",
from.as_ref().display(),
to.as_ref().display(),
));
}

/// A wrapper around [`std::fs::File::create`] which includes the file path in the panic message..
#[track_caller]
pub fn create_file<P: AsRef<Path>>(path: P) {
fs::File::create(path.as_ref())
.expect(&format!("the file in path \"{}\" could not be created", path.as_ref().display()));
}

/// A wrapper around [`std::fs::read`] which includes the file path in the panic message..
#[track_caller]
pub fn read<P: AsRef<Path>>(path: P) -> Vec<u8> {
fs::read(path.as_ref())
.expect(&format!("the file in path \"{}\" could not be read", path.as_ref().display()))
}

/// A wrapper around [`std::fs::read_to_string`] which includes the file path in the panic message..
#[track_caller]
pub fn read_to_string<P: AsRef<Path>>(path: P) -> String {
fs::read_to_string(path.as_ref()).expect(&format!(
"the file in path \"{}\" could not be read into a String",
path.as_ref().display()
))
}

/// A wrapper around [`std::fs::read_dir`] which includes the file path in the panic message..
#[track_caller]
pub fn read_dir<P: AsRef<Path>>(path: P) -> fs::ReadDir {
fs::read_dir(path.as_ref())
.expect(&format!("the directory in path \"{}\" could not be read", path.as_ref().display()))
}

/// A wrapper around [`std::fs::write`] which includes the file path in the panic message..
#[track_caller]
pub fn write<P: AsRef<Path>, C: AsRef<[u8]>>(path: P, contents: C) {
fs::write(path.as_ref(), contents.as_ref()).expect(&format!(
"the file in path \"{}\" could not be written to",
path.as_ref().display()
));
}

/// A wrapper around [`std::fs::remove_dir_all`] which includes the file path in the panic message..
#[track_caller]
pub fn remove_dir_all<P: AsRef<Path>>(path: P) {
fs::remove_dir_all(path.as_ref()).expect(&format!(
"the directory in path \"{}\" could not be removed alongside all its contents",
path.as_ref().display(),
));
}

/// A wrapper around [`std::fs::create_dir`] which includes the file path in the panic message..
#[track_caller]
pub fn create_dir<P: AsRef<Path>>(path: P) {
fs::create_dir(path.as_ref()).expect(&format!(
"the directory in path \"{}\" could not be created",
path.as_ref().display()
));
}

/// A wrapper around [`std::fs::create_dir_all`] which includes the file path in the panic message..
#[track_caller]
pub fn create_dir_all<P: AsRef<Path>>(path: P) {
fs::create_dir_all(path.as_ref()).expect(&format!(
"the directory (and all its parents) in path \"{}\" could not be created",
path.as_ref().display()
));
}

/// A wrapper around [`std::fs::metadata`] which includes the file path in the panic message..
#[track_caller]
pub fn metadata<P: AsRef<Path>>(path: P) -> fs::Metadata {
fs::metadata(path.as_ref()).expect(&format!(
"the file's metadata in path \"{}\" could not be read",
path.as_ref().display()
))
}

/// A wrapper around [`std::fs::rename`] which includes the file path in the panic message.
#[track_caller]
pub fn rename<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) {
fs::rename(from.as_ref(), to.as_ref()).expect(&format!(
"the file \"{}\" could not be moved over to \"{}\"",
from.as_ref().display(),
to.as_ref().display(),
));
}

/// A wrapper around [`std::fs::set_permissions`] which includes the file path in the panic message.
#[track_caller]
pub fn set_permissions<P: AsRef<Path>>(path: P, perm: fs::Permissions) {
fs::set_permissions(path.as_ref(), perm).expect(&format!(
"the file's permissions in path \"{}\" could not be changed",
path.as_ref().display()
));
}
22 changes: 8 additions & 14 deletions src/tools/run-make-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ pub mod clang;
mod command;
pub mod diff;
mod drop_bomb;
pub mod fs_wrapper;
pub mod llvm_readobj;
pub mod run;
pub mod rustc;
Expand Down Expand Up @@ -153,7 +154,7 @@ pub fn dynamic_lib_extension() -> &'static str {
}
}

/// Construct a rust library (rlib) name.
/// Generate the name a rust library (rlib) would have.
pub fn rust_lib_name(name: &str) -> String {
format!("lib{name}.rlib")
}
Expand Down Expand Up @@ -228,15 +229,15 @@ pub fn copy_dir_all(src: impl AsRef<Path>, dst: impl AsRef<Path>) {
fn copy_dir_all_inner(src: impl AsRef<Path>, dst: impl AsRef<Path>) -> io::Result<()> {
let dst = dst.as_ref();
if !dst.is_dir() {
fs::create_dir_all(&dst)?;
std::fs::create_dir_all(&dst)?;
}
for entry in fs::read_dir(src)? {
for entry in std::fs::read_dir(src)? {
let entry = entry?;
let ty = entry.file_type()?;
if ty.is_dir() {
copy_dir_all_inner(entry.path(), dst.join(entry.file_name()))?;
} else {
fs::copy(entry.path(), dst.join(entry.file_name()))?;
std::fs::copy(entry.path(), dst.join(entry.file_name()))?;
}
}
Ok(())
Expand All @@ -255,22 +256,15 @@ pub fn copy_dir_all(src: impl AsRef<Path>, dst: impl AsRef<Path>) {

/// Check that all files in `dir1` exist and have the same content in `dir2`. Panic otherwise.
pub fn recursive_diff(dir1: impl AsRef<Path>, dir2: impl AsRef<Path>) {
fn read_file(path: &Path) -> Vec<u8> {
match fs::read(path) {
Ok(c) => c,
Err(e) => panic!("Failed to read `{}`: {:?}", path.display(), e),
}
}

let dir2 = dir2.as_ref();
read_dir(dir1, |entry_path| {
let entry_name = entry_path.file_name().unwrap();
if entry_path.is_dir() {
recursive_diff(&entry_path, &dir2.join(entry_name));
} else {
let path2 = dir2.join(entry_name);
let file1 = read_file(&entry_path);
let file2 = read_file(&path2);
let file1 = fs_wrapper::read(&entry_path);
let file2 = fs_wrapper::read(&path2);

// We don't use `assert_eq!` because they are `Vec<u8>`, so not great for display.
// Why not using String? Because there might be minified files or even potentially
Expand All @@ -286,7 +280,7 @@ pub fn recursive_diff(dir1: impl AsRef<Path>, dir2: impl AsRef<Path>) {
}

pub fn read_dir<F: Fn(&Path)>(dir: impl AsRef<Path>, callback: F) {
for entry in fs::read_dir(dir).unwrap() {
for entry in fs_wrapper::read_dir(dir) {
callback(&entry.unwrap().path());
}
}
Expand Down
4 changes: 2 additions & 2 deletions tests/run-make/CURRENT_RUSTC_VERSION/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@

use std::path::PathBuf;

use run_make_support::{aux_build, rustc, source_root};
use run_make_support::{aux_build, fs_wrapper, rustc, source_root};

fn main() {
aux_build().input("stable.rs").emit("metadata").run();

let output =
rustc().input("main.rs").emit("metadata").extern_("stable", "libstable.rmeta").run();

let version = std::fs::read_to_string(source_root().join("src/version")).unwrap();
let version = fs_wrapper::read_to_string(source_root().join("src/version"));
let expected_string = format!("stable since {}", version.trim());
output.assert_stderr_contains(expected_string);
}
8 changes: 4 additions & 4 deletions tests/run-make/c-link-to-rust-dylib/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@

//@ ignore-cross-compile

use std::fs::remove_file;

use run_make_support::{cc, cwd, dynamic_lib_extension, is_msvc, read_dir, run, run_fail, rustc};
use run_make_support::{
cc, cwd, dynamic_lib_extension, fs_wrapper, is_msvc, read_dir, run, run_fail, rustc,
};

fn main() {
rustc().input("foo.rs").run();
Expand All @@ -28,7 +28,7 @@ fn main() {
name.ends_with(".so") || name.ends_with(".dll") || name.ends_with(".dylib")
})
{
remove_file(path).unwrap();
fs_wrapper::remove_file(path);
}
});
run_fail("bar");
Expand Down
3 changes: 2 additions & 1 deletion tests/run-make/c-link-to-rust-staticlib/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@

//@ ignore-cross-compile

use run_make_support::fs_wrapper::remove_file;
use run_make_support::{cc, extra_c_flags, run, rustc, static_lib_name};
use std::fs;

fn main() {
rustc().input("foo.rs").run();
cc().input("bar.c").input(static_lib_name("foo")).out_exe("bar").args(&extra_c_flags()).run();
run("bar");
fs::remove_file(static_lib_name("foo"));
remove_file(static_lib_name("foo"));
run("bar");
}
6 changes: 2 additions & 4 deletions tests/run-make/cdylib/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@

//@ ignore-cross-compile

use std::fs::remove_file;

use run_make_support::{cc, cwd, dynamic_lib_name, is_msvc, run, rustc};
use run_make_support::{cc, cwd, dynamic_lib_name, fs_wrapper, is_msvc, run, rustc};

fn main() {
rustc().input("bar.rs").run();
Expand All @@ -25,7 +23,7 @@ fn main() {
}

run("foo");
remove_file(dynamic_lib_name("foo")).unwrap();
fs_wrapper::remove_file(dynamic_lib_name("foo"));

rustc().input("foo.rs").arg("-Clto").run();
run("foo");
Expand Down
6 changes: 3 additions & 3 deletions tests/run-make/compiler-builtins/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

#![deny(warnings)]

use run_make_support::fs_wrapper::{read, read_dir};
use run_make_support::object::read::archive::ArchiveFile;
use run_make_support::object::read::Object;
use run_make_support::object::ObjectSection;
Expand Down Expand Up @@ -55,8 +56,7 @@ fn main() {
cmd.run();

let rlibs_path = target_dir.join(target).join("debug").join("deps");
let compiler_builtins_rlib = std::fs::read_dir(rlibs_path)
.unwrap()
let compiler_builtins_rlib = read_dir(rlibs_path)
.find_map(|e| {
let path = e.unwrap().path();
let file_name = path.file_name().unwrap().to_str().unwrap();
Expand All @@ -70,7 +70,7 @@ fn main() {

// rlib files are archives, where the archive members each a CGU, and we also have one called
// lib.rmeta which is the encoded metadata. Each of the CGUs is an object file.
let data = std::fs::read(compiler_builtins_rlib).unwrap();
let data = read(compiler_builtins_rlib);

let mut defined_symbols = HashSet::new();
let mut undefined_relocations = HashSet::new();
Expand Down
6 changes: 2 additions & 4 deletions tests/run-make/const-prop-lint/rmake.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
// Tests that const prop lints interrupting codegen don't leave `.o` files around.

use std::fs;

use run_make_support::{cwd, rustc};
use run_make_support::{cwd, fs_wrapper, rustc};

fn main() {
rustc().input("input.rs").run_fail().assert_exit_code(1);

for entry in fs::read_dir(cwd()).unwrap() {
for entry in fs_wrapper::read_dir(cwd()) {
let entry = entry.unwrap();
let path = entry.path();

Expand Down
8 changes: 4 additions & 4 deletions tests/run-make/doctests-keep-binaries/rmake.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
// Check that valid binaries are persisted by running them, regardless of whether the
// --run or --no-run option is used.

use run_make_support::fs_wrapper::{create_dir, remove_dir_all};
use run_make_support::{run, rustc, rustdoc};
use std::fs::{create_dir, remove_dir_all};
use std::path::Path;

fn setup_test_env<F: FnOnce(&Path, &Path)>(callback: F) {
let out_dir = Path::new("doctests");
create_dir(&out_dir).expect("failed to create doctests folder");
create_dir(&out_dir);
rustc().input("t.rs").crate_type("rlib").run();
callback(&out_dir, Path::new("libt.rlib"));
remove_dir_all(out_dir);
Expand Down Expand Up @@ -43,9 +43,9 @@ fn main() {
check_generated_binaries();
});
// Behavior with --test-run-directory with relative paths.
setup_test_env(|_out_dir, extern_path| {
setup_test_env(|_out_dir, _extern_path| {
let run_dir_path = Path::new("rundir");
create_dir(&run_dir_path).expect("failed to create rundir folder");
create_dir(&run_dir_path);

rustdoc()
.input("t.rs")
Expand Down
4 changes: 2 additions & 2 deletions tests/run-make/doctests-runtool/rmake.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
// Tests behavior of rustdoc `--runtool`.

use run_make_support::fs_wrapper::{create_dir, remove_dir_all};
use run_make_support::{rustc, rustdoc};
use std::fs::{create_dir, remove_dir_all};
use std::path::PathBuf;

fn mkdir(name: &str) -> PathBuf {
let dir = PathBuf::from(name);
create_dir(&dir).expect("failed to create doctests folder");
create_dir(&dir);
dir
}

Expand Down
5 changes: 2 additions & 3 deletions tests/run-make/emit-named-files/rmake.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::fs::create_dir;
use std::path::Path;

use run_make_support::rustc;
use run_make_support::{fs_wrapper, rustc};

fn emit_and_check(out_dir: &Path, out_file: &str, format: &str) {
let out_file = out_dir.join(out_file);
Expand All @@ -12,7 +11,7 @@ fn emit_and_check(out_dir: &Path, out_file: &str, format: &str) {
fn main() {
let out_dir = Path::new("emit");

create_dir(&out_dir).unwrap();
fs_wrapper::create_dir(&out_dir);

emit_and_check(&out_dir, "libfoo.s", "asm");
emit_and_check(&out_dir, "libfoo.bc", "llvm-bc");
Expand Down
11 changes: 5 additions & 6 deletions tests/run-make/incr-prev-body-beyond-eof/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,14 @@
//@ ignore-nvptx64-nvidia-cuda
// FIXME: can't find crate for `std`

use run_make_support::fs_wrapper as fs;
use run_make_support::rustc;
use std::fs;

fn main() {
// FIXME(Oneirical): Use run_make_support::fs_wrapper here.
fs::create_dir("src").unwrap();
fs::create_dir("incr").unwrap();
fs::copy("a.rs", "src/main.rs").unwrap();
fs::create_dir("src");
fs::create_dir("incr");
fs::copy("a.rs", "src/main.rs");
rustc().incremental("incr").input("src/main.rs").run();
fs::copy("b.rs", "src/main.rs").unwrap();
fs::copy("b.rs", "src/main.rs");
rustc().incremental("incr").input("src/main.rs").run();
}
Loading

0 comments on commit 3ea5e23

Please sign in to comment.