From bb83a6283aeb357df3762b32f03cebf444d2c890 Mon Sep 17 00:00:00 2001 From: Nicolas NOSAL Date: Tue, 30 Jun 2026 16:45:02 +0200 Subject: [PATCH] tar: add --backup[=CONTROL] and --suffix options Back up existing files before overwriting during extraction. CONTROL may be none/off, simple/never, numbered/t, existing/nil (default when flag is given: existing). --suffix overrides the simple backup suffix (default: ~). Related to #250 --- benches/bench_tar.rs | 2 + src/uu/tar/src/errors.rs | 4 + src/uu/tar/src/operations/extract.rs | 69 ++++-- src/uu/tar/src/operations/extract_tests.rs | 259 +++++++++++++++++++-- src/uu/tar/src/tar.rs | 62 ++++- 5 files changed, 364 insertions(+), 32 deletions(-) diff --git a/benches/bench_tar.rs b/benches/bench_tar.rs index 638813c..373451d 100644 --- a/benches/bench_tar.rs +++ b/benches/bench_tar.rs @@ -187,6 +187,8 @@ fn extract_archive_20_files(bencher: divan::Bencher) { &archive_path, false, CompressionMode::None, + tar::BackupControl::None, + "~", ) .unwrap(); }); diff --git a/src/uu/tar/src/errors.rs b/src/uu/tar/src/errors.rs index 7938beb..0e50581 100644 --- a/src/uu/tar/src/errors.rs +++ b/src/uu/tar/src/errors.rs @@ -55,6 +55,10 @@ pub enum TarError { #[error("tar: Cannot extract '{path}': {source}")] CannotExtract { path: PathBuf, source: io::Error }, + /// Cannot create backup of an existing destination file + #[error("tar: Cannot backup '{path}': {source}")] + CannotBackup { path: PathBuf, source: io::Error }, + /// General tar operation error #[error("tar: {0}")] TarOperationError(String), diff --git a/src/uu/tar/src/operations/extract.rs b/src/uu/tar/src/operations/extract.rs index 4c36cdc..d86a1f2 100644 --- a/src/uu/tar/src/operations/extract.rs +++ b/src/uu/tar/src/operations/extract.rs @@ -5,46 +5,88 @@ use crate::compression::open_archive_reader; use crate::errors::TarError; -use crate::CompressionMode; +use crate::{BackupControl, CompressionMode}; use std::io::Read; use std::io::{self, BufWriter, Write}; -use std::path::Path; +use std::path::{Path, PathBuf}; use tar::Archive; use uucore::error::UResult; -/// Extract files from a tar archive +fn simple_backup_path(path: &Path, suffix: &str) -> PathBuf { + let mut s = path.as_os_str().to_owned(); + s.push(suffix); + PathBuf::from(s) +} + +fn numbered_backup_path(path: &Path) -> PathBuf { + for n in 1u64.. { + let mut s = path.as_os_str().to_owned(); + s.push(format!(".~{n}~")); + let candidate = PathBuf::from(&s); + if !candidate.exists() { + return candidate; + } + } + unreachable!() +} + +fn backup_file(path: &Path, control: BackupControl, suffix: &str) -> Result<(), TarError> { + if matches!(control, BackupControl::None) || !path.is_file() { + return Ok(()); + } + let backup = match control { + BackupControl::None => unreachable!(), + BackupControl::Simple => simple_backup_path(path, suffix), + BackupControl::Numbered => numbered_backup_path(path), + BackupControl::Existing => { + // use numbered if a .~1~ backup already exists, otherwise simple + let first_numbered = simple_backup_path(path, ".~1~"); + if first_numbered.exists() { + numbered_backup_path(path) + } else { + simple_backup_path(path, suffix) + } + } + }; + std::fs::rename(path, &backup).map_err(|e| TarError::CannotBackup { + path: path.to_path_buf(), + source: e, + }) +} + +/// Extract files from a tar archive. /// /// # Arguments /// -/// * `archive_path` - Path to the tar archive to extract -/// * `verbose` - Whether to print verbose output during extraction +/// * `input` - Readable source of the archive data +/// * `archive_path` - Path used for error messages and verbose header +/// * `verbose` - Print each extracted path to stdout +/// * `compression` - Compression mode to use when reading +/// * `backup_control` - Whether and how to back up existing files before overwriting +/// * `backup_suffix` - Suffix used for simple backups (default: `~`) /// /// # Errors /// -/// Returns an error if: -/// - The archive file cannot be opened -/// - The archive format is invalid -/// - Files cannot be extracted due to I/O or permission errors +/// Returns an error if the archive cannot be read or entries cannot be extracted. pub fn extract_archive( input: impl Read, archive_path: &Path, verbose: bool, compression: CompressionMode, + backup_control: BackupControl, + backup_suffix: &str, ) -> UResult<()> { let reader = open_archive_reader(input, compression)?; let mut archive = Archive::new(reader); let mut out = BufWriter::new(io::stdout().lock()); - // Extract to current directory if verbose { writeln!(out, "Extracting archive: {}", archive_path.display()).map_err(TarError::Io)?; } - // Iterate through entries for verbose output and error handling 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(TarError::CannotReadEntryPath)? @@ -54,7 +96,8 @@ pub fn extract_archive( writeln!(out, "{}", path.display()).map_err(TarError::Io)?; } - // Unpack the entry + backup_file(&path, backup_control, backup_suffix)?; + entry.unpack_in(".").map_err(|e| TarError::CannotExtract { path: path.clone(), source: e, diff --git a/src/uu/tar/src/operations/extract_tests.rs b/src/uu/tar/src/operations/extract_tests.rs index 898caff..360c2d5 100644 --- a/src/uu/tar/src/operations/extract_tests.rs +++ b/src/uu/tar/src/operations/extract_tests.rs @@ -4,38 +4,265 @@ // file that was distributed with this source code. use super::*; -use crate::CompressionMode; +use crate::{BackupControl, CompressionMode}; use std::fs; use tar::Builder; use tempfile::tempdir; -#[test] -fn test_extract_archive_with_zstd() { - let tempdir = tempdir().unwrap(); - let archive_path = tempdir.path().join("archive.tar.zst"); - +fn make_zstd_tar(archive_path: &std::path::Path, entries: &[(&str, &str)]) { let mut tar_bytes = Vec::new(); { let mut builder = Builder::new(&mut tar_bytes); - let mut header = tar::Header::new_gnu(); - header.set_mode(0o644); - header.set_size("hello".len() as u64); - header.set_cksum(); - builder - .append_data(&mut header, "extracted.txt", std::io::Cursor::new("hello")) - .unwrap(); + for (name, content) in entries { + let mut header = tar::Header::new_gnu(); + header.set_mode(0o644); + header.set_size(content.len() as u64); + header.set_cksum(); + builder + .append_data(&mut header, name, std::io::Cursor::new(*content)) + .unwrap(); + } builder.finish().unwrap(); } let compressed = zstd::stream::encode_all(std::io::Cursor::new(tar_bytes), 0).unwrap(); - fs::write(&archive_path, compressed).unwrap(); + fs::write(archive_path, compressed).unwrap(); +} + +#[test] +fn test_extract_archive_with_zstd() { + let tempdir = tempdir().unwrap(); + let archive_path = tempdir.path().join("archive.tar.zst"); + make_zstd_tar(&archive_path, &[("extracted.txt", "hello")]); let _guard = crate::operations::TestDirGuard::enter(tempdir.path()); let input = fs::File::open(&archive_path).unwrap(); - let result = extract_archive(input, &archive_path, true, CompressionMode::Zstd); + extract_archive( + input, + &archive_path, + true, + CompressionMode::Zstd, + BackupControl::None, + "~", + ) + .unwrap(); - result.unwrap(); assert_eq!( fs::read_to_string(tempdir.path().join("extracted.txt")).unwrap(), "hello" ); } + +// --- backup: None (default, file overwritten) --- + +#[test] +fn test_backup_none_overwrites_existing() { + let tempdir = tempdir().unwrap(); + let archive_path = tempdir.path().join("archive.tar.zst"); + make_zstd_tar(&archive_path, &[("file.txt", "new")]); + fs::write(tempdir.path().join("file.txt"), "old").unwrap(); + + let _guard = crate::operations::TestDirGuard::enter(tempdir.path()); + let input = fs::File::open(&archive_path).unwrap(); + extract_archive( + input, + &archive_path, + false, + CompressionMode::Zstd, + BackupControl::None, + "~", + ) + .unwrap(); + + assert_eq!( + fs::read_to_string(tempdir.path().join("file.txt")).unwrap(), + "new" + ); + assert!(!tempdir.path().join("file.txt~").exists()); +} + +// --- backup: Simple --- + +#[test] +fn test_backup_simple_renames_existing() { + let tempdir = tempdir().unwrap(); + let archive_path = tempdir.path().join("archive.tar.zst"); + make_zstd_tar(&archive_path, &[("file.txt", "new")]); + fs::write(tempdir.path().join("file.txt"), "old").unwrap(); + + let _guard = crate::operations::TestDirGuard::enter(tempdir.path()); + let input = fs::File::open(&archive_path).unwrap(); + extract_archive( + input, + &archive_path, + false, + CompressionMode::Zstd, + BackupControl::Simple, + "~", + ) + .unwrap(); + + assert_eq!( + fs::read_to_string(tempdir.path().join("file.txt")).unwrap(), + "new" + ); + assert_eq!( + fs::read_to_string(tempdir.path().join("file.txt~")).unwrap(), + "old" + ); +} + +#[test] +fn test_backup_simple_custom_suffix() { + let tempdir = tempdir().unwrap(); + let archive_path = tempdir.path().join("archive.tar.zst"); + make_zstd_tar(&archive_path, &[("file.txt", "new")]); + fs::write(tempdir.path().join("file.txt"), "old").unwrap(); + + let _guard = crate::operations::TestDirGuard::enter(tempdir.path()); + let input = fs::File::open(&archive_path).unwrap(); + extract_archive( + input, + &archive_path, + false, + CompressionMode::Zstd, + BackupControl::Simple, + ".bak", + ) + .unwrap(); + + assert!(tempdir.path().join("file.txt.bak").exists()); +} + +#[test] +fn test_backup_simple_no_op_when_no_existing_file() { + let tempdir = tempdir().unwrap(); + let archive_path = tempdir.path().join("archive.tar.zst"); + make_zstd_tar(&archive_path, &[("file.txt", "new")]); + + let _guard = crate::operations::TestDirGuard::enter(tempdir.path()); + let input = fs::File::open(&archive_path).unwrap(); + extract_archive( + input, + &archive_path, + false, + CompressionMode::Zstd, + BackupControl::Simple, + "~", + ) + .unwrap(); + + assert!(tempdir.path().join("file.txt").exists()); + assert!(!tempdir.path().join("file.txt~").exists()); +} + +// --- backup: Numbered --- + +#[test] +fn test_backup_numbered_first_backup() { + let tempdir = tempdir().unwrap(); + let archive_path = tempdir.path().join("archive.tar.zst"); + make_zstd_tar(&archive_path, &[("file.txt", "new")]); + fs::write(tempdir.path().join("file.txt"), "old").unwrap(); + + let _guard = crate::operations::TestDirGuard::enter(tempdir.path()); + let input = fs::File::open(&archive_path).unwrap(); + extract_archive( + input, + &archive_path, + false, + CompressionMode::Zstd, + BackupControl::Numbered, + "~", + ) + .unwrap(); + + assert_eq!( + fs::read_to_string(tempdir.path().join("file.txt")).unwrap(), + "new" + ); + assert_eq!( + fs::read_to_string(tempdir.path().join("file.txt.~1~")).unwrap(), + "old" + ); +} + +#[test] +fn test_backup_numbered_increments() { + let tempdir = tempdir().unwrap(); + let archive_path = tempdir.path().join("archive.tar.zst"); + fs::write(tempdir.path().join("file.txt"), "v1").unwrap(); + fs::write(tempdir.path().join("file.txt.~1~"), "v0").unwrap(); + make_zstd_tar(&archive_path, &[("file.txt", "v2")]); + + let _guard = crate::operations::TestDirGuard::enter(tempdir.path()); + let input = fs::File::open(&archive_path).unwrap(); + extract_archive( + input, + &archive_path, + false, + CompressionMode::Zstd, + BackupControl::Numbered, + "~", + ) + .unwrap(); + + assert_eq!( + fs::read_to_string(tempdir.path().join("file.txt.~2~")).unwrap(), + "v1" + ); + assert_eq!( + fs::read_to_string(tempdir.path().join("file.txt.~1~")).unwrap(), + "v0" + ); +} + +// --- backup: Existing --- + +#[test] +fn test_backup_existing_uses_simple_when_no_numbered_backup() { + let tempdir = tempdir().unwrap(); + let archive_path = tempdir.path().join("archive.tar.zst"); + make_zstd_tar(&archive_path, &[("file.txt", "new")]); + fs::write(tempdir.path().join("file.txt"), "old").unwrap(); + + let _guard = crate::operations::TestDirGuard::enter(tempdir.path()); + let input = fs::File::open(&archive_path).unwrap(); + extract_archive( + input, + &archive_path, + false, + CompressionMode::Zstd, + BackupControl::Existing, + "~", + ) + .unwrap(); + + // no .~1~ existed → simple backup + assert!(tempdir.path().join("file.txt~").exists()); + assert!(!tempdir.path().join("file.txt.~1~").exists()); +} + +#[test] +fn test_backup_existing_uses_numbered_when_numbered_backup_present() { + let tempdir = tempdir().unwrap(); + let archive_path = tempdir.path().join("archive.tar.zst"); + make_zstd_tar(&archive_path, &[("file.txt", "new")]); + fs::write(tempdir.path().join("file.txt"), "old").unwrap(); + fs::write(tempdir.path().join("file.txt.~1~"), "older").unwrap(); + + let _guard = crate::operations::TestDirGuard::enter(tempdir.path()); + let input = fs::File::open(&archive_path).unwrap(); + extract_archive( + input, + &archive_path, + false, + CompressionMode::Zstd, + BackupControl::Existing, + "~", + ) + .unwrap(); + + // .~1~ existed → numbered mode → backup goes to .~2~ + assert!(tempdir.path().join("file.txt.~2~").exists()); + assert!(!tempdir.path().join("file.txt~").exists()); +} diff --git a/src/uu/tar/src/tar.rs b/src/uu/tar/src/tar.rs index 60ce5a5..18a640b 100644 --- a/src/uu/tar/src/tar.rs +++ b/src/uu/tar/src/tar.rs @@ -8,7 +8,7 @@ pub mod errors; pub mod operations; use crate::errors::TarError; -use clap::{arg, crate_version, ArgAction, Command}; +use clap::{arg, crate_version, Arg, ArgAction, Command}; use std::fs::File; use std::io::{self, IsTerminal}; use std::path::{Path, PathBuf}; @@ -26,6 +26,26 @@ pub enum CompressionMode { Zstd, } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum BackupControl { + None, + Simple, + Numbered, + Existing, +} + +fn parse_backup_control(s: &str) -> Result { + match s { + "none" | "off" => Ok(BackupControl::None), + "simple" | "never" => Ok(BackupControl::Simple), + "numbered" | "t" => Ok(BackupControl::Numbered), + "existing" | "nil" => Ok(BackupControl::Existing), + other => Err(format!( + "invalid backup type '{other}': must be one of none, off, simple, never, numbered, t, existing, nil" + )), + } +} + /// Determines whether a string looks like a POSIX tar keystring. /// /// A valid keystring must not start with '-', must contain at least one @@ -152,6 +172,14 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } else { None }; + let backup_control = matches + .get_one::("backup") + .copied() + .unwrap_or(BackupControl::None); + let backup_suffix = matches + .get_one::("suffix") + .cloned() + .unwrap_or_else(|| "~".to_string()); // Handle extract operation if matches.get_flag("extract") { @@ -161,11 +189,25 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let compression = explicit_compression.unwrap_or(CompressionMode::Auto); return if archive_path == Path::new("-") { - operations::extract::extract_archive(io::stdin(), archive_path, verbose, compression) + operations::extract::extract_archive( + io::stdin(), + archive_path, + verbose, + compression, + backup_control, + &backup_suffix, + ) } else { let file = File::open(archive_path).map_err(|e| TarError::from_io_error(e, archive_path))?; - operations::extract::extract_archive(file, archive_path, verbose, compression) + operations::extract::extract_archive( + file, + archive_path, + verbose, + compression, + backup_control, + &backup_suffix, + ) }; } @@ -278,6 +320,20 @@ pub fn uu_app() -> Command { arg!(-v --verbose "Verbosely list files processed"), // arg!(-h --dereference "Follow symlinks"), // arg!(-p --"preserve-permissions" "Extract information about file permissions"), + // Backup options (extract only) + Arg::new("backup") + .long("backup") + .value_name("CONTROL") + .num_args(0..=1) + .require_equals(true) + .default_missing_value("existing") + .value_parser(parse_backup_control) + .help( + "back up existing files before overwriting; \ + CONTROL may be: none/off, simple/never, numbered/t, existing/nil \ + (default: existing)", + ), + arg!(--suffix "override the usual backup suffix ('~')"), // Help arg!(--help "Print help information").action(ArgAction::Help), // Files to process