uutils/tar
0
0
Fork 0
mirror of https://github.com/uutils/tar.git synced 2026-06-10 16:14:35 -07:00
Code Issues Packages Projects Releases Wiki Activity

tar: replace all string-based error variants with structured types (#149)

Browse Source 
* tar: replace all string-based error variants with structured types

Every TarError variant now carries PathBuf and/or io::Error data
instead of a pre-formatted String. Add thiserror derive macros so
Display implementations are generated from #[error(...)] attributes
in one central place. Eliminates the last generic TarOperation and
InvalidArchive string variants.

Add tar prefixes, update tests to match.
This commit is contained in:
Jeff Bailey
2026-04-04 23:47:25 +01:00
committed by GitHub
parent b7c0578352
commit f541b90a9c
8 changed files with 155 additions and 126 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Cargo.lock
Generated
+1
View File
@@ -1405,6 +1405,7 @@ dependencies = [
"clap",
"regex",
"tar",
"thiserror",
"uucore",
]
Cargo.toml
+1
View File
@@ -43,6 +43,7 @@ phf = "0.13.0"
phf_codegen = "0.13.0"
rand = { version = "0.10.0" }
regex = "1.10.4"
thiserror = "2.0.3"
uucore = { git = "https://github.com/uutils/coreutils" }
uutests = { git = "https://github.com/uutils/coreutils" }
tar = "0.4"
src/uu/tar/Cargo.toml
+1
View File
@@ -18,6 +18,7 @@ clap = { workspace = true }
regex = { workspace = true }
tar = { workspace = true }
chrono = { workspace = true }
thiserror = { workspace = true }
[lib]
path = "src/tar.rs"
src/uu/tar/src/errors.rs
+53 -59
View File
@@ -3,62 +3,70 @@
// For the full copyright and license information, please view the LICENSE
// file that was distributed with this source code.
use std::fmt;
use std::io;
use std::path::PathBuf;
use thiserror::Error;
use uucore::error::UError;
/// Error types for tar operations
#[derive(Debug)]
#[derive(Debug, Error)]
pub enum TarError {
/// I/O error occurred
IoError(io::Error),
/// Invalid archive format or corrupted archive
InvalidArchive(String),
/// File or directory not found
FileNotFound(String),
/// Permission denied
PermissionDenied(String),
/// General tar operation error
TarOperationError(String),
}
/// I/O error occurred while reading/writing
#[error("{0}")]
Io(#[from] io::Error),
/// Implements display formatting for TarError.
impl fmt::Display for TarError {
/// Formats the error for display to users
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
TarError::IoError(err) => write!(f, "{err}"),
TarError::InvalidArchive(msg) => write!(f, "{msg}"),
TarError::FileNotFound(path) => {
write!(f, "{path}: Cannot open: No such file or directory")
}
TarError::PermissionDenied(path) => {
write!(f, "{path}: Cannot open: Permission denied")
}
TarError::TarOperationError(msg) => write!(f, "{msg}"),
}
}
/// Cannot read entries from archive
#[error("tar: Cannot read archive entries: {0}")]
CannotReadEntries(io::Error),
/// Cannot read an individual archive entry
#[error("tar: Cannot read archive entry: {0}")]
CannotReadEntry(io::Error),
/// Cannot read the path of an archive entry
#[error("tar: Cannot read entry path: {0}")]
CannotReadEntryPath(io::Error),
/// File or directory not found
#[error("tar: {path}: Cannot open: No such file or directory")]
FileNotFound { path: PathBuf },
/// Permission denied when accessing file
#[error("tar: {path}: Cannot open: Permission denied")]
PermissionDenied { path: PathBuf },
/// Cannot create archive file
#[error("tar: Cannot create archive '{path}': {source}")]
CannotCreateArchive { path: PathBuf, source: io::Error },
/// Cannot add a directory to the archive
#[error("tar: Cannot add directory '{path}': {source}")]
CannotAddDirectory { path: PathBuf, source: io::Error },
/// Cannot add a file to the archive
#[error("tar: Cannot add file '{path}': {source}")]
CannotAddFile { path: PathBuf, source: io::Error },
/// Cannot extract an archive entry
#[error("tar: Cannot extract '{path}': {source}")]
CannotExtract { path: PathBuf, source: io::Error },
/// Cannot finalize the archive
#[error("tar: Cannot finalize archive: {0}")]
CannotFinalizeArchive(io::Error),
}
impl TarError {
/// Create a TarError from an io::Error with file path context
pub fn from_io_error(err: io::Error, path: &std::path::Path) -> Self {
match err.kind() {
io::ErrorKind::NotFound => TarError::FileNotFound(path.display().to_string()),
io::ErrorKind::PermissionDenied => {
TarError::PermissionDenied(path.display().to_string())
}
_ => TarError::IoError(err),
}
}
}
impl std::error::Error for TarError {
/// Returns the underlying error cause, if any
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
match self {
TarError::IoError(err) => Some(err),
_ => None,
io::ErrorKind::NotFound => TarError::FileNotFound {
path: path.to_path_buf(),
},
io::ErrorKind::PermissionDenied => TarError::PermissionDenied {
path: path.to_path_buf(),
},
_ => TarError::Io(err),
}
}
}
@@ -66,20 +74,6 @@ impl std::error::Error for TarError {
impl UError for TarError {
/// Returns the exit code for this error type
fn code(&self) -> i32 {
match self {
TarError::IoError(_) => 2,
TarError::InvalidArchive(_) => 2,
TarError::FileNotFound(_) => 2,
TarError::PermissionDenied(_) => 2,
TarError::TarOperationError(_) => 2,
}
}
}
impl From<io::Error> for TarError {
/// Converts io::Error into the appropriate TarError variant
fn from(err: io::Error) -> Self {
// For generic io::Error without context, just wrap it
TarError::IoError(err)
2 // TarError variants exit with code 2; argument/usage errors use code 64 (see tar.rs)
}
}
src/uu/tar/src/operations/create.rs
+15 -21
View File
@@ -27,12 +27,9 @@ use uucore::error::UResult;
/// - Files cannot be added due to I/O or permission errors
pub fn create_archive(archive_path: &Path, files: &[&Path], verbose: bool) -> UResult<()> {
// Create the output file
let file = File::create(archive_path).map_err(|e| {
TarError::TarOperationError(format!(
"Cannot create archive '{}': {}",
archive_path.display(),
e
))
let file = File::create(archive_path).map_err(|e| TarError::CannotCreateArchive {
path: archive_path.to_path_buf(),
source: e,
})?;
// Create Builder instance
@@ -42,7 +39,10 @@ pub fn create_archive(archive_path: &Path, files: &[&Path], verbose: bool) -> UR
for &path in files {
// Check if path exists
if !path.exists() {
return Err(TarError::FileNotFound(path.display().to_string()).into());
return Err(TarError::FileNotFound {
path: path.to_path_buf(),
}
.into());
}
if verbose {
@@ -81,30 +81,24 @@ pub fn create_archive(archive_path: &Path, files: &[&Path], verbose: bool) -> UR
// If it's a directory, recursively add all contents
if path.is_dir() {
builder.append_dir_all(normalized_name, path).map_err(|e| {
TarError::TarOperationError(format!(
"Failed to add directory '{}': {}",
path.display(),
e
))
TarError::CannotAddDirectory {
path: path.to_path_buf(),
source: e,
}
})?;
} else {
// For files, add them directly
builder
.append_path_with_name(path, normalized_name)
.map_err(|e| {
TarError::TarOperationError(format!(
"Failed to add file '{}': {}",
path.display(),
e
))
.map_err(|e| TarError::CannotAddFile {
path: path.to_path_buf(),
source: e,
})?;
}
}
// Finish writing the archive
builder
.finish()
.map_err(|e| TarError::TarOperationError(format!("Failed to finalize archive: {e}")))?;
builder.finish().map_err(TarError::CannotFinalizeArchive)?;
Ok(())
}
src/uu/tar/src/operations/extract.rs
+6 -9
View File
@@ -35,17 +35,13 @@ pub fn extract_archive(archive_path: &Path, verbose: bool) -> UResult<()> {
}
// Iterate through entries for verbose output and error handling
for entry_result in archive
.entries()
.map_err(|e| TarError::InvalidArchive(format!("Failed to read archive entries: {e}")))?
{
let mut entry = entry_result
.map_err(|e| TarError::InvalidArchive(format!("Failed to read entry: {e}")))?;
for entry_result in archive.entries().map_err(TarError::CannotReadEntries)? {
let mut entry = entry_result.map_err(TarError::CannotReadEntry)?;
// Get the path before unpacking (clone it so we can use it after borrowing entry mutably)
let path = entry
.path()
.map_err(|e| TarError::InvalidArchive(format!("Failed to read entry path: {e}")))?
.map_err(TarError::CannotReadEntryPath)?
.to_path_buf();
if verbose {
@@ -53,8 +49,9 @@ pub fn extract_archive(archive_path: &Path, verbose: bool) -> UResult<()> {
}
// Unpack the entry
entry.unpack_in(".").map_err(|e| {
TarError::TarOperationError(format!("Failed to extract '{}': {}", path.display(), e))
entry.unpack_in(".").map_err(|e| TarError::CannotExtract {
path: path.clone(),
source: e,
})?;
}
src/uu/tar/src/operations/list.rs
+4 -12
View File
@@ -17,12 +17,8 @@ pub fn list_archive(archive_path: &Path, verbose: bool) -> UResult<()> {
File::open(archive_path).map_err(|e| TarError::from_io_error(e, archive_path))?;
let mut archive = Archive::new(file);
for entry_result in archive
.entries()
.map_err(|e| TarError::InvalidArchive(format!("Failed to read archive entries: {e}")))?
{
let entry = entry_result
.map_err(|e| TarError::InvalidArchive(format!("Failed to read entry: {e}")))?;
for entry_result in archive.entries().map_err(TarError::CannotReadEntries)? {
let entry = entry_result.map_err(TarError::CannotReadEntry)?;
if verbose {
// Collect all header fields into owned values before borrowing entry for the path,
@@ -49,9 +45,7 @@ pub fn list_archive(archive_path: &Path, verbose: bool) -> UResult<()> {
)
};
let path = entry
.path()
.map_err(|e| TarError::InvalidArchive(format!("Failed to read entry path: {e}")))?;
let path = entry.path().map_err(TarError::CannotReadEntryPath)?;
let type_char = match entry_type {
tar::EntryType::Directory => 'd',
@@ -79,9 +73,7 @@ pub fn list_archive(archive_path: &Path, verbose: bool) -> UResult<()> {
path.display()
);
} else {
let path = entry
.path()
.map_err(|e| TarError::InvalidArchive(format!("Failed to read entry path: {e}")))?;
let path = entry.path().map_err(TarError::CannotReadEntryPath)?;
println!("{}", path.display());
}
src/uu/tar/tests/test_errors.rs
+74 -25
View File
@@ -4,37 +4,50 @@
// file that was distributed with this source code.
use std::io;
use std::path::PathBuf;
use uu_tar::errors::TarError;
#[test]
fn test_tar_error_display() {
let err = TarError::FileNotFound("test.txt".to_string());
let err = TarError::FileNotFound {
path: PathBuf::from("test.txt"),
};
assert_eq!(
err.to_string(),
"test.txt: Cannot open: No such file or directory"
"tar: test.txt: Cannot open: No such file or directory"
);
let err = TarError::InvalidArchive("corrupted header".to_string());
assert_eq!(err.to_string(), "corrupted header");
let err = TarError::PermissionDenied("/root/file".to_string());
let err = TarError::PermissionDenied {
path: PathBuf::from("/root/file"),
};
assert_eq!(
err.to_string(),
"/root/file: Cannot open: Permission denied"
"tar: /root/file: Cannot open: Permission denied"
);
let err = TarError::TarOperationError("failed to write".to_string());
assert_eq!(err.to_string(), "failed to write");
}
#[test]
fn test_tar_error_code() {
use uucore::error::UError;
assert_eq!(TarError::FileNotFound("test".to_string()).code(), 2);
assert_eq!(TarError::InvalidArchive("test".to_string()).code(), 2);
assert_eq!(TarError::PermissionDenied("test".to_string()).code(), 2);
assert_eq!(TarError::TarOperationError("test".to_string()).code(), 2);
assert_eq!(
TarError::FileNotFound {
path: PathBuf::from("test")
}
.code(),
2
);
assert_eq!(
TarError::PermissionDenied {
path: PathBuf::from("test")
}
.code(),
2
);
assert_eq!(
TarError::Io(io::Error::new(io::ErrorKind::Other, "test")).code(),
2
);
}
#[test]
@@ -43,8 +56,8 @@ fn test_io_error_conversion_not_found() {
let tar_err = TarError::from(io_err);
match tar_err {
TarError::IoError(e) => assert_eq!(e.kind(), io::ErrorKind::NotFound),
_ => panic!("Expected IoError variant"),
TarError::Io(e) => assert_eq!(e.kind(), io::ErrorKind::NotFound),
_ => panic!("Expected Io variant"),
}
}
@@ -54,8 +67,8 @@ fn test_io_error_conversion_permission_denied() {
let tar_err = TarError::from(io_err);
match tar_err {
TarError::IoError(e) => assert_eq!(e.kind(), io::ErrorKind::PermissionDenied),
_ => panic!("Expected IoError variant"),
TarError::Io(e) => assert_eq!(e.kind(), io::ErrorKind::PermissionDenied),
_ => panic!("Expected Io variant"),
}
}
@@ -65,28 +78,64 @@ fn test_io_error_conversion_other() {
let tar_err = TarError::from(io_err);
match tar_err {
TarError::IoError(e) => assert_eq!(e.kind(), io::ErrorKind::BrokenPipe),
_ => panic!("Expected IoError variant"),
TarError::Io(e) => assert_eq!(e.kind(), io::ErrorKind::BrokenPipe),
_ => panic!("Expected Io variant"),
}
}
#[test]
fn test_error_source() {
let io_err = io::Error::other("some error");
let tar_err = TarError::IoError(io_err);
let tar_err = TarError::Io(io_err);
// IoError should have a source
// Io should have a source
assert!(std::error::Error::source(&tar_err).is_some());
// Other variants should not have a source
let tar_err = TarError::FileNotFound("test".to_string());
let tar_err = TarError::FileNotFound {
path: PathBuf::from("test"),
};
assert!(std::error::Error::source(&tar_err).is_none());
}
#[test]
fn test_from_io_error_not_found() {
let io_err = io::Error::new(io::ErrorKind::NotFound, "not found");
let tar_err = TarError::from_io_error(io_err, std::path::Path::new("myfile.txt"));
assert!(matches!(tar_err, TarError::FileNotFound { .. }));
assert_eq!(
tar_err.to_string(),
"tar: myfile.txt: Cannot open: No such file or directory"
);
}
#[test]
fn test_from_io_error_permission_denied() {
let io_err = io::Error::new(io::ErrorKind::PermissionDenied, "denied");
let tar_err = TarError::from_io_error(io_err, std::path::Path::new("/root/secret"));
assert!(matches!(tar_err, TarError::PermissionDenied { .. }));
assert_eq!(
tar_err.to_string(),
"tar: /root/secret: Cannot open: Permission denied"
);
}
#[test]
fn test_from_io_error_other() {
let io_err = io::Error::new(io::ErrorKind::BrokenPipe, "broken");
let tar_err = TarError::from_io_error(io_err, std::path::Path::new("file.txt"));
assert!(matches!(tar_err, TarError::Io(_)));
}
#[test]
fn test_tar_error_is_debug() {
let err = TarError::TarOperationError("test".to_string());
let err = TarError::FileNotFound {
path: PathBuf::from("test"),
};
let debug_str = format!("{err:?}");
assert!(debug_str.contains("TarOperationError"));
assert!(debug_str.contains("FileNotFound"));
assert!(debug_str.contains("test"));
}