Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions benches/bench_tar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ fn extract_archive_20_files(bencher: divan::Bencher) {
input,
&archive_path,
false,
false,
CompressionMode::None,
)
.unwrap();
Expand Down
29 changes: 28 additions & 1 deletion src/uu/tar/src/operations/extract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ use uucore::error::UResult;
///
/// * `archive_path` - Path to the tar archive to extract
/// * `verbose` - Whether to print verbose output during extraction
/// * `to_stdout` - Write the contents of regular files to standard output instead
/// of creating them; verbose output then goes to standard error
///
/// # Errors
///
Expand All @@ -29,11 +31,19 @@ pub fn extract_archive(
input: impl Read,
archive_path: &Path,
verbose: bool,
to_stdout: bool,
compression: CompressionMode,
) -> UResult<()> {
let reader = open_archive_reader(input, compression)?;
let mut archive = Archive::new(reader);
let mut out = BufWriter::new(io::stdout().lock());
// With --to-stdout the member contents own stdout, so status messages go to stderr.
let status: Box<dyn Write> = if to_stdout {
Box::new(io::stderr())
} else {
Box::new(io::stdout().lock())
};
let mut out = BufWriter::new(status);
let mut data = to_stdout.then(|| BufWriter::new(io::stdout().lock()));

// Extract to current directory
if verbose {
Expand All @@ -58,6 +68,20 @@ pub fn extract_archive(
writeln!(out, "{}", path.display()).map_err(TarError::Io)?;
}

if let Some(data) = data.as_mut() {
// Like GNU tar, only regular files have contents to write.
if entry.header().entry_type().is_file() {
out.flush().map_err(TarError::Io)?;
io::copy(&mut entry, data).map_err(|e| TarError::CannotExtract {
path: path.clone(),
source: e,
})?;
// Keep each member's contents next to its name under -v.
data.flush().map_err(TarError::Io)?;
}
continue;
}

// Unpack the entry. `unpack_in` skips, and returns false for, a member whose
// name contains a `..` component, as it would escape the target directory.
let unpacked = entry.unpack_in(".").map_err(|e| TarError::CannotExtract {
Expand All @@ -72,6 +96,9 @@ pub fn extract_archive(
}

out.flush().map_err(TarError::Io)?;
if let Some(data) = data.as_mut() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we still need this? data is already flushed after each member

data.flush().map_err(TarError::Io)?;
}
if refused_member {
return finish_after_previous_errors();
}
Expand Down
2 changes: 1 addition & 1 deletion src/uu/tar/src/operations/extract_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ fn test_extract_archive_with_zstd() {

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);
let result = extract_archive(input, &archive_path, true, false, CompressionMode::Zstd);

result.unwrap();
assert_eq!(
Expand Down
18 changes: 16 additions & 2 deletions src/uu/tar/src/tar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {

let verbose = matches.get_flag("verbose");
let allow_absolute = matches.get_flag("absolute-names");
let to_stdout = matches.get_flag("to-stdout");
let recursive = !matches.get_flag("no-recursion");
let explicit_compression = if matches.get_flag("gzip") {
Some(CompressionMode::Gzip)
Expand All @@ -162,11 +163,23 @@ 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,
to_stdout,
compression,
)
} 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,
to_stdout,
compression,
)
};
}

Expand Down Expand Up @@ -279,6 +292,7 @@ pub fn uu_app() -> Command {
arg!(--zstd "Filter through zstd").conflicts_with("gzip"),
// Common options
arg!(-v --verbose "Verbosely list files processed"),
arg!(-O --"to-stdout" "Extract files to standard output"),
arg!(--"no-recursion" "Avoid descending automatically in directories"),
// arg!(-h --dereference "Follow symlinks"),
// arg!(-p --"preserve-permissions" "Extract information about file permissions"),
Expand Down
47 changes: 46 additions & 1 deletion tests/by-util/test_tar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ use std::io::{Cursor, Read, Write};
use std::path::{self, PathBuf};

use tar_rs_crate::{Archive as TarRsArchive, Builder as TarRsBuilder, Header as TarRsHeader};
use uutests::{at_and_ucmd, new_ucmd};
use uutests::util::TestScenario;
use uutests::{at_and_ucmd, new_ucmd, util_name};

/// Size of a single tar block in bytes (per POSIX specification).
// TODO: This should be exported from the tar crate instead of being redefined here.
Expand Down Expand Up @@ -86,6 +87,50 @@ fn test_create_dir_verbose() {
.stdout_contains(file2_path);
}

#[test]
fn test_extract_to_stdout() {
let ts = TestScenario::new(util_name!());
let at = &ts.fixtures;

at.mkdir("dir");
at.write("dir/a.txt", "first\n");
at.write("dir/b.txt", "second\n");
ts.ucmd()
.args(&["-cf", "archive.tar", "dir/a.txt", "dir/b.txt"])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please archive dir itself too and check that it isn't created with -O
this way the "only regular files" path is tested

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added: it archives a directory, extracts it with -O and checks the directory isn't created.

.succeeds();
at.remove("dir/a.txt");
at.remove("dir/b.txt");

ts.ucmd()
.args(&["-xOf", "archive.tar"])
.succeeds()
.stdout_only("first\nsecond\n");
// The contents went to stdout, so nothing was written to disk.
assert!(!at.file_exists("dir/a.txt"));
assert!(!at.file_exists("dir/b.txt"));

// With -v, the member names go to stderr so they don't mix with the contents
ts.ucmd()
.args(&["-xvf", "archive.tar", "--to-stdout"])
.succeeds()
.stdout_is("first\nsecond\n")
.stderr_contains("dir/a.txt")
.stderr_contains("dir/b.txt");
assert!(!at.file_exists("dir/a.txt"));

// A directory member has no contents, and -O doesn't create it either
at.mkdir("only");
at.write("only/c.txt", "third\n");
ts.ucmd().args(&["-cf", "dir.tar", "only"]).succeeds();
at.remove("only/c.txt");
at.rmdir("only");
ts.ucmd()
.args(&["-xOf", "dir.tar"])
.succeeds()
.stdout_only("third\n");
assert!(!at.dir_exists("only"));
}

#[test]
fn test_create_no_recursion() {
let (at, mut ucmd) = at_and_ucmd!();
Expand Down
Loading