From d0e6a6271c6900907ff87548e3ed4455d249eade Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Wed, 19 Mar 2025 22:27:38 +0100 Subject: [PATCH 1/6] join: move to thiserror --- src/uu/join/Cargo.toml | 1 + src/uu/join/src/join.rs | 28 +++++++--------------------- 2 files changed, 8 insertions(+), 21 deletions(-) diff --git a/src/uu/join/Cargo.toml b/src/uu/join/Cargo.toml index d9835833f..a9689e95e 100644 --- a/src/uu/join/Cargo.toml +++ b/src/uu/join/Cargo.toml @@ -20,6 +20,7 @@ path = "src/join.rs" clap = { workspace = true } uucore = { workspace = true } memchr = { workspace = true } +thiserror = { workspace = true } [[bin]] name = "join" diff --git a/src/uu/join/src/join.rs b/src/uu/join/src/join.rs index 19e74aa5f..593100b7c 100644 --- a/src/uu/join/src/join.rs +++ b/src/uu/join/src/join.rs @@ -9,14 +9,13 @@ use clap::builder::ValueParser; use clap::{Arg, ArgAction, Command}; use memchr::{memchr_iter, memmem::Finder, Memchr3}; use std::cmp::Ordering; -use std::error::Error; use std::ffi::OsString; -use std::fmt::Display; use std::fs::File; use std::io::{stdin, stdout, BufRead, BufReader, BufWriter, Split, Stdin, Write}; use std::num::IntErrorKind; #[cfg(unix)] use std::os::unix::ffi::OsStrExt; +use thiserror::Error; use uucore::display::Quotable; use uucore::error::{set_exit_code, FromIo, UError, UResult, USimpleError}; use uucore::line_ending::LineEnding; @@ -25,35 +24,22 @@ use uucore::{format_usage, help_about, help_usage}; const ABOUT: &str = help_about!("join.md"); const USAGE: &str = help_usage!("join.md"); -#[derive(Debug)] +#[derive(Debug, Error)] enum JoinError { - IOError(std::io::Error), + #[error("io error: {0}")] + IOError(#[from] std::io::Error), + + #[error("{0}")] UnorderedInput(String), } +// If you still need the UError implementation for compatibility: impl UError for JoinError { fn code(&self) -> i32 { 1 } } -impl Error for JoinError {} - -impl Display for JoinError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::IOError(e) => write!(f, "io error: {e}"), - Self::UnorderedInput(e) => f.write_str(e), - } - } -} - -impl From for JoinError { - fn from(error: std::io::Error) -> Self { - Self::IOError(error) - } -} - #[derive(Copy, Clone, PartialEq)] enum FileNum { File1, From c1bb57fd1e0363996363f548bb86674a5e31c8b8 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Wed, 19 Mar 2025 23:03:50 +0100 Subject: [PATCH 2/6] ln: move to thiserror --- src/uu/ln/Cargo.toml | 1 + src/uu/ln/src/ln.rs | 44 +++++++++++++++++--------------------------- 2 files changed, 18 insertions(+), 27 deletions(-) diff --git a/src/uu/ln/Cargo.toml b/src/uu/ln/Cargo.toml index 5038f4564..2101a6605 100644 --- a/src/uu/ln/Cargo.toml +++ b/src/uu/ln/Cargo.toml @@ -19,6 +19,7 @@ path = "src/ln.rs" [dependencies] clap = { workspace = true } uucore = { workspace = true, features = ["backup-control", "fs"] } +thiserror = { workspace = true } [[bin]] name = "ln" diff --git a/src/uu/ln/src/ln.rs b/src/uu/ln/src/ln.rs index fef1b1e0b..2529fdbf7 100644 --- a/src/uu/ln/src/ln.rs +++ b/src/uu/ln/src/ln.rs @@ -13,10 +13,9 @@ use uucore::{format_usage, help_about, help_section, help_usage, prompt_yes, sho use std::borrow::Cow; use std::collections::HashSet; -use std::error::Error; use std::ffi::OsString; -use std::fmt::Display; use std::fs; +use thiserror::Error; #[cfg(any(unix, target_os = "redox"))] use std::os::unix::fs::symlink; @@ -46,38 +45,25 @@ pub enum OverwriteMode { Force, } -#[derive(Debug)] +#[derive(Error, Debug)] enum LnError { + #[error("target {} is not a directory", _0.quote())] TargetIsDirectory(PathBuf), + + #[error("")] SomeLinksFailed, + + #[error("{} and {} are the same file", _0.quote(), _1.quote())] SameFile(PathBuf, PathBuf), + + #[error("missing destination file operand after {}", _0.quote())] MissingDestination(PathBuf), - ExtraOperand(OsString), -} -impl Display for LnError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::TargetIsDirectory(s) => write!(f, "target {} is not a directory", s.quote()), - Self::SameFile(s, d) => { - write!(f, "{} and {} are the same file", s.quote(), d.quote()) - } - Self::SomeLinksFailed => Ok(()), - Self::MissingDestination(s) => { - write!(f, "missing destination file operand after {}", s.quote()) - } - Self::ExtraOperand(s) => write!( - f, - "extra operand {}\nTry '{} --help' for more information.", - s.quote(), - uucore::execution_phrase() - ), - } - } + #[error("extra operand {}\nTry '{} --help' for more information.", + format!("{:?}", _0).trim_matches('"'), _1)] + ExtraOperand(OsString, String), } -impl Error for LnError {} - impl UError for LnError { fn code(&self) -> i32 { 1 @@ -284,7 +270,11 @@ fn exec(files: &[PathBuf], settings: &Settings) -> UResult<()> { return Err(LnError::MissingDestination(files[0].clone()).into()); } if files.len() > 2 { - return Err(LnError::ExtraOperand(files[2].clone().into()).into()); + return Err(LnError::ExtraOperand( + files[2].clone().into(), + uucore::execution_phrase().to_string(), + ) + .into()); } assert!(!files.is_empty()); From 9d123febb3a950414ff5c16fcae0d4e631e1f4aa Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 21 Mar 2025 16:31:55 +0100 Subject: [PATCH 3/6] install: move to thiserror --- src/uu/install/Cargo.toml | 1 + src/uu/install/src/install.rs | 97 +++++++++++++++-------------------- 2 files changed, 41 insertions(+), 57 deletions(-) diff --git a/src/uu/install/Cargo.toml b/src/uu/install/Cargo.toml index dd9a5c8ae..3b2724436 100644 --- a/src/uu/install/Cargo.toml +++ b/src/uu/install/Cargo.toml @@ -21,6 +21,7 @@ clap = { workspace = true } filetime = { workspace = true } file_diff = { workspace = true } libc = { workspace = true } +thiserror = { workspace = true } uucore = { workspace = true, features = [ "backup-control", "buf-copy", diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index bffc93974..42de1ef0c 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -10,22 +10,22 @@ mod mode; use clap::{Arg, ArgAction, ArgMatches, Command}; use file_diff::diff; use filetime::{set_file_times, FileTime}; -use std::error::Error; -use std::fmt::{Debug, Display}; +use std::fmt::Debug; use std::fs::File; use std::fs::{self, metadata}; use std::path::{Path, PathBuf, MAIN_SEPARATOR}; use std::process; +use thiserror::Error; use uucore::backup_control::{self, BackupMode}; use uucore::buf_copy::copy_stream; use uucore::display::Quotable; use uucore::entries::{grp2gid, usr2uid}; -use uucore::error::{FromIo, UError, UIoError, UResult, UUsageError}; +use uucore::error::{FromIo, UError, UResult, UUsageError}; use uucore::fs::dir_strip_dot_for_creation; use uucore::mode::get_umask; use uucore::perms::{wrap_chown, Verbosity, VerbosityLevel}; use uucore::process::{getegid, geteuid}; -use uucore::{format_usage, help_about, help_usage, show, show_error, show_if_err, uio_error}; +use uucore::{format_usage, help_about, help_usage, show, show_error, show_if_err}; #[cfg(unix)] use std::os::unix::fs::{FileTypeExt, MetadataExt}; @@ -52,22 +52,51 @@ pub struct Behavior { target_dir: Option, } -#[derive(Debug)] +#[derive(Error, Debug)] enum InstallError { + #[error("Unimplemented feature: {0}")] Unimplemented(String), - DirNeedsArg(), - CreateDirFailed(PathBuf, std::io::Error), + + #[error("{} with -d requires at least one argument.", uucore::util_name())] + DirNeedsArg, + + #[error("failed to create {0}")] + CreateDirFailed(PathBuf, #[source] std::io::Error), + + #[error("failed to chmod {}", .0.quote())] ChmodFailed(PathBuf), + + #[error("failed to chown {}: {}", .0.quote(), .1)] ChownFailed(PathBuf, String), + + #[error("invalid target {}: No such file or directory", .0.quote())] InvalidTarget(PathBuf), + + #[error("target {} is not a directory", .0.quote())] TargetDirIsntDir(PathBuf), - BackupFailed(PathBuf, PathBuf, std::io::Error), - InstallFailed(PathBuf, PathBuf, std::io::Error), + + #[error("cannot backup {0} to {1}")] + BackupFailed(PathBuf, PathBuf, #[source] std::io::Error), + + #[error("cannot install {0} to {1}")] + InstallFailed(PathBuf, PathBuf, #[source] std::io::Error), + + #[error("strip program failed: {0}")] StripProgramFailed(String), - MetadataFailed(std::io::Error), + + #[error("metadata error")] + MetadataFailed(#[source] std::io::Error), + + #[error("invalid user: {}", .0.quote())] InvalidUser(String), + + #[error("invalid group: {}", .0.quote())] InvalidGroup(String), + + #[error("omitting directory {}", .0.quote())] OmittingDirectory(PathBuf), + + #[error("failed to access {}: Not a directory", .0.quote())] NotADirectory(PathBuf), } @@ -84,52 +113,6 @@ impl UError for InstallError { } } -impl Error for InstallError {} - -impl Display for InstallError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Unimplemented(opt) => write!(f, "Unimplemented feature: {opt}"), - Self::DirNeedsArg() => { - write!( - f, - "{} with -d requires at least one argument.", - uucore::util_name() - ) - } - Self::CreateDirFailed(dir, e) => { - Display::fmt(&uio_error!(e, "failed to create {}", dir.quote()), f) - } - Self::ChmodFailed(file) => write!(f, "failed to chmod {}", file.quote()), - Self::ChownFailed(file, msg) => write!(f, "failed to chown {}: {}", file.quote(), msg), - Self::InvalidTarget(target) => write!( - f, - "invalid target {}: No such file or directory", - target.quote() - ), - Self::TargetDirIsntDir(target) => { - write!(f, "target {} is not a directory", target.quote()) - } - Self::BackupFailed(from, to, e) => Display::fmt( - &uio_error!(e, "cannot backup {} to {}", from.quote(), to.quote()), - f, - ), - Self::InstallFailed(from, to, e) => Display::fmt( - &uio_error!(e, "cannot install {} to {}", from.quote(), to.quote()), - f, - ), - Self::StripProgramFailed(msg) => write!(f, "strip program failed: {msg}"), - Self::MetadataFailed(e) => Display::fmt(&uio_error!(e, ""), f), - Self::InvalidUser(user) => write!(f, "invalid user: {}", user.quote()), - Self::InvalidGroup(group) => write!(f, "invalid group: {}", group.quote()), - Self::OmittingDirectory(dir) => write!(f, "omitting directory {}", dir.quote()), - Self::NotADirectory(dir) => { - write!(f, "failed to access {}: Not a directory", dir.quote()) - } - } - } -} - #[derive(Clone, Eq, PartialEq)] pub enum MainFunction { /// Create directories @@ -456,7 +439,7 @@ fn behavior(matches: &ArgMatches) -> UResult { /// fn directory(paths: &[String], b: &Behavior) -> UResult<()> { if paths.is_empty() { - Err(InstallError::DirNeedsArg().into()) + Err(InstallError::DirNeedsArg.into()) } else { for path in paths.iter().map(Path::new) { // if the path already exist, don't try to create it again From 305be094032a66bad90809c5b56a0642a904f318 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 21 Mar 2025 17:18:40 +0100 Subject: [PATCH 4/6] ls: move to thiserror --- src/uu/ls/Cargo.toml | 1 + src/uu/ls/src/ls.rs | 142 +++++++++++-------------------------------- 2 files changed, 38 insertions(+), 105 deletions(-) diff --git a/src/uu/ls/Cargo.toml b/src/uu/ls/Cargo.toml index ef7452469..566f558dd 100644 --- a/src/uu/ls/Cargo.toml +++ b/src/uu/ls/Cargo.toml @@ -26,6 +26,7 @@ lscolors = { workspace = true } number_prefix = { workspace = true } selinux = { workspace = true, optional = true } terminal_size = { workspace = true } +thiserror = { workspace = true } uucore = { workspace = true, features = [ "colors", "custom-tz-fmt", diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 3765fc5b6..4d9899169 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -10,9 +10,8 @@ use std::os::windows::fs::MetadataExt; use std::{cell::OnceCell, num::IntErrorKind}; use std::{ cmp::Reverse, - error::Error, ffi::{OsStr, OsString}, - fmt::{Display, Write as FmtWrite}, + fmt::Write as FmtWrite, fs::{self, DirEntry, FileType, Metadata, ReadDir}, io::{stdout, BufWriter, ErrorKind, Stdout, Write}, path::{Path, PathBuf}, @@ -35,7 +34,7 @@ use clap::{ use glob::{MatchOptions, Pattern}; use lscolors::LsColors; use term_grid::{Direction, Filling, Grid, GridOptions}; - +use thiserror::Error; use uucore::error::USimpleError; use uucore::format::human::{human_readable, SizeFormat}; #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))] @@ -175,14 +174,41 @@ const POSIXLY_CORRECT_BLOCK_SIZE: u64 = 512; const DEFAULT_BLOCK_SIZE: u64 = 1024; const DEFAULT_FILE_SIZE_BLOCK_SIZE: u64 = 1; -#[derive(Debug)] +#[derive(Error, Debug)] enum LsError { + #[error("invalid line width: '{0}'")] InvalidLineWidth(String), - IOError(std::io::Error), - IOErrorContext(std::io::Error, PathBuf, bool), + + #[error("general io error: {0}")] + IOError(#[from] std::io::Error), + + #[error("{}", match .1.kind() { + ErrorKind::NotFound => format!("cannot access '{}': No such file or directory", .0.to_string_lossy()), + ErrorKind::PermissionDenied => match .1.raw_os_error().unwrap_or(1) { + 1 => format!("cannot access '{}': Operation not permitted", .0.to_string_lossy()), + _ => if .0.is_dir() { + format!("cannot open directory '{}': Permission denied", .0.to_string_lossy()) + } else { + format!("cannot open file '{}': Permission denied", .0.to_string_lossy()) + }, + }, + _ => match .1.raw_os_error().unwrap_or(1) { + 9 => format!("cannot open directory '{}': Bad file descriptor", .0.to_string_lossy()), + _ => format!("unknown io error: '{:?}', '{:?}'", .0.to_string_lossy(), .1), + }, + })] + IOErrorContext(PathBuf, std::io::Error, bool), + + #[error("invalid --block-size argument '{0}'")] BlockSizeParseError(String), + + #[error("--dired and --zero are incompatible")] DiredAndZeroAreIncompatible, + + #[error("{}: not listing already-listed directory", .0.to_string_lossy())] AlreadyListedError(PathBuf), + + #[error("invalid --time-style argument {0}\nPossible values are: {1:?}\n\nFor more information try --help")] TimeStyleParseError(String, Vec), } @@ -201,100 +227,6 @@ impl UError for LsError { } } -impl Error for LsError {} - -impl Display for LsError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::BlockSizeParseError(s) => { - write!(f, "invalid --block-size argument {}", s.quote()) - } - Self::DiredAndZeroAreIncompatible => { - write!(f, "--dired and --zero are incompatible") - } - Self::TimeStyleParseError(s, possible_time_styles) => { - write!( - f, - "invalid --time-style argument {}\nPossible values are: {:?}\n\nFor more information try --help", - s.quote(), - possible_time_styles - ) - } - Self::InvalidLineWidth(s) => write!(f, "invalid line width: {}", s.quote()), - Self::IOError(e) => write!(f, "general io error: {e}"), - Self::IOErrorContext(e, p, _) => { - let error_kind = e.kind(); - let errno = e.raw_os_error().unwrap_or(1i32); - - match error_kind { - // No such file or directory - ErrorKind::NotFound => { - write!( - f, - "cannot access '{}': No such file or directory", - p.to_string_lossy(), - ) - } - // Permission denied and Operation not permitted - ErrorKind::PermissionDenied => - { - #[allow(clippy::wildcard_in_or_patterns)] - match errno { - 1i32 => { - write!( - f, - "cannot access '{}': Operation not permitted", - p.to_string_lossy(), - ) - } - 13i32 | _ => { - if p.is_dir() { - write!( - f, - "cannot open directory '{}': Permission denied", - p.to_string_lossy(), - ) - } else { - write!( - f, - "cannot open file '{}': Permission denied", - p.to_string_lossy(), - ) - } - } - } - } - _ => match errno { - 9i32 => { - // only should ever occur on a read_dir on a bad fd - write!( - f, - "cannot open directory '{}': Bad file descriptor", - p.to_string_lossy(), - ) - } - _ => { - write!( - f, - "unknown io error: '{:?}', '{:?}'", - p.to_string_lossy(), - e - ) - } - }, - } - } - Self::AlreadyListedError(path) => { - write!( - f, - "{}: not listing already-listed directory", - path.to_string_lossy() - ) - } - } - } -} - #[derive(PartialEq, Eq, Debug)] pub enum Format { Columns, @@ -2054,8 +1986,8 @@ impl PathData { } } show!(LsError::IOErrorContext( - err, self.p_buf.clone(), + err, self.command_line )); None @@ -2161,8 +2093,8 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { // flush stdout buffer before the error to preserve formatting and order out.flush()?; show!(LsError::IOErrorContext( - err, path_data.p_buf.clone(), + err, path_data.command_line )); continue; @@ -2396,8 +2328,8 @@ fn enter_directory( Err(err) => { out.flush()?; show!(LsError::IOErrorContext( - err, e.p_buf.clone(), + err, e.command_line )); continue; @@ -3365,7 +3297,7 @@ fn display_item_name( } } Err(err) => { - show!(LsError::IOErrorContext(err, path.p_buf.clone(), false)); + show!(LsError::IOErrorContext(path.p_buf.clone(), err, false)); } } } @@ -3448,7 +3380,7 @@ fn get_security_context(config: &Config, p_buf: &Path, must_dereference: bool) - Err(err) => { // The Path couldn't be dereferenced, so return early and set exit code 1 // to indicate a minor error - show!(LsError::IOErrorContext(err, p_buf.to_path_buf(), false)); + show!(LsError::IOErrorContext(p_buf.to_path_buf(), err, false)); return substitute_string; } Ok(_md) => (), From 5e1677bb9e1e9dc4a81968d31344489210a02698 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 21 Mar 2025 17:34:29 +0100 Subject: [PATCH 5/6] adjust cargo.lock --- Cargo.lock | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 1bfb2778c..acf4bcd2c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2850,6 +2850,7 @@ dependencies = [ "file_diff", "filetime", "libc", + "thiserror 2.0.12", "uucore", ] @@ -2859,6 +2860,7 @@ version = "0.0.30" dependencies = [ "clap", "memchr", + "thiserror 2.0.12", "uucore", ] @@ -2884,6 +2886,7 @@ name = "uu_ln" version = "0.0.30" dependencies = [ "clap", + "thiserror 2.0.12", "uucore", ] @@ -2909,6 +2912,7 @@ dependencies = [ "number_prefix", "selinux", "terminal_size", + "thiserror 2.0.12", "uucore", "uutils_term_grid", ] From ffe8762ee69bac18557dc2f58ab821a8fc5cc424 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 23 Mar 2025 09:00:02 +0100 Subject: [PATCH 6/6] Fix the GNU test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Dorian Péron <72708393+RenjiSann@users.noreply.github.com> --- src/uu/ls/src/ls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 4d9899169..e276eaa11 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -208,7 +208,7 @@ enum LsError { #[error("{}: not listing already-listed directory", .0.to_string_lossy())] AlreadyListedError(PathBuf), - #[error("invalid --time-style argument {0}\nPossible values are: {1:?}\n\nFor more information try --help")] + #[error("invalid --time-style argument {}\nPossible values are: {:?}\n\nFor more information try --help", .0.quote(), .1)] TimeStyleParseError(String, Vec), }