From fe295fa97ae063411ef3165b72e0aba1180ba5a0 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Tue, 21 Feb 2023 08:37:42 +0100 Subject: [PATCH] use fs_err for sake of better io errors (#1616) * use fs_err Provides path names of failing io operations where possible by default, both in the resulting binaries which ease debugging of i.e. selinux or seccomp filters, without resorting to strace, but also local unit test file operations. --- Cargo.lock | 7 +++++++ Cargo.toml | 1 + src/bin/sccache-dist/build.rs | 2 +- src/cache/cache.rs | 2 +- src/commands.rs | 3 ++- src/compiler/c.rs | 2 +- src/compiler/clang.rs | 3 ++- src/compiler/compiler.rs | 8 ++++---- src/compiler/diab.rs | 8 +++++--- src/compiler/gcc.rs | 5 +++-- src/compiler/msvc.rs | 3 ++- src/compiler/nvcc.rs | 3 ++- src/compiler/rust.rs | 6 +++--- src/config.rs | 3 ++- src/dist/cache.rs | 8 ++++---- src/dist/http.rs | 2 +- src/dist/pkg.rs | 8 ++++---- src/lru_disk_cache/mod.rs | 5 +++-- src/server.rs | 5 +++-- src/test/tests.rs | 3 ++- src/test/utils.rs | 7 ++++--- src/util.rs | 3 ++- tests/harness/mod.rs | 2 +- tests/oauth.rs | 2 +- tests/sccache_cargo.rs | 2 +- tests/system.rs | 3 ++- 26 files changed, 64 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d1e932aa..dc74020e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -666,6 +666,12 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fs-err" +version = "2.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0845fa252299212f0389d64ba26f34fa32cfe41588355f21ed507c59a0f64541" + [[package]] name = "futures" version = "0.3.26" @@ -2039,6 +2045,7 @@ dependencies = [ "env_logger", "filetime", "flate2", + "fs-err", "futures", "gzp", "http", diff --git a/Cargo.toml b/Cargo.toml index 4e68540b..ab260695 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,6 +39,7 @@ directories = "4.0.1" env_logger = "0.10" filetime = "0.2" flate2 = { version = "1.0", optional = true, default-features = false, features = ["rust_backend"] } +fs-err = "2.9" futures = "0.3" gzp = { version = "0.11.1", default-features = false, features = ["deflate_rust"] } http = "0.2" diff --git a/src/bin/sccache-dist/build.rs b/src/bin/sccache-dist/build.rs index b0ab4ede..f93bcb54 100644 --- a/src/bin/sccache-dist/build.rs +++ b/src/bin/sccache-dist/build.rs @@ -14,6 +14,7 @@ use anyhow::{anyhow, bail, Context, Error, Result}; use flate2::read::GzDecoder; +use fs_err as fs; use libmount::Overlay; use sccache::dist::{ BuildResult, BuilderIncoming, CompileCommand, InputsReader, OutputData, ProcessOutput, TcCache, @@ -21,7 +22,6 @@ use sccache::dist::{ }; use sccache::lru_disk_cache::Error as LruError; use std::collections::{hash_map, HashMap}; -use std::fs; use std::io; use std::iter; use std::path::{self, Path, PathBuf}; diff --git a/src/cache/cache.rs b/src/cache/cache.rs index 1d7be539..6850e2cc 100644 --- a/src/cache/cache.rs +++ b/src/cache/cache.rs @@ -38,8 +38,8 @@ use crate::config::Config; feature = "webdav" ))] use crate::config::{self, CacheType}; +use fs_err as fs; use std::fmt; -use std::fs; use std::io::{self, Cursor, Read, Seek, Write}; use std::path::{Path, PathBuf}; use std::sync::Arc; diff --git a/src/commands.rs b/src/commands.rs index 86b21bd4..492163b7 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -23,10 +23,11 @@ use crate::server::{self, DistInfo, ServerInfo, ServerStartup}; use crate::util::daemonize; use atty::Stream; use byteorder::{BigEndian, ByteOrder}; +use fs::{File, OpenOptions}; +use fs_err as fs; use log::Level::Trace; use std::env; use std::ffi::{OsStr, OsString}; -use std::fs::{File, OpenOptions}; use std::io::{self, Write}; #[cfg(unix)] use std::os::unix::process::ExitStatusExt; diff --git a/src/compiler/c.rs b/src/compiler/c.rs index 323381f5..f07a1c6a 100644 --- a/src/compiler/c.rs +++ b/src/compiler/c.rs @@ -24,11 +24,11 @@ use crate::dist; use crate::dist::pkg; use crate::mock_command::CommandCreatorSync; use crate::util::{hash_all, Digest, HashToDigest}; +use fs_err as fs; use std::borrow::Cow; use std::collections::{HashMap, HashSet}; use std::ffi::{OsStr, OsString}; use std::fmt; -use std::fs; use std::hash::Hash; #[cfg(feature = "dist-client")] use std::io; diff --git a/src/compiler/clang.rs b/src/compiler/clang.rs index b6cee51e..4319e8d4 100644 --- a/src/compiler/clang.rs +++ b/src/compiler/clang.rs @@ -23,9 +23,10 @@ use crate::compiler::{gcc, write_temp_file, Cacheable, CompileCommand, CompilerA use crate::mock_command::{CommandCreator, CommandCreatorSync, RunCommand}; use crate::util::{run_input_output, OsStrExt}; use crate::{counted_array, dist}; +use fs::File; +use fs_err as fs; use semver::{BuildMetadata, Prerelease, Version}; use std::ffi::OsString; -use std::fs::File; use std::future::Future; use std::io::{self, Write}; use std::path::{Path, PathBuf}; diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index eacff57f..c515d331 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -30,12 +30,12 @@ use crate::lru_disk_cache; use crate::mock_command::{exit_status, CommandChild, CommandCreatorSync, RunCommand}; use crate::util::{fmt_duration_as_secs, ref_env, run_input_output}; use filetime::FileTime; +use fs::File; +#[cfg(feature = "dist-client")] +use fs_err as fs; use std::borrow::Cow; use std::ffi::{OsStr, OsString}; use std::fmt; -#[cfg(feature = "dist-client")] -use std::fs; -use std::fs::File; use std::future::Future; use std::io::prelude::*; use std::path::{Path, PathBuf}; @@ -1231,7 +1231,7 @@ mod test { use crate::mock_command::*; use crate::test::mock_storage::MockStorage; use crate::test::utils::*; - use std::fs::{self, File}; + use fs::File; use std::io::Write; use std::sync::Arc; use std::time::Duration; diff --git a/src/compiler/diab.rs b/src/compiler/diab.rs index a5e6a79e..c9fed77a 100644 --- a/src/compiler/diab.rs +++ b/src/compiler/diab.rs @@ -25,10 +25,11 @@ use crate::errors::*; use crate::mock_command::{CommandCreatorSync, RunCommand}; use crate::util::{run_input_output, OsStrExt}; use crate::{counted_array, dist}; +use fs::File; +use fs_err as fs; use log::Level::Trace; use std::collections::HashMap; use std::ffi::OsString; -use std::fs::File; use std::io::Read; use std::path::{Path, PathBuf}; use std::process; @@ -435,13 +436,14 @@ impl<'a> Iterator for ExpandAtArgs<'a> { #[cfg(test)] mod test { use super::{ - dist, generate_compile_commands, parse_arguments, Language, OsString, ParsedArguments, ARGS, + dist, fs, generate_compile_commands, parse_arguments, Language, OsString, ParsedArguments, + ARGS, }; use crate::compiler::c::ArtifactDescriptor; use crate::compiler::*; use crate::mock_command::*; use crate::test::utils::*; - use std::fs::File; + use fs::File; use std::io::Write; fn parse_arguments_(arguments: Vec) -> CompilerArguments { diff --git a/src/compiler/gcc.rs b/src/compiler/gcc.rs index 58823537..e955db8f 100644 --- a/src/compiler/gcc.rs +++ b/src/compiler/gcc.rs @@ -20,10 +20,11 @@ use crate::compiler::{clang, Cacheable, ColorMode, CompileCommand, CompilerArgum use crate::mock_command::{CommandCreatorSync, RunCommand}; use crate::util::{run_input_output, OsStrExt}; use crate::{counted_array, dist}; +use fs::File; +use fs_err as fs; use log::Level::Trace; use std::collections::HashMap; use std::ffi::OsString; -use std::fs::File; use std::io::Read; use std::path::{Path, PathBuf}; use std::process; @@ -871,7 +872,7 @@ impl<'a> Iterator for ExpandIncludeFile<'a> { #[cfg(test)] mod test { - use std::fs::File; + use fs::File; use std::io::Write; use super::*; diff --git a/src/compiler/msvc.rs b/src/compiler/msvc.rs index 689e35c1..c5b84f9d 100644 --- a/src/compiler/msvc.rs +++ b/src/compiler/msvc.rs @@ -22,10 +22,11 @@ use crate::compiler::{ use crate::mock_command::{CommandCreatorSync, RunCommand}; use crate::util::run_input_output; use crate::{counted_array, dist}; +use fs::File; +use fs_err as fs; use log::Level::Debug; use std::collections::{HashMap, HashSet}; use std::ffi::{OsStr, OsString}; -use std::fs::File; use std::io::{self, BufWriter, Write}; use std::path::{Path, PathBuf}; use std::process::{self, Stdio}; diff --git a/src/compiler/nvcc.rs b/src/compiler/nvcc.rs index dacc6c08..db1d5a5c 100644 --- a/src/compiler/nvcc.rs +++ b/src/compiler/nvcc.rs @@ -23,9 +23,10 @@ use crate::compiler::{gcc, write_temp_file, Cacheable, CompileCommand, CompilerA use crate::mock_command::{CommandCreator, CommandCreatorSync, RunCommand}; use crate::util::{run_input_output, OsStrExt}; use crate::{counted_array, dist}; +use fs::File; +use fs_err as fs; use log::Level::Trace; use std::ffi::OsString; -use std::fs::File; use std::future::Future; use std::io::{self, Write}; use std::path::{Path, PathBuf}; diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index f9bf3c97..95895ca2 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -29,6 +29,7 @@ use crate::util::{fmt_duration_as_secs, hash_all, hash_all_archives, run_input_o use crate::util::{ref_env, HashToDigest, OsStrExt}; use crate::{counted_array, dist}; use filetime::FileTime; +use fs_err as fs; use log::Level::Trace; #[cfg(feature = "dist-client")] #[cfg(feature = "dist-client")] @@ -42,7 +43,6 @@ use std::env::consts::DLL_EXTENSION; use std::env::consts::{DLL_PREFIX, EXE_EXTENSION}; use std::ffi::OsString; use std::fmt; -use std::fs; use std::future::Future; use std::hash::Hash; #[cfg(feature = "dist-client")] @@ -280,7 +280,7 @@ where T: AsRef, U: AsRef, { - let mut f = fs::File::open(file)?; + let mut f = fs::File::open(file.as_ref())?; let mut deps = String::new(); f.read_to_string(&mut deps)?; Ok((parse_dep_info(&deps, cwd), parse_env_dep_info(&deps))) @@ -2405,9 +2405,9 @@ mod test { use crate::compiler::*; use crate::mock_command::*; use crate::test::utils::*; + use fs::File; use itertools::Itertools; use std::ffi::OsStr; - use std::fs::File; use std::io::{self, Write}; use std::sync::{Arc, Mutex}; diff --git a/src/config.rs b/src/config.rs index ec354836..949b3708 100644 --- a/src/config.rs +++ b/src/config.rs @@ -13,6 +13,8 @@ // limitations under the License. use directories::ProjectDirs; +use fs::File; +use fs_err as fs; use regex::Regex; use serde::de::{Deserialize, DeserializeOwned, Deserializer}; #[cfg(any(feature = "dist-client", feature = "dist-server"))] @@ -22,7 +24,6 @@ use serde::ser::{Serialize, Serializer}; use serial_test::serial; use std::collections::HashMap; use std::env; -use std::fs::{self, File}; use std::io::{Read, Write}; use std::path::{Path, PathBuf}; use std::result::Result as StdResult; diff --git a/src/dist/cache.rs b/src/dist/cache.rs index cb9ab37b..946b55b0 100644 --- a/src/dist/cache.rs +++ b/src/dist/cache.rs @@ -2,7 +2,7 @@ use crate::dist::Toolchain; use crate::lru_disk_cache::Result as LruResult; use crate::lru_disk_cache::{LruDiskCache, ReadSeek}; use anyhow::{anyhow, Result}; -use std::fs; +use fs_err as fs; use std::io; use std::path::{Path, PathBuf}; @@ -18,8 +18,8 @@ mod client { use crate::dist::Toolchain; use crate::lru_disk_cache::Error as LruError; use anyhow::{bail, Context, Error, Result}; + use fs_err as fs; use std::collections::{HashMap, HashSet}; - use std::fs; use std::io::Write; use std::path::{Path, PathBuf}; use std::sync::Mutex; @@ -214,7 +214,7 @@ mod client { debug!("Weak key {} appears to be new", weak_key); let tmpfile = tempfile::NamedTempFile::new_in(self.cache_dir.join("toolchain_tmp"))?; toolchain_packager - .write_pkg(tmpfile.reopen()?) + .write_pkg(fs_err::File::from_parts(tmpfile.reopen()?, tmpfile.path())) .context("Could not package toolchain")?; let tc = cache.insert_file(tmpfile.path())?; self.record_weak(weak_key.to_owned(), tc.archive_id.clone())?; @@ -304,7 +304,7 @@ mod client { } #[cfg(all(target_os = "linux", target_arch = "x86_64"))] impl crate::dist::pkg::ToolchainPackager for PanicToolchainPackager { - fn write_pkg(self: Box, _f: ::std::fs::File) -> crate::errors::Result<()> { + fn write_pkg(self: Box, _f: super::fs::File) -> crate::errors::Result<()> { panic!("should not have called packager") } } diff --git a/src/dist/http.rs b/src/dist/http.rs index b4e9f7e1..6bb78297 100644 --- a/src/dist/http.rs +++ b/src/dist/http.rs @@ -1239,7 +1239,7 @@ mod client { Ok(Some(toolchain_file)) => { let url = urls::server_submit_toolchain(job_alloc.server_id, job_alloc.job_id); let req = self.client.lock().unwrap().post(url); - let toolchain_file = tokio::fs::File::from_std(toolchain_file); + let toolchain_file = tokio::fs::File::from_std(toolchain_file.into()); let toolchain_file_stream = tokio_util::io::ReaderStream::new(toolchain_file); let body = Body::wrap_stream(toolchain_file_stream); let req = req.bearer_auth(job_alloc.auth).body(body); diff --git a/src/dist/pkg.rs b/src/dist/pkg.rs index fc22031f..3e50ce54 100644 --- a/src/dist/pkg.rs +++ b/src/dist/pkg.rs @@ -13,7 +13,7 @@ // limitations under the License. use crate::dist; -use std::fs; +use fs_err as fs; use std::io; use std::path::{Component, Path, PathBuf}; use std::str; @@ -38,7 +38,7 @@ pub trait OutputsRepackager { #[cfg(not(all(target_os = "linux", target_arch = "x86_64")))] mod toolchain_imp { use super::ToolchainPackager; - use std::fs; + use fs_err as fs; use crate::errors::*; @@ -54,8 +54,8 @@ mod toolchain_imp { #[cfg(all(target_os = "linux", target_arch = "x86_64"))] mod toolchain_imp { use super::tarify_path; + use fs_err as fs; use std::collections::BTreeMap; - use std::fs; use std::io::{Read, Write}; use std::path::{Component, Path, PathBuf}; use std::process; @@ -176,7 +176,7 @@ mod toolchain_imp { } for (tar_path, file_path) in file_set.into_iter() { let file = &mut fs::File::open(file_path)?; - builder.append_file(tar_path, file)? + builder.append_file(tar_path, file.file_mut())? } builder.finish().map_err(Into::into) } diff --git a/src/lru_disk_cache/mod.rs b/src/lru_disk_cache/mod.rs index 924dfa79..7d837ec0 100644 --- a/src/lru_disk_cache/mod.rs +++ b/src/lru_disk_cache/mod.rs @@ -1,12 +1,13 @@ pub mod lru_cache; +use fs::File; +use fs_err as fs; use std::borrow::Borrow; use std::boxed::Box; use std::collections::hash_map::RandomState; use std::error::Error as StdError; use std::ffi::{OsStr, OsString}; use std::fmt; -use std::fs::{self, File}; use std::hash::BuildHasher; use std::io; use std::io::prelude::*; @@ -315,10 +316,10 @@ impl LruDiskCache { #[cfg(test)] mod tests { + use super::fs::{self, File}; use super::{Error, LruDiskCache}; use filetime::{set_file_times, FileTime}; - use std::fs::{self, File}; use std::io::{self, Read, Write}; use std::path::{Path, PathBuf}; use tempfile::TempDir; diff --git a/src/server.rs b/src/server.rs index c7c2ba76..40643d88 100644 --- a/src/server.rs +++ b/src/server.rs @@ -29,6 +29,8 @@ use crate::util; use anyhow::Context as _; use bytes::{buf::BufMut, Bytes, BytesMut}; use filetime::FileTime; +use fs::metadata; +use fs_err as fs; use futures::channel::mpsc; use futures::future::FutureExt; use futures::{future, stream, Sink, SinkExt, Stream, StreamExt, TryFutureExt}; @@ -36,7 +38,6 @@ use number_prefix::NumberPrefix; use std::collections::HashMap; use std::env; use std::ffi::{OsStr, OsString}; -use std::fs::metadata; use std::future::Future; use std::io::{self, Write}; use std::marker::Unpin; @@ -114,7 +115,7 @@ fn notify_server_startup(name: &Option, status: ServerStartup) -> Resu #[cfg(windows)] fn notify_server_startup(name: &Option, status: ServerStartup) -> Result<()> { - use std::fs::OpenOptions; + use fs::OpenOptions; let name = match *name { Some(ref s) => s, diff --git a/src/test/tests.rs b/src/test/tests.rs index 3da473ff..821ba91e 100644 --- a/src/test/tests.rs +++ b/src/test/tests.rs @@ -19,10 +19,11 @@ use crate::jobserver::Client; use crate::mock_command::*; use crate::server::{DistClientContainer, SccacheServer, ServerMessage}; use crate::test::utils::*; +use fs::File; +use fs_err as fs; use futures::channel::oneshot::{self, Sender}; #[cfg(not(target_os = "macos"))] use serial_test::serial; -use std::fs::File; use std::io::{Cursor, Write}; #[cfg(not(target_os = "macos"))] use std::net::TcpListener; diff --git a/src/test/utils.rs b/src/test/utils.rs index 0278f821..dac5acbe 100644 --- a/src/test/utils.rs +++ b/src/test/utils.rs @@ -13,10 +13,11 @@ // limitations under the License. use crate::mock_command::*; +use fs::File; +use fs_err as fs; use std::collections::HashMap; use std::env; use std::ffi::OsString; -use std::fs::{self, File}; use std::future::Future; use std::io; use std::path::{Path, PathBuf}; @@ -150,7 +151,7 @@ pub fn mk_bin_contents io::Result<()>>( path: &str, fill_contents: F, ) -> io::Result { - use std::os::unix::fs::OpenOptionsExt; + use fs_err::os::unix::fs::OpenOptionsExt; let bin = dir.join(path); let parent = bin.parent().unwrap(); fs::create_dir_all(parent)?; @@ -202,7 +203,7 @@ impl TestFixture { .prefix("sccache_test") .tempdir() .unwrap(); - let mut builder = fs::DirBuilder::new(); + let mut builder = std::fs::DirBuilder::new(); builder.recursive(true); let mut paths = vec![]; let mut bins = vec![]; diff --git a/src/util.rs b/src/util.rs index b3441b3e..8d85f541 100644 --- a/src/util.rs +++ b/src/util.rs @@ -16,9 +16,10 @@ use crate::mock_command::{CommandChild, RunCommand}; use ar::Archive; use blake3::Hasher as blake3_Hasher; use byteorder::{BigEndian, ByteOrder}; +use fs::File; +use fs_err as fs; use serde::Serialize; use std::ffi::{OsStr, OsString}; -use std::fs::File; use std::hash::Hasher; use std::io::prelude::*; use std::path::{Path, PathBuf}; diff --git a/tests/harness/mod.rs b/tests/harness/mod.rs index 84ed3687..3bb84660 100644 --- a/tests/harness/mod.rs +++ b/tests/harness/mod.rs @@ -1,9 +1,9 @@ +use fs_err as fs; #[cfg(any(feature = "dist-client", feature = "dist-server"))] use sccache::config::HTTPUrl; use sccache::dist::{self, SchedulerStatusResult, ServerId}; use sccache::server::ServerInfo; use std::env; -use std::fs; use std::io::Write; use std::net::{self, IpAddr, SocketAddr}; use std::path::{Path, PathBuf}; diff --git a/tests/oauth.rs b/tests/oauth.rs index ce88f622..3548efbd 100755 --- a/tests/oauth.rs +++ b/tests/oauth.rs @@ -1,7 +1,7 @@ #![deny(rust_2018_idioms)] #![cfg(all(feature = "dist-client"))] -use std::fs; +use fs_err as fs; use std::io::{self, Read, Write}; use std::net::TcpStream; use std::path::Path; diff --git a/tests/sccache_cargo.rs b/tests/sccache_cargo.rs index dbddd10a..ecdab438 100644 --- a/tests/sccache_cargo.rs +++ b/tests/sccache_cargo.rs @@ -8,11 +8,11 @@ use once_cell::sync::Lazy; use assert_cmd::prelude::*; use chrono::Local; +use fs_err as fs; use predicates::prelude::*; use serial_test::serial; use std::convert::Infallible; use std::ffi::OsString; -use std::fs; use std::io::Write; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; diff --git a/tests/system.rs b/tests/system.rs index 8979140e..977977c2 100644 --- a/tests/system.rs +++ b/tests/system.rs @@ -24,6 +24,8 @@ use crate::harness::{ write_json_cfg, write_source, zero_stats, }; use assert_cmd::prelude::*; +use fs::File; +use fs_err as fs; use log::Level::Trace; use predicates::prelude::*; use regex::Regex; @@ -31,7 +33,6 @@ use std::collections::HashMap; use std::env; use std::ffi::{OsStr, OsString}; use std::fmt; -use std::fs::{self, File}; use std::io::{self, Read, Write}; use std::path::{Path, PathBuf}; use std::process::{Command, Output, Stdio};