From 3be98e9e5cec4fc321e3150e19a6ea81c812dc3e Mon Sep 17 00:00:00 2001 From: Jeff Bailey Date: Sat, 4 Apr 2026 23:27:21 +0000 Subject: [PATCH] tar: support -f - for stdin/stdout archives When the archive path is "-", read archives from stdin for list and extract, and write archive data to stdout for create. Route create status output to stderr when writing the archive to stdout so verbose output does not corrupt the stream. Add CLI tests for create-to-stdout, list-from-stdin, extract-from-stdin, and the stderr routing cases. --- benches/bench_tar.rs | 28 +++++-- src/uu/tar/src/operations/create.rs | 28 +++---- src/uu/tar/src/operations/extract.rs | 10 +-- src/uu/tar/src/operations/list.rs | 10 +-- src/uu/tar/src/tar.rs | 45 +++++++++- tests/by-util/test_tar.rs | 119 +++++++++++++++++++++++++++ 6 files changed, 200 insertions(+), 40 deletions(-) diff --git a/benches/bench_tar.rs b/benches/bench_tar.rs index cb11a1c..da0830c 100644 --- a/benches/bench_tar.rs +++ b/benches/bench_tar.rs @@ -4,7 +4,7 @@ // at various scales to track performance over time. use std::fs::{self, File}; -use std::io::Write; +use std::io::{self, Write}; use std::path::{Path, PathBuf}; use tar::operations; use tempfile::TempDir; @@ -39,7 +39,9 @@ fn collect_files(dir: &Path) -> Vec { fn build_archive(archive_path: &Path, source_dir: &Path) { let files = collect_files(source_dir); let refs: Vec<&Path> = files.iter().map(|p| p.as_path()).collect(); - operations::create::create_archive(archive_path, &refs, false, false).unwrap(); + let output = File::create(archive_path).unwrap(); + let status_output = io::sink(); + operations::create::create_archive(output, status_output, &refs, false, false).unwrap(); } // --------------------------------------------------------------------------- @@ -57,7 +59,9 @@ fn create_archive_10_files(bencher: divan::Bencher) { bencher.bench_local(|| { let refs: Vec<&Path> = files.iter().map(|p| p.as_path()).collect(); - operations::create::create_archive(&archive_path, &refs, false, false).unwrap(); + let output = File::create(&archive_path).unwrap(); + let status_output = io::sink(); + operations::create::create_archive(output, status_output, &refs, false, false).unwrap(); }); } @@ -72,7 +76,9 @@ fn create_archive_100_files(bencher: divan::Bencher) { bencher.bench_local(|| { let refs: Vec<&Path> = files.iter().map(|p| p.as_path()).collect(); - operations::create::create_archive(&archive_path, &refs, false, false).unwrap(); + let output = File::create(&archive_path).unwrap(); + let status_output = io::sink(); + operations::create::create_archive(output, status_output, &refs, false, false).unwrap(); }); } @@ -87,7 +93,10 @@ fn create_archive_directory(bencher: divan::Bencher) { let archive_path = out.path().join("bench.tar"); bencher.bench_local(|| { - operations::create::create_archive(&archive_path, &[sub.as_path()], false, false).unwrap(); + let output = File::create(&archive_path).unwrap(); + let status_output = io::sink(); + operations::create::create_archive(output, status_output, &[sub.as_path()], false, false) + .unwrap(); }); } @@ -104,7 +113,8 @@ fn list_archive_50_files(bencher: divan::Bencher) { build_archive(&archive_path, source.path()); bencher.bench_local(|| { - operations::list::list_archive(&archive_path, false).unwrap(); + let input = File::open(&archive_path).unwrap(); + operations::list::list_archive(input, false).unwrap(); }); } @@ -117,7 +127,8 @@ fn list_archive_verbose_50_files(bencher: divan::Bencher) { build_archive(&archive_path, source.path()); bencher.bench_local(|| { - operations::list::list_archive(&archive_path, true).unwrap(); + let input = File::open(&archive_path).unwrap(); + operations::list::list_archive(input, true).unwrap(); }); } @@ -138,7 +149,8 @@ fn extract_archive_20_files(bencher: divan::Bencher) { .with_inputs(|| TempDir::new().unwrap()) .bench_local_values(|extract_dir| { std::env::set_current_dir(extract_dir.path()).unwrap(); - operations::extract::extract_archive(&archive_path, false).unwrap(); + let input = File::open(&archive_path).unwrap(); + operations::extract::extract_archive(input, &archive_path, false).unwrap(); }); std::env::set_current_dir(original_dir).unwrap(); } diff --git a/src/uu/tar/src/operations/create.rs b/src/uu/tar/src/operations/create.rs index 39b6d63..bcdbfee 100644 --- a/src/uu/tar/src/operations/create.rs +++ b/src/uu/tar/src/operations/create.rs @@ -5,8 +5,8 @@ use crate::errors::TarError; use std::collections::VecDeque; -use std::fs::{self, File}; -use std::io::{self, BufWriter, Write}; +use std::fs; +use std::io::{BufWriter, Write}; use std::path::Component::{self, ParentDir, Prefix, RootDir}; use std::path::{self, Path, PathBuf}; use tar::Builder; @@ -28,23 +28,19 @@ use uucore::error::UResult; /// - Any input file cannot be read /// - Files cannot be added due to I/O or permission errors pub fn create_archive( - archive_path: &Path, + output: impl Write, + status_output: impl Write, files: &[&Path], allow_absolute: bool, verbose: bool, ) -> UResult<()> { - // Create the output file - let file = File::create(archive_path).map_err(|e| TarError::CannotCreateArchive { - path: archive_path.to_path_buf(), - source: e, - })?; + let mut output = BufWriter::new(output); + let mut status_output = BufWriter::new(status_output); // Create Builder instance - let mut builder = Builder::new(file); + let mut builder = Builder::new(&mut output); builder.preserve_absolute(allow_absolute); - let mut out = BufWriter::new(io::stdout().lock()); - // Add each file or directory to the archive for &path in files { // Check if path exists @@ -68,7 +64,7 @@ pub fn create_archive( }) .collect::>() .join("\n"); - writeln!(out, "{to_print}").map_err(TarError::Io)?; + writeln!(status_output, "{to_print}").map_err(TarError::Io)?; } // Normalize path if needed (so far, handles only absolute paths) @@ -81,7 +77,7 @@ pub fn create_archive( .iter() .collect(); writeln!( - out, + status_output, "Removing leading `{}' from member names", removed.display() ) @@ -112,9 +108,11 @@ pub fn create_archive( } } - // Finish writing the archive - out.flush().map_err(TarError::Io)?; builder.finish().map_err(TarError::CannotFinalizeArchive)?; + drop(builder); + + status_output.flush().map_err(TarError::Io)?; + output.flush().map_err(TarError::Io)?; Ok(()) } diff --git a/src/uu/tar/src/operations/extract.rs b/src/uu/tar/src/operations/extract.rs index 135b66f..47cd1dc 100644 --- a/src/uu/tar/src/operations/extract.rs +++ b/src/uu/tar/src/operations/extract.rs @@ -4,8 +4,7 @@ // file that was distributed with this source code. use crate::errors::TarError; -use std::fs::File; -use std::io::{self, BufWriter, Write}; +use std::io::{self, BufReader, BufWriter, Read, Write}; use std::path::Path; use tar::Archive; use uucore::error::UResult; @@ -23,12 +22,9 @@ use uucore::error::UResult; /// - The archive file cannot be opened /// - The archive format is invalid /// - Files cannot be extracted due to I/O or permission errors -pub fn extract_archive(archive_path: &Path, verbose: bool) -> UResult<()> { - // Open the archive file - let file = File::open(archive_path).map_err(|e| TarError::from_io_error(e, archive_path))?; - +pub fn extract_archive(input: impl Read, archive_path: &Path, verbose: bool) -> UResult<()> { // Create Archive instance - let mut archive = Archive::new(file); + let mut archive = Archive::new(BufReader::new(input)); let mut out = BufWriter::new(io::stdout().lock()); // Extract to current directory diff --git a/src/uu/tar/src/operations/list.rs b/src/uu/tar/src/operations/list.rs index c424ca8..0bd98e2 100644 --- a/src/uu/tar/src/operations/list.rs +++ b/src/uu/tar/src/operations/list.rs @@ -5,18 +5,14 @@ use crate::errors::TarError; use chrono::{TimeZone, Utc}; -use std::fs::File; -use std::io::{self, BufWriter, Write}; -use std::path::Path; +use std::io::{self, BufReader, BufWriter, Read, Write}; use tar::Archive; use uucore::error::UResult; use uucore::fs::display_permissions_unix; /// List the contents of a tar archive, printing one entry per line. -pub fn list_archive(archive_path: &Path, verbose: bool) -> UResult<()> { - let file: File = - File::open(archive_path).map_err(|e| TarError::from_io_error(e, archive_path))?; - let mut archive = Archive::new(file); +pub fn list_archive(input: impl Read, verbose: bool) -> UResult<()> { + let mut archive = Archive::new(BufReader::new(input)); let mut out = BufWriter::new(io::stdout().lock()); for entry_result in archive.entries().map_err(TarError::CannotReadEntries)? { diff --git a/src/uu/tar/src/tar.rs b/src/uu/tar/src/tar.rs index ff6800f..1d97510 100644 --- a/src/uu/tar/src/tar.rs +++ b/src/uu/tar/src/tar.rs @@ -6,7 +6,10 @@ pub mod errors; pub mod operations; +use crate::errors::TarError; use clap::{arg, crate_version, ArgAction, Command}; +use std::fs::File; +use std::io; use std::path::{Path, PathBuf}; use uucore::error::UResult; use uucore::format_usage; @@ -139,7 +142,13 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { uucore::error::USimpleError::new(64, "option requires an argument -- 'f'") })?; - return operations::extract::extract_archive(archive_path, verbose); + return if archive_path == Path::new("-") { + operations::extract::extract_archive(io::stdin(), archive_path, verbose) + } 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) + }; } // Handle create operation @@ -160,7 +169,31 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { )); } - return operations::create::create_archive(archive_path, &files, allow_absolute, verbose); + let output_is_stdout = archive_path == Path::new("-"); + return if output_is_stdout { + let output = io::stdout(); + let status_output = io::stderr(); + operations::create::create_archive( + output, + status_output, + &files, + allow_absolute, + verbose, + ) + } else { + let output = File::create(archive_path).map_err(|e| TarError::CannotCreateArchive { + path: archive_path.clone(), + source: e, + })?; + let status_output = io::stdout(); + operations::create::create_archive( + output, + status_output, + &files, + allow_absolute, + verbose, + ) + }; } // Handle list operation @@ -169,7 +202,13 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { uucore::error::USimpleError::new(64, "option requires an argument -- 'f'") })?; - return operations::list::list_archive(archive_path, verbose); + return if archive_path == Path::new("-") { + operations::list::list_archive(io::stdin(), verbose) + } else { + let file = + File::open(archive_path).map_err(|e| TarError::from_io_error(e, archive_path))?; + operations::list::list_archive(file, verbose) + }; } // If no operation specified, show error diff --git a/tests/by-util/test_tar.rs b/tests/by-util/test_tar.rs index 3fb603f..80196e6 100644 --- a/tests/by-util/test_tar.rs +++ b/tests/by-util/test_tar.rs @@ -145,6 +145,17 @@ fn test_create_verbose() { assert!(at.file_exists("archive.tar")); } +#[test] +fn test_create_verbose_to_stdout_uses_stderr() { + let (at, mut ucmd) = at_and_ucmd!(); + + at.write("file.txt", "hello"); + + ucmd.args(&["-cvf", "-", "file.txt"]) + .succeeds() + .stderr_contains("file.txt"); +} + #[test] fn test_create_empty_archive_fails() { new_ucmd!() @@ -814,3 +825,111 @@ fn test_list_conflicts_with_extract() { .code_is(2) .stderr_contains("cannot be used with"); } + +// stdin/stdout (-f -) tests + +#[test] +fn test_create_to_stdout() { + let (at, mut ucmd) = at_and_ucmd!(); + at.write("file.txt", "hello"); + + let result = ucmd.args(&["-cf", "-", "file.txt"]).succeeds(); + let bytes = result.stdout(); + // A tar archive is at least two blocks: one header + two end-of-archive blocks + assert!( + bytes.len() >= TAR_BLOCK_SIZE, + "stdout should contain tar data (got {} bytes)", + bytes.len() + ); + // The first 100 bytes of a tar header are the file name + let header_prefix = std::str::from_utf8(&bytes[..100]).unwrap_or(""); + assert!( + header_prefix.contains("file.txt"), + "tar header should contain filename" + ); +} + +#[test] +fn test_create_verbose_to_stdout_keeps_stdout_as_tar_data() { + let (at, mut ucmd) = at_and_ucmd!(); + at.write("file.txt", "hello"); + + let result = ucmd.args(&["-cvf", "-", "file.txt"]).succeeds(); + let bytes = result.stdout(); + let header_prefix = std::str::from_utf8(&bytes[..100]).unwrap_or(""); + + assert!( + bytes.len() >= TAR_BLOCK_SIZE, + "stdout should contain tar data (got {} bytes)", + bytes.len() + ); + assert!( + header_prefix.contains("file.txt"), + "tar header should contain filename" + ); + assert_eq!(&bytes[..8], b"file.txt"); +} + +#[test] +fn test_create_absolute_path_to_stdout_uses_stderr_for_normalization_notice() { + let (at, mut ucmd) = at_and_ucmd!(); + + let mut file_abs_path = PathBuf::from(at.root_dir_resolved()); + file_abs_path.push("file1.txt"); + + at.write(&file_abs_path.display().to_string(), "content1"); + + let result = ucmd + .args(&["-cf", "-", &file_abs_path.display().to_string()]) + .succeeds(); + result.stderr_contains("Removing leading"); + let bytes = result.stdout(); + let header_prefix = std::str::from_utf8(&bytes[..100]).unwrap_or(""); + + assert!( + bytes.len() >= TAR_BLOCK_SIZE, + "stdout should contain tar data (got {} bytes)", + bytes.len() + ); + assert!( + !header_prefix.contains("Removing leading"), + "stdout should not contain normalization notices" + ); +} + +#[test] +fn test_list_from_stdin() { + let (at, mut ucmd) = at_and_ucmd!(); + at.write("file.txt", "hello"); + + // Create archive to a file in the same sandbox + new_ucmd!() + .args(&["-cf", &at.plus_as_string("archive.tar"), "file.txt"]) + .current_dir(at.as_string()) + .succeeds(); + + let archive_bytes = at.read_bytes("archive.tar"); + ucmd.args(&["-tf", "-"]) + .pipe_in(archive_bytes) + .succeeds() + .stdout_contains("file.txt"); +} + +#[test] +fn test_extract_from_stdin() { + let (at, mut ucmd) = at_and_ucmd!(); + at.write("file.txt", "hello world"); + + // Create archive to a file in the same sandbox + new_ucmd!() + .args(&["-cf", &at.plus_as_string("archive.tar"), "file.txt"]) + .current_dir(at.as_string()) + .succeeds(); + + at.remove("file.txt"); + + let archive_bytes = at.read_bytes("archive.tar"); + ucmd.args(&["-xf", "-"]).pipe_in(archive_bytes).succeeds(); + + assert_eq!(at.read("file.txt"), "hello world"); +}