Merge pull request #156 from kaladron/dashfdash

tar: support -f - for stdin/stdout archives

Cargo.lock

@@ -1643,6 +1643,7 @@ dependencies = [
  "clap",
  "regex",
  "tar",
+ "tempfile",
  "thiserror",
  "uucore",
 ]

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<PathBuf> {
 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, true, 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, true, 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, true, 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()], true, 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();
 }

src/uu/tar/Cargo.toml

@@ -26,3 +26,6 @@ path = "src/tar.rs"
 [[bin]]
 name = "tar"
 path = "src/main.rs"
+
+[dev-dependencies]
+tempfile = { workspace = true }

src/uu/tar/src/errors.rs

@@ -54,6 +54,10 @@ pub enum TarError {
     /// Cannot finalize the archive
     #[error("tar: Cannot finalize archive: {0}")]
     CannotFinalizeArchive(io::Error),
+
+    /// Refusing to write archive contents to terminal
+    #[error("tar: Refusing to write archive contents to terminal (missing -f option?)")]
+    RefuseWriteArchiveToTerminal,
 }
 
 impl TarError {

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::<Vec<_>>()
                 .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,8 +77,8 @@ pub fn create_archive(
                     .iter()
                     .collect();
                 writeln!(
-                    out,
-                    "Removing leading `{}' from member names",
+                    std::io::stderr(),
+                    "tar: Removing leading `{}' from member names",
                     removed.display()
                 )
                 .map_err(TarError::Io)?;
@@ -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(())
 }
@@ -148,3 +146,32 @@ fn normalize_path(path: &Path, allow_absolute: bool) -> Option<PathBuf> {
         None
     }
 }
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::io::{self, Write};
+    use tempfile::TempDir;
+
+    struct FailFlushWriter;
+    impl Write for FailFlushWriter {
+        fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
+            Ok(buf.len())
+        }
+        fn flush(&mut self) -> io::Result<()> {
+            Err(io::Error::other("flush failed"))
+        }
+    }
+
+    #[test]
+    fn test_create_archive_flush_failed() {
+        let dir = TempDir::new().unwrap();
+        let file_path = dir.path().join("test.txt");
+        fs::write(&file_path, "hello").unwrap();
+
+        let output = FailFlushWriter;
+        let status_output = io::sink();
+
+        let res = create_archive(output, status_output, &[file_path.as_path()], false, false);
+        assert!(res.is_err());
+    }
+}

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

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)? {

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::{self, IsTerminal};
 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,35 @@ 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 {
+            if io::stdout().is_terminal() {
+                Err(TarError::RefuseWriteArchiveToTerminal.into())
+            } else {
+                let output = io::stdout().lock();
+                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().lock();
+            operations::create::create_archive(
+                output,
+                status_output,
+                &files,
+                allow_absolute,
+                verbose,
+            )
+        };
     }
 
     // Handle list operation
@@ -169,7 +206,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

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!()
@@ -179,7 +190,7 @@ fn test_create_absolute_path() {
         &file_abs_path.display().to_string(),
     ])
     .succeeds()
-    .stdout_contains("Removing leading");
+    .stderr_contains("tar: Removing leading");
 
     assert!(at.file_exists("archive-trimed.tar"));
 
@@ -814,3 +825,115 @@ 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");
+    assert_eq!(
+        bytes[8], 0,
+        "stdout should not contain verbose output prefix"
+    );
+}
+
+#[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");
+}