From 94bd3a6ebd9e8579c0b50f37e4c4ca2cd46d0821 Mon Sep 17 00:00:00 2001 From: valoq Date: Sat, 25 Apr 2026 13:55:27 +0200 Subject: [PATCH 01/13] fix decompression --- src/commands/decompress.rs | 8 ++++++-- tests/integration.rs | 22 +++++++++++----------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index f87684600..6b379b77c 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -7,7 +7,7 @@ use std::{ use fs_err::{self as fs, PathExt}; use crate::{ - BUFFER_CAPACITY, QuestionAction, QuestionPolicy, Result, + BUFFER_CAPACITY, INITIAL_CURRENT_DIR, QuestionAction, QuestionPolicy, Result, commands::{warn_user_about_loading_sevenz_in_memory, warn_user_about_loading_zip_in_memory}, extension::{ CompressionFormat::{self, *}, @@ -215,13 +215,17 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { /// directory or replace it if it already exists. The `output_dir` needs to be empty /// - If `output_dir` does not exist OR is a empty directory, it will unpack there /// - If `output_dir` exist OR is a directory not empty, the user will be asked what to do +/// - If `output_dir` is the current working directory, files are extracted directly without prompting fn unpack_archive( unpack_fn: impl FnOnce(&Path) -> Result, output_dir: &Path, question_policy: QuestionPolicy, ) -> Result> { + // Extracting into the CWD is a merge into the user's workspace and should not prompt, + // matching the behaviour of `tar xf` and `unzip` when no destination is given. + let is_cwd = output_dir == *INITIAL_CURRENT_DIR; let is_valid_output_dir = - !output_dir.fs_err_try_exists()? || (output_dir.is_dir() && output_dir.read_dir()?.next().is_none()); + is_cwd || !output_dir.fs_err_try_exists()? || (output_dir.is_dir() && output_dir.read_dir()?.next().is_none()); let output_dir_cleaned = if is_valid_output_dir { output_dir.to_owned() diff --git a/tests/integration.rs b/tests/integration.rs index 1fc428a7b..b83d9bfd8 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -1288,10 +1288,10 @@ fn yes_flag_merges_into_nonempty_dir() { ); } -/// Regression test: the CWD guard must block `remove_dir_all` on the current working directory -/// even when the user explicitly selects the overwrite option interactively. +/// Regression test for the CWD-extraction bug: `ouch d archive` in a non-empty CWD +/// should merge into the CWD without prompting, but ask for confirmation otherwise #[test] -fn cwd_guard_blocks_explicit_overwrite() { +fn decompress_into_cwd_merges_without_prompt() { let (_tempdir, dir) = testdir().unwrap(); let src = dir.join("src"); @@ -1300,24 +1300,24 @@ fn cwd_guard_blocks_explicit_overwrite() { let archive = dir.join("archive.tar.gz"); ouch!("-A", "c", &src, &archive); - // Give the "CWD" a pre-existing file to trigger a conflict on the output directory + // CWD already contains the archive itself plus an unrelated file let cwd = dir.join("cwd"); fs::create_dir_all(&cwd).unwrap(); fs::write(cwd.join("important.txt"), "keep this").unwrap(); + fs::copy(&archive, cwd.join("archive.tar.gz")).unwrap(); - // User explicitly answers "y" (overwrite); the guard should block deletion and fail + // Decompress with no `-d`; nothing typed on stdin crate::utils::cargo_bin() .current_dir(&cwd) .arg("decompress") - .arg(&archive) - .write_stdin("y") + .arg("archive.tar.gz") .assert() - .failure(); + .success(); - assert!(cwd.exists(), "CWD was deleted despite guard"); + assert!(cwd.join("important.txt").exists(), "pre-existing file was lost"); assert!( - cwd.join("important.txt").exists(), - "CWD contents were deleted despite guard" + cwd.join("src").join("file.txt").exists(), + "archive contents were not extracted" ); } From 7d74bc895fe666e7392e57d0be24391e5c62dbcb Mon Sep 17 00:00:00 2001 From: valoq Date: Mon, 27 Apr 2026 17:01:46 +0200 Subject: [PATCH 02/13] default to smart unpack --- src/cli/args.rs | 12 ++++- src/commands/decompress.rs | 97 +++++++++++++++++++++++++++++++++++--- src/commands/mod.rs | 4 ++ tests/integration.rs | 89 ++++++++++++++++++++++++++++++---- 4 files changed, 185 insertions(+), 17 deletions(-) diff --git a/src/cli/args.rs b/src/cli/args.rs index e66bd5b43..0da80e4f8 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -94,10 +94,14 @@ pub enum Subcommand { #[arg(required = true, num_args = 1.., value_hint = ValueHint::FilePath)] files: Vec, - /// Decompress inside OUTPUT_DIR instead of current directory - #[arg(short = 'd', long = "dir", value_hint = ValueHint::FilePath)] + /// Decompress inside OUTPUT_DIR instead of the basename-derived subdirectory + #[arg(short = 'd', long = "dir", value_hint = ValueHint::FilePath, conflicts_with = "here")] output_dir: Option, + /// Extract directly into the current directory, like `tar -xf` and `unzip` do + #[arg(long = "here")] + here: bool, + /// Remove the source file after successful decompression #[arg(short = 'r', long)] remove: bool, @@ -156,6 +160,7 @@ mod tests { // Put a crazy value here so no test can assert it unintentionally files: vec!["\x00\x11\x22".into()], output_dir: None, + here: false, remove: false, }, } @@ -169,6 +174,7 @@ mod tests { cmd: Subcommand::Decompress { files: to_paths(["file.tar.gz"]), output_dir: None, + here: false, remove: false, }, ..mock_cli_args() @@ -180,6 +186,7 @@ mod tests { cmd: Subcommand::Decompress { files: to_paths(["file.tar.gz"]), output_dir: None, + here: false, remove: false, }, ..mock_cli_args() @@ -191,6 +198,7 @@ mod tests { cmd: Subcommand::Decompress { files: to_paths(["a", "b", "c"]), output_dir: None, + here: false, remove: false, }, ..mock_cli_args() diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index 6b379b77c..0a8cb1f70 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -30,13 +30,19 @@ pub struct DecompressOptions<'a> { pub output_dir: &'a Path, /// Used when extracting single file formats and not archive formats pub output_file_path: PathBuf, + /// Whether the user passed `--dir` explicitly. When false (and `here` is false), + /// archives are extracted into a basename-derived subdirectory of the CWD. + pub output_dir_was_explicit: bool, + /// `--here`: extract directly into the current directory like `tar -xf`. + /// Only meaningful when `output_dir_was_explicit` is false. + pub here: bool, pub question_policy: QuestionPolicy, pub password: Option<&'a [u8]>, pub remove: bool, } enum DecompressionSummary { - Archive { files_unpacked: u64 }, + Archive { files_unpacked: u64, output_path: PathBuf }, NonArchive { output_path: PathBuf }, } @@ -87,6 +93,18 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { Ok(reader) }; + // Decide where archives extract: + // --dir -> extract into , no wrapper, no flatten + // --here -> extract into CWD (output_dir), no wrapper + // default -> extract into a basename-derived subdirectory; flatten the + // duplicate when the wrapper would contain exactly one entry + // whose name equals the basename + let archive_output_dir: &Path = if options.output_dir_was_explicit || options.here { + options.output_dir + } else { + &options.output_file_path + }; + let control_flow = match first_extension { Gzip | Bzip | Bzip3 | Lz4 | Lzma | Xz | Lzip | Snappy | Zstd | Brotli => { let reader = create_decoder_up_to_first_extension()?; @@ -108,7 +126,7 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { } Tar => unpack_archive( |output_dir| crate::archive::tar::unpack_archive(create_decoder_up_to_first_extension()?, output_dir), - options.output_dir, + archive_output_dir, options.question_policy, )?, Zip | SevenZip => { @@ -153,7 +171,7 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { unpack_archive( |output_dir| unpack_fn(reader, output_dir, options.password), - options.output_dir, + archive_output_dir, options.question_policy, )? } @@ -171,7 +189,7 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { }) }; - unpack_archive(unpack_fn, options.output_dir, options.question_policy)? + unpack_archive(unpack_fn, archive_output_dir, options.question_policy)? } #[cfg(not(feature = "unrar"))] Rar => { @@ -184,8 +202,17 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { }; match decompression_summary { - DecompressionSummary::Archive { files_unpacked } => { - info_accessible!("Successfully decompressed archive to {}", PathFmt(options.output_dir)); + DecompressionSummary::Archive { files_unpacked, output_path } => { + // In default mode (no --dir, no --here), if the wrapper subdir we created + // ended up containing exactly one entry whose name matches the wrapper itself + // (e.g. `archive.zip` contained a single `archive/` root), flatten that + // duplicate so the user sees `./archive/...` not `./archive/archive/...`. + let final_path = if !options.output_dir_was_explicit && !options.here { + deduplicate_basename_wrapper(&output_path)? + } else { + output_path + }; + info_accessible!("Successfully decompressed archive to {}", PathFmt(&final_path)); info_accessible!("Files unpacked: {files_unpacked}"); } DecompressionSummary::NonArchive { output_path } => { @@ -241,5 +268,61 @@ fn unpack_archive( let files_unpacked = unpack_fn(&output_dir_cleaned)?; - Ok(ControlFlow::Continue(DecompressionSummary::Archive { files_unpacked })) + Ok(ControlFlow::Continue(DecompressionSummary::Archive { + files_unpacked, + output_path: output_dir_cleaned, + })) +} + +/// If `wrapper` is a directory containing exactly one entry whose name matches the +/// wrapper's own basename (e.g. extracting `archive.zip` produced `archive/archive/`), +/// flatten it: move the inner entry up one level and remove the empty wrapper. +/// +/// Returns the resulting path the user should see. If anything goes wrong with the +/// flattening, leaves the wrapper in place — extraction has already succeeded, the +/// content is intact, just one directory level deeper than ideal — and returns the +/// original `wrapper` path. +fn deduplicate_basename_wrapper(wrapper: &Path) -> Result { + let wrapper_name = match wrapper.file_name() { + Some(n) => n, + None => return Ok(wrapper.to_path_buf()), + }; + + // Read at most two entries. A single-entry directory has exactly one. + let mut entries = fs::read_dir(wrapper)?; + let first = match entries.next().transpose()? { + Some(e) => e, + None => return Ok(wrapper.to_path_buf()), + }; + if entries.next().transpose()?.is_some() { + return Ok(wrapper.to_path_buf()); + } + drop(entries); + + if first.file_name() != wrapper_name { + return Ok(wrapper.to_path_buf()); + } + + // The wrapper duplicates the inner entry's name. Promote the inner entry by: + // 1. moving it into a sibling staging directory + // 2. removing the now-empty wrapper + // 3. moving it back to the wrapper's path + // Each step leaves the filesystem in a consistent state, and a failure midway + // leaves the user with valid extracted content (just nested one level). + let inner_path = first.path(); + let parent = match wrapper.parent() { + Some(p) => p, + None => return Ok(wrapper.to_path_buf()), + }; + + let staging = tempfile::Builder::new() + .prefix(".ouch-staging-") + .tempdir_in(parent)?; + let staging_target = staging.path().join(wrapper_name); + fs::rename(&inner_path, &staging_target)?; + fs::remove_dir(wrapper)?; + fs::rename(&staging_target, wrapper)?; + // The (empty) staging directory is cleaned up when `staging` drops. + + Ok(wrapper.to_path_buf()) } diff --git a/src/commands/mod.rs b/src/commands/mod.rs index f155f38c1..f712adea1 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -141,6 +141,7 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic Subcommand::Decompress { files, output_dir, + here, remove, } => { let mut files_output_paths: Vec<_> = vec![]; @@ -181,6 +182,7 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic // The directory that will contain the output files // We default to the current directory if the user didn't specify an output directory with --dir + let output_dir_was_explicit = output_dir.is_some(); let output_dir = if let Some(dir) = output_dir { utils::create_dir_if_non_existent(&dir)?; // If not canonicalized, strip_prefix won't work and logs will break @@ -207,6 +209,8 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic formats, output_dir: &output_dir, output_file_path, + output_dir_was_explicit, + here, question_policy, password: args.password.as_deref().map(|str| { <[u8] as ByteSlice>::from_os_str(str).expect("convert password to bytes failed") diff --git a/tests/integration.rs b/tests/integration.rs index b83d9bfd8..9d721b91f 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -1283,15 +1283,16 @@ fn yes_flag_merges_into_nonempty_dir() { "--yes wiped the output directory instead of merging" ); assert!( - output.join("src").join("new_file.txt").exists(), - "archive contents were not extracted" + output.join("archive").join("src").join("new_file.txt").exists(), + "archive contents were not extracted into the basename subdir" ); } -/// Regression test for the CWD-extraction bug: `ouch d archive` in a non-empty CWD -/// should merge into the CWD without prompting, but ask for confirmation otherwise +/// `ouch d archive` with no `-d`/`--here` should extract into a basename-derived +/// subdirectory of the CWD (`./archive/`), not directly into the CWD. Pre-existing +/// CWD contents stay intact. #[test] -fn decompress_into_cwd_merges_without_prompt() { +fn decompress_default_creates_basename_subdir() { let (_tempdir, dir) = testdir().unwrap(); let src = dir.join("src"); @@ -1300,13 +1301,11 @@ fn decompress_into_cwd_merges_without_prompt() { let archive = dir.join("archive.tar.gz"); ouch!("-A", "c", &src, &archive); - // CWD already contains the archive itself plus an unrelated file let cwd = dir.join("cwd"); fs::create_dir_all(&cwd).unwrap(); fs::write(cwd.join("important.txt"), "keep this").unwrap(); fs::copy(&archive, cwd.join("archive.tar.gz")).unwrap(); - // Decompress with no `-d`; nothing typed on stdin crate::utils::cargo_bin() .current_dir(&cwd) .arg("decompress") @@ -1314,10 +1313,84 @@ fn decompress_into_cwd_merges_without_prompt() { .assert() .success(); + assert!(cwd.join("important.txt").exists(), "pre-existing file was lost"); + assert!( + cwd.join("archive").join("src").join("file.txt").exists(), + "archive contents were not extracted into the basename subdir" + ); + assert!( + !cwd.join("src").join("file.txt").exists(), + "archive contents leaked directly into the CWD instead of the subdir" + ); +} + +/// When the archive's single root entry has the same name as the wrapper directory +/// (e.g. `testing.tar.gz` containing `testing/`), the duplicate is flattened: the +/// user gets `cwd/testing/...` rather than `cwd/testing/testing/...`. +#[test] +fn decompress_default_flattens_duplicate_basename_wrapper() { + let (_tempdir, dir) = testdir().unwrap(); + + let src = dir.join("testing"); + fs::create_dir_all(&src).unwrap(); + fs::write(src.join("file.txt"), "content").unwrap(); + let archive = dir.join("testing.tar.gz"); + ouch!("-A", "c", &src, &archive); + + let cwd = dir.join("cwd"); + fs::create_dir_all(&cwd).unwrap(); + fs::copy(&archive, cwd.join("testing.tar.gz")).unwrap(); + + crate::utils::cargo_bin() + .current_dir(&cwd) + .arg("decompress") + .arg("testing.tar.gz") + .assert() + .success(); + + assert!( + cwd.join("testing").join("file.txt").exists(), + "flattened content not present" + ); + assert!( + !cwd.join("testing").join("testing").exists(), + "duplicate basename wrapper was not flattened" + ); +} + +/// `--here` extracts directly into the current directory, like `tar -xf` and `unzip`. +/// No wrapper folder, no flatten, no overwrite prompt for pre-existing CWD contents. +#[test] +fn decompress_here_flag_extracts_into_cwd() { + let (_tempdir, dir) = testdir().unwrap(); + + let src = dir.join("src"); + fs::create_dir_all(&src).unwrap(); + fs::write(src.join("file.txt"), "content").unwrap(); + let archive = dir.join("archive.tar.gz"); + ouch!("-A", "c", &src, &archive); + + let cwd = dir.join("cwd"); + fs::create_dir_all(&cwd).unwrap(); + fs::write(cwd.join("important.txt"), "keep this").unwrap(); + fs::copy(&archive, cwd.join("archive.tar.gz")).unwrap(); + + crate::utils::cargo_bin() + .current_dir(&cwd) + .arg("decompress") + .arg("archive.tar.gz") + .arg("--here") + .assert() + .success(); + assert!(cwd.join("important.txt").exists(), "pre-existing file was lost"); assert!( cwd.join("src").join("file.txt").exists(), - "archive contents were not extracted" + "--here did not extract directly into CWD" + ); + assert!( + !cwd.join("archive").exists(), + "--here unexpectedly created a wrapper subdirectory" ); } From adc7d4e9e3eba876955b5540f140215fdc17a4af Mon Sep 17 00:00:00 2001 From: valoq Date: Mon, 27 Apr 2026 17:10:11 +0200 Subject: [PATCH 03/13] formatting --- src/commands/decompress.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index 0a8cb1f70..d92adeb53 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -202,7 +202,10 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { }; match decompression_summary { - DecompressionSummary::Archive { files_unpacked, output_path } => { + DecompressionSummary::Archive { + files_unpacked, + output_path, + } => { // In default mode (no --dir, no --here), if the wrapper subdir we created // ended up containing exactly one entry whose name matches the wrapper itself // (e.g. `archive.zip` contained a single `archive/` root), flatten that @@ -315,9 +318,7 @@ fn deduplicate_basename_wrapper(wrapper: &Path) -> Result { None => return Ok(wrapper.to_path_buf()), }; - let staging = tempfile::Builder::new() - .prefix(".ouch-staging-") - .tempdir_in(parent)?; + let staging = tempfile::Builder::new().prefix(".ouch-staging-").tempdir_in(parent)?; let staging_target = staging.path().join(wrapper_name); fs::rename(&inner_path, &staging_target)?; fs::remove_dir(wrapper)?; From c27d50b1060b88b51b79d0cf9c906093d38633da Mon Sep 17 00:00:00 2001 From: valoq Date: Tue, 28 Apr 2026 00:12:46 +0200 Subject: [PATCH 04/13] more unit tests --- src/commands/decompress.rs | 137 +++++++++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+) diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index d92adeb53..1f9b94d05 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -327,3 +327,140 @@ fn deduplicate_basename_wrapper(wrapper: &Path) -> Result { Ok(wrapper.to_path_buf()) } + +#[cfg(test)] +mod tests { + use std::fs as std_fs; + + use tempfile::tempdir; + + use super::*; + + /// Helper: collect the relative paths of every entry under `root`, sorted. + fn list_tree(root: &Path) -> Vec { + fn walk(p: &Path, base: &Path, out: &mut Vec) { + if let Ok(entries) = std_fs::read_dir(p) { + for entry in entries.flatten() { + let path = entry.path(); + let rel = path + .strip_prefix(base) + .unwrap() + .to_string_lossy() + .replace('\\', "/"); + if path.is_dir() { + out.push(format!("{rel}/")); + walk(&path, base, out); + } else { + out.push(rel); + } + } + } + } + let mut out = Vec::new(); + walk(root, root, &mut out); + out.sort(); + out + } + + /// The main case: wrapper contains exactly one entry whose name equals the wrapper's + /// name. The inner entry should be promoted up one level. + #[test] + fn deduplicate_flattens_when_inner_dir_matches_wrapper_name() { + let dir = tempdir().unwrap(); + let wrapper = dir.path().join("archive"); + let inner = wrapper.join("archive"); + std_fs::create_dir_all(&inner).unwrap(); + std_fs::write(inner.join("a.txt"), "a").unwrap(); + std_fs::write(inner.join("b.txt"), "b").unwrap(); + + let result = deduplicate_basename_wrapper(&wrapper).unwrap(); + + assert_eq!(result, wrapper); + assert_eq!(list_tree(&wrapper), vec!["a.txt", "b.txt"]); + } + + /// Wrapper contains a single entry, but its name differs from the wrapper's name. + /// No flatten should happen — the wrapper survives and the inner entry stays nested. + #[test] + fn deduplicate_keeps_wrapper_when_inner_name_differs() { + let dir = tempdir().unwrap(); + let wrapper = dir.path().join("archive"); + let inner = wrapper.join("mytool"); + std_fs::create_dir_all(&inner).unwrap(); + std_fs::write(inner.join("file.txt"), "x").unwrap(); + + let result = deduplicate_basename_wrapper(&wrapper).unwrap(); + + assert_eq!(result, wrapper); + assert_eq!(list_tree(&wrapper), vec!["mytool/", "mytool/file.txt"]); + } + + /// Wrapper contains two or more entries — no flatten regardless of names. + #[test] + fn deduplicate_keeps_wrapper_when_multiple_entries() { + let dir = tempdir().unwrap(); + let wrapper = dir.path().join("archive"); + std_fs::create_dir_all(&wrapper).unwrap(); + std_fs::write(wrapper.join("a.txt"), "a").unwrap(); + std_fs::write(wrapper.join("b.txt"), "b").unwrap(); + + let result = deduplicate_basename_wrapper(&wrapper).unwrap(); + + assert_eq!(result, wrapper); + assert_eq!(list_tree(&wrapper), vec!["a.txt", "b.txt"]); + } + + /// Empty wrapper — nothing to flatten, no-op. + #[test] + fn deduplicate_is_noop_on_empty_wrapper() { + let dir = tempdir().unwrap(); + let wrapper = dir.path().join("archive"); + std_fs::create_dir(&wrapper).unwrap(); + + let result = deduplicate_basename_wrapper(&wrapper).unwrap(); + + assert_eq!(result, wrapper); + assert!(wrapper.is_dir()); + assert_eq!(list_tree(&wrapper), Vec::::new()); + } + + /// Edge case: the single inner entry is a *file* (not a directory) whose name + /// equals the wrapper's name. The wrapper directory should be replaced with that file. + #[test] + fn deduplicate_promotes_single_inner_file_with_matching_name() { + let dir = tempdir().unwrap(); + let wrapper = dir.path().join("archive"); + std_fs::create_dir(&wrapper).unwrap(); + std_fs::write(wrapper.join("archive"), "data").unwrap(); + + let result = deduplicate_basename_wrapper(&wrapper).unwrap(); + + assert_eq!(result, wrapper); + assert!(wrapper.is_file(), "wrapper should now be a file, not a directory"); + assert_eq!(std_fs::read(&wrapper).unwrap(), b"data"); + } + + /// The flatten only collapses *one* level: nested same-name directories produced + /// by the archive itself stay intact. For example, an archive whose layout is + /// `testing/testing/file` extracted into `./testing/` should leave the user with + /// `./testing/testing/file`, not `./testing/file`. + #[test] + fn deduplicate_only_flattens_outer_wrapper_not_inner_duplicates() { + let dir = tempdir().unwrap(); + let wrapper = dir.path().join("testing"); + let outer_inner = wrapper.join("testing"); + let nested = outer_inner.join("testing"); + std_fs::create_dir_all(&nested).unwrap(); + std_fs::write(nested.join("file"), "deep").unwrap(); + + let result = deduplicate_basename_wrapper(&wrapper).unwrap(); + + assert_eq!(result, wrapper); + // After one flatten, `testing/testing/file` should remain — the algorithm only + // collapses the outer wrapper exactly once. + assert_eq!( + list_tree(&wrapper), + vec!["testing/", "testing/file"] + ); + } +} From 4bbfd13e21ad50e89fc38a106b3c038a83ffec3b Mon Sep 17 00:00:00 2001 From: valoq Date: Tue, 28 Apr 2026 00:17:45 +0200 Subject: [PATCH 05/13] fix formatting --- src/commands/decompress.rs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index 1f9b94d05..fa1128caa 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -342,11 +342,7 @@ mod tests { if let Ok(entries) = std_fs::read_dir(p) { for entry in entries.flatten() { let path = entry.path(); - let rel = path - .strip_prefix(base) - .unwrap() - .to_string_lossy() - .replace('\\', "/"); + let rel = path.strip_prefix(base).unwrap().to_string_lossy().replace('\\', "/"); if path.is_dir() { out.push(format!("{rel}/")); walk(&path, base, out); @@ -458,9 +454,6 @@ mod tests { assert_eq!(result, wrapper); // After one flatten, `testing/testing/file` should remain — the algorithm only // collapses the outer wrapper exactly once. - assert_eq!( - list_tree(&wrapper), - vec!["testing/", "testing/file"] - ); + assert_eq!(list_tree(&wrapper), vec!["testing/", "testing/file"]); } } From a58b5e1a6d981251df0c58fd170860001134f598 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Marcos?= Date: Tue, 28 Apr 2026 04:48:06 -0300 Subject: [PATCH 06/13] fix changelog --- CHANGELOG.md | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f2a8b11c7..bbf2e0015 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,17 +15,17 @@ Categories Used: **Bullet points in chronological order by PR** -## [Unreleased](https://github.com/ouch-org/ouch/compare/0.7.0...HEAD) - -### Removals +## [Unreleased](https://github.com/ouch-org/ouch/compare/0.7.1...HEAD) ### New Features -- List: show symlink targets (for tar and zip) (https://github.com/ouch-org/ouch/pull/934) +- Unpack into new folder by default (https://github.com/ouch-org/ouch/pull/962). +- Add `--here` flag to unpack into current directory (https://github.com/ouch-org/ouch/pull/962). ### Improvements - Report mtime-set errors for `.7z` as warnings (https://github.com/ouch-org/ouch/pull/950) +- `list`: also display symlink targets (https://github.com/ouch-org/ouch/pull/934) ### Bug Fixes @@ -33,6 +33,13 @@ Categories Used: ### Tweaks + +## [0.7.1](https://github.com/ouch-org/ouch/releases/tag/0.7.1) + +### Bug Fixes + +- Fix `ouch --version` being outdated. + ## [0.7.0](https://github.com/ouch-org/ouch/releases/tag/0.7.0) ### Removals From ba024e820861e54bf5efb46fc92044a838fb4192 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Marcos?= Date: Tue, 28 Apr 2026 04:56:55 -0300 Subject: [PATCH 07/13] rename temporary file prefix + tiny refac --- src/commands/decompress.rs | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index fa1128caa..cfe985ca1 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -286,23 +286,25 @@ fn unpack_archive( /// content is intact, just one directory level deeper than ideal — and returns the /// original `wrapper` path. fn deduplicate_basename_wrapper(wrapper: &Path) -> Result { - let wrapper_name = match wrapper.file_name() { - Some(n) => n, - None => return Ok(wrapper.to_path_buf()), + let Some(wrapper_name) = wrapper.file_name() else { + return Ok(wrapper.to_path_buf()); }; - // Read at most two entries. A single-entry directory has exactly one. - let mut entries = fs::read_dir(wrapper)?; - let first = match entries.next().transpose()? { - Some(e) => e, - None => return Ok(wrapper.to_path_buf()), + let only_file_in_dir = { + // Read at most two entries. A single-entry directory has exactly one. + let mut entries = fs::read_dir(wrapper)?; + let Some(first_file) = entries.next().transpose()? else { + return Ok(wrapper.to_path_buf()); + }; + // More than one entry, don't deduplicate + if entries.next().transpose()?.is_some() { + return Ok(wrapper.to_path_buf()); + } + first_file }; - if entries.next().transpose()?.is_some() { - return Ok(wrapper.to_path_buf()); - } - drop(entries); - if first.file_name() != wrapper_name { + // name doesn't match, nothing to deduplicate + if only_file_in_dir.file_name() != wrapper_name { return Ok(wrapper.to_path_buf()); } @@ -312,13 +314,12 @@ fn deduplicate_basename_wrapper(wrapper: &Path) -> Result { // 3. moving it back to the wrapper's path // Each step leaves the filesystem in a consistent state, and a failure midway // leaves the user with valid extracted content (just nested one level). - let inner_path = first.path(); - let parent = match wrapper.parent() { - Some(p) => p, - None => return Ok(wrapper.to_path_buf()), + let inner_path = only_file_in_dir.path(); + let Some(parent) = wrapper.parent() else { + return Ok(wrapper.to_path_buf()); }; - let staging = tempfile::Builder::new().prefix(".ouch-staging-").tempdir_in(parent)?; + let staging = tempfile::Builder::new().prefix("temporary-").tempdir_in(parent)?; let staging_target = staging.path().join(wrapper_name); fs::rename(&inner_path, &staging_target)?; fs::remove_dir(wrapper)?; From b9d7e5f268abb1c7396903e5ed447dfed86b67ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Marcos?= Date: Tue, 28 Apr 2026 05:24:07 -0300 Subject: [PATCH 08/13] in deduplication don't use tempfile to fullfil this promise: ``` // Each step leaves the filesystem in a consistent state, and a failure midway // leaves the user with valid extracted content (just nested one level). ``` --- src/commands/decompress.rs | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index cfe985ca1..d27987e55 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -309,7 +309,7 @@ fn deduplicate_basename_wrapper(wrapper: &Path) -> Result { } // The wrapper duplicates the inner entry's name. Promote the inner entry by: - // 1. moving it into a sibling staging directory + // 1. moving it into a sibling temporary directory // 2. removing the now-empty wrapper // 3. moving it back to the wrapper's path // Each step leaves the filesystem in a consistent state, and a failure midway @@ -319,12 +319,22 @@ fn deduplicate_basename_wrapper(wrapper: &Path) -> Result { return Ok(wrapper.to_path_buf()); }; - let staging = tempfile::Builder::new().prefix("temporary-").tempdir_in(parent)?; - let staging_target = staging.path().join(wrapper_name); - fs::rename(&inner_path, &staging_target)?; + // Create the sibling + let sibling_path = parent.join("ouch-temporary"); + fs::create_dir(&sibling_path)?; + + // Move child inside the sibling + let path_inside_sibling = sibling_path.join(wrapper_name); + fs::rename(&inner_path, &path_inside_sibling)?; + + // Delete old parent fs::remove_dir(wrapper)?; - fs::rename(&staging_target, wrapper)?; - // The (empty) staging directory is cleaned up when `staging` drops. + + // Move child to its dead parent place + fs::rename(&path_inside_sibling, wrapper)?; + + // Delete the temporary sibling + fs::remove_dir(sibling_path)?; Ok(wrapper.to_path_buf()) } From ef9a42bb75a0bccfa54b6aca4a7568248c37f7e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Marcos?= Date: Tue, 28 Apr 2026 05:28:14 -0300 Subject: [PATCH 09/13] update docs --- src/commands/decompress.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index d27987e55..22581ce0b 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -277,14 +277,11 @@ fn unpack_archive( })) } -/// If `wrapper` is a directory containing exactly one entry whose name matches the -/// wrapper's own basename (e.g. extracting `archive.zip` produced `archive/archive/`), -/// flatten it: move the inner entry up one level and remove the empty wrapper. +/// Expects `wrapper` to be a just-decompressed archive output directory, if +/// `wrapper` contains exactly one entry with the same name (e.g. extracting +/// `archive.zip` produced `archive/archive/...`), then flatten it to `archive/...`. /// -/// Returns the resulting path the user should see. If anything goes wrong with the -/// flattening, leaves the wrapper in place — extraction has already succeeded, the -/// content is intact, just one directory level deeper than ideal — and returns the -/// original `wrapper` path. +/// Returns the resulting path the user should see. If reads fail, fn deduplicate_basename_wrapper(wrapper: &Path) -> Result { let Some(wrapper_name) = wrapper.file_name() else { return Ok(wrapper.to_path_buf()); From 5b086c2e3d307cfcc47f7b7bac5192e8025ef363 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Marcos?= Date: Tue, 28 Apr 2026 05:28:23 -0300 Subject: [PATCH 10/13] remove return, all paths return the same input --- src/commands/decompress.rs | 48 ++++++++++++++------------------------ 1 file changed, 17 insertions(+), 31 deletions(-) diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index 22581ce0b..9258c2e96 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -210,12 +210,10 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { // ended up containing exactly one entry whose name matches the wrapper itself // (e.g. `archive.zip` contained a single `archive/` root), flatten that // duplicate so the user sees `./archive/...` not `./archive/archive/...`. - let final_path = if !options.output_dir_was_explicit && !options.here { - deduplicate_basename_wrapper(&output_path)? - } else { - output_path - }; - info_accessible!("Successfully decompressed archive to {}", PathFmt(&final_path)); + if !options.output_dir_was_explicit && !options.here { + deduplicate_basename_wrapper(&output_path)?; + } + info_accessible!("Successfully decompressed archive to {}", PathFmt(&output_path)); info_accessible!("Files unpacked: {files_unpacked}"); } DecompressionSummary::NonArchive { output_path } => { @@ -282,27 +280,27 @@ fn unpack_archive( /// `archive.zip` produced `archive/archive/...`), then flatten it to `archive/...`. /// /// Returns the resulting path the user should see. If reads fail, -fn deduplicate_basename_wrapper(wrapper: &Path) -> Result { +fn deduplicate_basename_wrapper(wrapper: &Path) -> Result<()> { let Some(wrapper_name) = wrapper.file_name() else { - return Ok(wrapper.to_path_buf()); + return Ok(()); }; let only_file_in_dir = { // Read at most two entries. A single-entry directory has exactly one. let mut entries = fs::read_dir(wrapper)?; let Some(first_file) = entries.next().transpose()? else { - return Ok(wrapper.to_path_buf()); + return Ok(()); }; // More than one entry, don't deduplicate if entries.next().transpose()?.is_some() { - return Ok(wrapper.to_path_buf()); + return Ok(()); } first_file }; // name doesn't match, nothing to deduplicate if only_file_in_dir.file_name() != wrapper_name { - return Ok(wrapper.to_path_buf()); + return Ok(()); } // The wrapper duplicates the inner entry's name. Promote the inner entry by: @@ -313,7 +311,7 @@ fn deduplicate_basename_wrapper(wrapper: &Path) -> Result { // leaves the user with valid extracted content (just nested one level). let inner_path = only_file_in_dir.path(); let Some(parent) = wrapper.parent() else { - return Ok(wrapper.to_path_buf()); + return Ok(()); }; // Create the sibling @@ -333,7 +331,7 @@ fn deduplicate_basename_wrapper(wrapper: &Path) -> Result { // Delete the temporary sibling fs::remove_dir(sibling_path)?; - Ok(wrapper.to_path_buf()) + Ok(()) } #[cfg(test)] @@ -377,9 +375,7 @@ mod tests { std_fs::write(inner.join("a.txt"), "a").unwrap(); std_fs::write(inner.join("b.txt"), "b").unwrap(); - let result = deduplicate_basename_wrapper(&wrapper).unwrap(); - - assert_eq!(result, wrapper); + deduplicate_basename_wrapper(&wrapper).unwrap(); assert_eq!(list_tree(&wrapper), vec!["a.txt", "b.txt"]); } @@ -393,9 +389,7 @@ mod tests { std_fs::create_dir_all(&inner).unwrap(); std_fs::write(inner.join("file.txt"), "x").unwrap(); - let result = deduplicate_basename_wrapper(&wrapper).unwrap(); - - assert_eq!(result, wrapper); + deduplicate_basename_wrapper(&wrapper).unwrap(); assert_eq!(list_tree(&wrapper), vec!["mytool/", "mytool/file.txt"]); } @@ -408,9 +402,7 @@ mod tests { std_fs::write(wrapper.join("a.txt"), "a").unwrap(); std_fs::write(wrapper.join("b.txt"), "b").unwrap(); - let result = deduplicate_basename_wrapper(&wrapper).unwrap(); - - assert_eq!(result, wrapper); + deduplicate_basename_wrapper(&wrapper).unwrap(); assert_eq!(list_tree(&wrapper), vec!["a.txt", "b.txt"]); } @@ -421,9 +413,7 @@ mod tests { let wrapper = dir.path().join("archive"); std_fs::create_dir(&wrapper).unwrap(); - let result = deduplicate_basename_wrapper(&wrapper).unwrap(); - - assert_eq!(result, wrapper); + deduplicate_basename_wrapper(&wrapper).unwrap(); assert!(wrapper.is_dir()); assert_eq!(list_tree(&wrapper), Vec::::new()); } @@ -437,9 +427,7 @@ mod tests { std_fs::create_dir(&wrapper).unwrap(); std_fs::write(wrapper.join("archive"), "data").unwrap(); - let result = deduplicate_basename_wrapper(&wrapper).unwrap(); - - assert_eq!(result, wrapper); + deduplicate_basename_wrapper(&wrapper).unwrap(); assert!(wrapper.is_file(), "wrapper should now be a file, not a directory"); assert_eq!(std_fs::read(&wrapper).unwrap(), b"data"); } @@ -457,9 +445,7 @@ mod tests { std_fs::create_dir_all(&nested).unwrap(); std_fs::write(nested.join("file"), "deep").unwrap(); - let result = deduplicate_basename_wrapper(&wrapper).unwrap(); - - assert_eq!(result, wrapper); + deduplicate_basename_wrapper(&wrapper).unwrap(); // After one flatten, `testing/testing/file` should remain — the algorithm only // collapses the outer wrapper exactly once. assert_eq!(list_tree(&wrapper), vec!["testing/", "testing/file"]); From c2e7af47228ccf00104a8f7937fe438e88f4ccbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Marcos?= Date: Tue, 28 Apr 2026 05:36:06 -0300 Subject: [PATCH 11/13] use fs-tree to read and assert dir structure --- Cargo.lock | 11 +++++++ Cargo.toml | 1 + src/commands/decompress.rs | 59 +++++++++++++++++++++----------------- 3 files changed, 44 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d92e4b2ab..aaf1529c8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -705,6 +705,16 @@ dependencies = [ "autocfg", ] +[[package]] +name = "fs-tree" +version = "0.8.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5082380dac47b23b39a145837120bd0e7a9597ca8e70677d804f356034f55fbb" +dependencies = [ + "file_type_enum", + "fs-err 3.3.0", +] + [[package]] name = "futures-core" version = "0.3.31" @@ -1175,6 +1185,7 @@ dependencies = [ "filetime_creation", "flate2", "fs-err 2.11.0", + "fs-tree", "glob", "gzp", "ignore", diff --git a/Cargo.toml b/Cargo.toml index 7ae38f409..7bed5606c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -54,6 +54,7 @@ zstd = { version = "0.13.2", default-features = false, features = ["zstdmt"] } [dev-dependencies] anyhow = "1.0.102" assert_cmd = "2.0.14" +fs-tree = "0.8.4" glob = "0.3.2" infer = "0.16.0" insta = { version = "1.40.0", features = ["filters"] } diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index 9258c2e96..86de5ca01 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -338,32 +338,11 @@ fn deduplicate_basename_wrapper(wrapper: &Path) -> Result<()> { mod tests { use std::fs as std_fs; + use fs_tree::{FsTree, tree}; use tempfile::tempdir; use super::*; - /// Helper: collect the relative paths of every entry under `root`, sorted. - fn list_tree(root: &Path) -> Vec { - fn walk(p: &Path, base: &Path, out: &mut Vec) { - if let Ok(entries) = std_fs::read_dir(p) { - for entry in entries.flatten() { - let path = entry.path(); - let rel = path.strip_prefix(base).unwrap().to_string_lossy().replace('\\', "/"); - if path.is_dir() { - out.push(format!("{rel}/")); - walk(&path, base, out); - } else { - out.push(rel); - } - } - } - } - let mut out = Vec::new(); - walk(root, root, &mut out); - out.sort(); - out - } - /// The main case: wrapper contains exactly one entry whose name equals the wrapper's /// name. The inner entry should be promoted up one level. #[test] @@ -376,7 +355,13 @@ mod tests { std_fs::write(inner.join("b.txt"), "b").unwrap(); deduplicate_basename_wrapper(&wrapper).unwrap(); - assert_eq!(list_tree(&wrapper), vec!["a.txt", "b.txt"]); + assert_eq!( + FsTree::read_at(&wrapper).unwrap(), + tree! { + "a.txt" + "b.txt" + } + ); } /// Wrapper contains a single entry, but its name differs from the wrapper's name. @@ -390,7 +375,14 @@ mod tests { std_fs::write(inner.join("file.txt"), "x").unwrap(); deduplicate_basename_wrapper(&wrapper).unwrap(); - assert_eq!(list_tree(&wrapper), vec!["mytool/", "mytool/file.txt"]); + assert_eq!( + FsTree::read_at(&wrapper).unwrap(), + tree! { + mytool: [ + "file.txt" + ] + } + ); } /// Wrapper contains two or more entries — no flatten regardless of names. @@ -403,7 +395,13 @@ mod tests { std_fs::write(wrapper.join("b.txt"), "b").unwrap(); deduplicate_basename_wrapper(&wrapper).unwrap(); - assert_eq!(list_tree(&wrapper), vec!["a.txt", "b.txt"]); + assert_eq!( + FsTree::read_at(&wrapper).unwrap(), + tree! { + "a.txt" + "b.txt" + } + ); } /// Empty wrapper — nothing to flatten, no-op. @@ -415,7 +413,7 @@ mod tests { deduplicate_basename_wrapper(&wrapper).unwrap(); assert!(wrapper.is_dir()); - assert_eq!(list_tree(&wrapper), Vec::::new()); + assert_eq!(FsTree::read_at(&wrapper).unwrap(), tree! {}); } /// Edge case: the single inner entry is a *file* (not a directory) whose name @@ -448,6 +446,13 @@ mod tests { deduplicate_basename_wrapper(&wrapper).unwrap(); // After one flatten, `testing/testing/file` should remain — the algorithm only // collapses the outer wrapper exactly once. - assert_eq!(list_tree(&wrapper), vec!["testing/", "testing/file"]); + assert_eq!( + FsTree::read_at(&wrapper).unwrap(), + tree! { + testing: [ + file + ] + } + ); } } From 075940c51d5df9cc0f208a773068e490ed0768da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Marcos?= Date: Tue, 28 Apr 2026 05:58:00 -0300 Subject: [PATCH 12/13] Revert "use fs-tree to read and assert dir structure" This reverts commit ec497cf1d39f3d2e317f37a40757bac5afb794b1. That crate is unix-only but we want these tests to pass on Windows too oof --- Cargo.lock | 11 ------- Cargo.toml | 1 - src/commands/decompress.rs | 59 +++++++++++++++++--------------------- 3 files changed, 27 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index aaf1529c8..d92e4b2ab 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -705,16 +705,6 @@ dependencies = [ "autocfg", ] -[[package]] -name = "fs-tree" -version = "0.8.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5082380dac47b23b39a145837120bd0e7a9597ca8e70677d804f356034f55fbb" -dependencies = [ - "file_type_enum", - "fs-err 3.3.0", -] - [[package]] name = "futures-core" version = "0.3.31" @@ -1185,7 +1175,6 @@ dependencies = [ "filetime_creation", "flate2", "fs-err 2.11.0", - "fs-tree", "glob", "gzp", "ignore", diff --git a/Cargo.toml b/Cargo.toml index 7bed5606c..7ae38f409 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -54,7 +54,6 @@ zstd = { version = "0.13.2", default-features = false, features = ["zstdmt"] } [dev-dependencies] anyhow = "1.0.102" assert_cmd = "2.0.14" -fs-tree = "0.8.4" glob = "0.3.2" infer = "0.16.0" insta = { version = "1.40.0", features = ["filters"] } diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index 86de5ca01..9258c2e96 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -338,11 +338,32 @@ fn deduplicate_basename_wrapper(wrapper: &Path) -> Result<()> { mod tests { use std::fs as std_fs; - use fs_tree::{FsTree, tree}; use tempfile::tempdir; use super::*; + /// Helper: collect the relative paths of every entry under `root`, sorted. + fn list_tree(root: &Path) -> Vec { + fn walk(p: &Path, base: &Path, out: &mut Vec) { + if let Ok(entries) = std_fs::read_dir(p) { + for entry in entries.flatten() { + let path = entry.path(); + let rel = path.strip_prefix(base).unwrap().to_string_lossy().replace('\\', "/"); + if path.is_dir() { + out.push(format!("{rel}/")); + walk(&path, base, out); + } else { + out.push(rel); + } + } + } + } + let mut out = Vec::new(); + walk(root, root, &mut out); + out.sort(); + out + } + /// The main case: wrapper contains exactly one entry whose name equals the wrapper's /// name. The inner entry should be promoted up one level. #[test] @@ -355,13 +376,7 @@ mod tests { std_fs::write(inner.join("b.txt"), "b").unwrap(); deduplicate_basename_wrapper(&wrapper).unwrap(); - assert_eq!( - FsTree::read_at(&wrapper).unwrap(), - tree! { - "a.txt" - "b.txt" - } - ); + assert_eq!(list_tree(&wrapper), vec!["a.txt", "b.txt"]); } /// Wrapper contains a single entry, but its name differs from the wrapper's name. @@ -375,14 +390,7 @@ mod tests { std_fs::write(inner.join("file.txt"), "x").unwrap(); deduplicate_basename_wrapper(&wrapper).unwrap(); - assert_eq!( - FsTree::read_at(&wrapper).unwrap(), - tree! { - mytool: [ - "file.txt" - ] - } - ); + assert_eq!(list_tree(&wrapper), vec!["mytool/", "mytool/file.txt"]); } /// Wrapper contains two or more entries — no flatten regardless of names. @@ -395,13 +403,7 @@ mod tests { std_fs::write(wrapper.join("b.txt"), "b").unwrap(); deduplicate_basename_wrapper(&wrapper).unwrap(); - assert_eq!( - FsTree::read_at(&wrapper).unwrap(), - tree! { - "a.txt" - "b.txt" - } - ); + assert_eq!(list_tree(&wrapper), vec!["a.txt", "b.txt"]); } /// Empty wrapper — nothing to flatten, no-op. @@ -413,7 +415,7 @@ mod tests { deduplicate_basename_wrapper(&wrapper).unwrap(); assert!(wrapper.is_dir()); - assert_eq!(FsTree::read_at(&wrapper).unwrap(), tree! {}); + assert_eq!(list_tree(&wrapper), Vec::::new()); } /// Edge case: the single inner entry is a *file* (not a directory) whose name @@ -446,13 +448,6 @@ mod tests { deduplicate_basename_wrapper(&wrapper).unwrap(); // After one flatten, `testing/testing/file` should remain — the algorithm only // collapses the outer wrapper exactly once. - assert_eq!( - FsTree::read_at(&wrapper).unwrap(), - tree! { - testing: [ - file - ] - } - ); + assert_eq!(list_tree(&wrapper), vec!["testing/", "testing/file"]); } } From 8c95ba58f0cedaf48c82f4bd8900c56016810a59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Marcos?= Date: Tue, 28 Apr 2026 18:51:00 -0300 Subject: [PATCH 13/13] add `--dir` tests for CWD and other folder the tests evidence that there is some inconsistency today between how '--dir' works when pointing to CWD or not when it points to CWD, it overwrites files inside of it without asking, that is a side-effect of this PR fixing it asking to remove CWD entirely --- tests/integration.rs | 134 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) diff --git a/tests/integration.rs b/tests/integration.rs index 9d721b91f..da9743a47 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -1422,6 +1422,140 @@ fn decompress_concatenated_bzip2_streams() { }); } +/// When decompressing with `--dir out`, first `ouch` should create the new +/// folder, on a second call, it fails and asks whether or not it should be +/// overwritten, a third call with `--yes` should succeed. +#[test] +fn decompress_dir_flag_to_another_dir_and_overwrite() { + let (_tempdir, dir) = testdir().unwrap(); + let input_folder = dir.join("folder"); + let input_file = input_folder.join("file"); + let output_file = dir.join("out/folder/file"); + let archive = dir.join("archive.zip"); + + fs::create_dir(&input_folder).unwrap(); + fs::write(&input_file, "first write").unwrap(); + + crate::utils::cargo_bin() + .arg("compress") + .arg(&input_folder) + .arg(&archive) + .assert() + .success(); + + // First decompress with `--dir out` — should succeed + crate::utils::cargo_bin() + .arg("decompress") + .arg(&archive) + .args(["--dir", "out"]) + .current_dir(dir) + .assert() + .success(); + + assert_eq!("first write", fs::read_to_string(&output_file).unwrap()); + + // Recreate archive, in case unpacking succeeds we'll see the new archive + // contents instead + fs::remove_file(&archive).unwrap(); + fs::remove_dir_all(&input_folder).unwrap(); + fs::create_dir(&input_folder).unwrap(); + fs::write(&input_file, "second write").unwrap(); + crate::utils::cargo_bin() + .arg("compress") + .arg(&input_folder) + .arg(&archive) + .assert() + .success(); + + // Decompressing again with `--dir out` — this fails due to directory + // conflict, we also didn't pass `--yes`. + crate::utils::cargo_bin() + .arg("decompress") + .arg(&archive) + .args(["--dir", "out"]) + .current_dir(dir) + .assert() + .failure(); + + assert_eq!("first write", fs::read_to_string(&output_file).unwrap()); + + // Third decompress attempt, now pass `--yes` to accept overwriting. + crate::utils::cargo_bin() + .arg("decompress") + .arg(&archive) + .args(["--dir", "out", "--yes"]) + .current_dir(dir) + .assert() + .success(); + + assert_eq!("second write", fs::read_to_string(&output_file).unwrap()); +} + +/// This test ensures the current behavior isn't modified by accident, even +/// if it's not the ideal behavior. +/// +/// Decompressing with `--dir .` succeeds without asking questions once. +/// +/// Non-ideal part: it also succeeds on the second call, without asking to +/// overwrite files, which is inconsistent with the behavior of `--dir` when +/// the given path is not `.` (or absolute path for CWD). +#[test] +fn decompress_dir_flag_current_dir_and_overwrite() { + let (_tempdir, dir) = testdir().unwrap(); + let input_folder = dir.join("folder"); + let input_file = input_folder.join("file"); + // Decompressing with `--dir .` means the file will be overwritten at the same path + let output_file = &input_file; + let archive = dir.join("archive.zip"); + + fs::create_dir(&input_folder).unwrap(); + fs::write(&input_file, "first write").unwrap(); + + crate::utils::cargo_bin() + .arg("compress") + .arg(&input_folder) + .arg(&archive) + .assert() + .success(); + + // First decompress with `--dir .` — should succeed + crate::utils::cargo_bin() + .arg("decompress") + .arg(&archive) + .args(["--dir", "."]) + .current_dir(dir) + .assert() + .success(); + + assert_eq!("first write", fs::read_to_string(output_file).unwrap()); + + // Recreate archive, in case unpacking succeeds we'll see the new archive + // contents instead + fs::remove_file(&archive).unwrap(); + fs::remove_dir_all(&input_folder).unwrap(); + fs::create_dir(&input_folder).unwrap(); + fs::write(&input_file, "second write").unwrap(); + crate::utils::cargo_bin() + .arg("compress") + .arg(&input_folder) + .arg(&archive) + .assert() + .success(); + + // Decompressing again with `--dir .` — this succeeds for now, and it's + // inconsistent with other `--dir PATH` usages, this test ensure this isn't + // changed by accident. + crate::utils::cargo_bin() + .arg("decompress") + .arg(&archive) + .args(["--dir", "."]) + .current_dir(dir) + .assert() + .success(); + + assert_eq!("second write", fs::read_to_string(output_file).unwrap()); +} + /// Test that concatenated lz4 frames are fully decompressed (related to issue #855) #[test] fn decompress_concatenated_lz4_frames() {