Skip to content
Open
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ Categories Used:

## [Unreleased](https://github.com/ouch-org/ouch/compare/0.8.1...HEAD)

### Bug Fixes

- Decompress: prevent directory conflict resolution from deleting unrelated destination contents (https://github.com/ouch-org/ouch/issues/1043)

### Tweaks

- Releases: sign assets with cosign instead of GitHub artifact attestations
Expand Down
9 changes: 9 additions & 0 deletions src/utils/question.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,15 @@ pub fn prompt_user_for_file_conflict_resolution(
],
)
.ask(),
QuestionAction::Decompression if path.is_dir() => ChoicePrompt::new(
format!("Handle file conflict for {}:", PathFmt(path)),
[
("rename", Op::Rename, *colors::BLUE),
("merge", Op::Merge, *colors::ORANGE),
("skip", Op::Cancel, *colors::RED),
],
)
.ask(),
QuestionAction::Decompression => ChoicePrompt::new(
format!("Handle file conflict for {}:", PathFmt(path)),
[
Expand Down
79 changes: 74 additions & 5 deletions tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ fn multiple_files(
}

#[proptest(cases = 25)]
fn multiple_files_with_conflict_and_choice_to_overwrite(
fn multiple_files_with_conflict_and_choice_to_merge(
ext: DirectoryExtension,
#[any(size_range(0..1).lift())] extra_extensions: Vec<FileExtension>,
#[strategy(0u8..3)] depth: u8,
Expand All @@ -243,9 +243,9 @@ fn multiple_files_with_conflict_and_choice_to_overwrite(
create_random_files(before_dir, depth, &mut SmallRng::from_os_rng());

let after = &dir.join("after");
let after_dir = &after.join("dir");
fs::create_dir_all(after_dir).unwrap();
create_random_files(after_dir, depth, &mut SmallRng::from_os_rng());
fs::create_dir_all(after).unwrap();
let unrelated_file = after.join("unrelated.txt");
fs::write(&unrelated_file, "keep this").unwrap();

let archive = &dir.join(format!("archive.{}", merge_extensions(ext, &extra_extensions)));
ouch!("-A", "c", before_dir, archive);
Expand All @@ -255,10 +255,12 @@ fn multiple_files_with_conflict_and_choice_to_overwrite(
.arg(archive)
.arg("-d")
.arg(after)
.write_stdin("o")
.write_stdin("m")
.assert()
.success();

assert_eq!("keep this", fs::read_to_string(&unrelated_file).unwrap());
fs::remove_file(unrelated_file).unwrap();
assert_same_directory(before, after, false);
}

Expand Down Expand Up @@ -1255,6 +1257,47 @@ fn test_concatenated_streams(extension: &str, compress_chunk: impl Fn(&[u8]) ->
);
}

/// Directory conflicts during decompression must not offer the destructive overwrite action.
/// An invalid overwrite choice should be ignored, allowing the user to choose merge instead.
#[test]
fn decompress_directory_conflict_preserves_unrelated_contents() {
let (_tempdir, dir) = testdir().unwrap();
let input_folder = dir.join("folder");
let archive = dir.join("archive.zip");
let output_dir = dir.join("out");

fs::create_dir(&input_folder).unwrap();
fs::write(input_folder.join("file"), "archive content").unwrap();
crate::utils::cargo_bin()
.arg("compress")
.arg(&input_folder)
.arg(&archive)
.assert()
.success();

fs::create_dir(&output_dir).unwrap();
fs::write(output_dir.join("important.txt"), "keep this").unwrap();

crate::utils::cargo_bin()
.arg("decompress")
.arg(&archive)
.arg("--dir")
.arg(&output_dir)
.write_stdin("o\nm\n")
.assert()
.success();

assert_eq!(
"keep this",
fs::read_to_string(output_dir.join("important.txt")).unwrap(),
"choosing overwrite removed unrelated destination contents"
);
assert_eq!(
"archive content",
fs::read_to_string(output_dir.join("folder").join("file")).unwrap()
);
}

/// Regression test: `--yes` should merge into a non-empty output directory rather than wiping it.
/// Previously, `--yes` defaulted to `Overwrite`, which would call `remove_dir_all` on the output
/// directory, including when that directory was `$CWD`.
Expand Down Expand Up @@ -1517,6 +1560,32 @@ fn decompress_single_file_dir_allows_non_empty_output_dir_with_no() {
assert_eq!("keep", fs::read_to_string(dir.join("out").join("other-file")).unwrap());
}

/// Overwrite remains available when decompression conflicts with an actual file.
#[test]
fn decompress_single_file_conflict_can_be_overwritten() {
let (_tempdir, dir) = testdir().unwrap();

fs::write(dir.join("a"), "new content").unwrap();
crate::utils::cargo_bin()
.args(["compress", "a", "a.gz"])
.current_dir(dir)
.assert()
.success();
fs::remove_file(dir.join("a")).unwrap();

fs::create_dir(dir.join("out")).unwrap();
fs::write(dir.join("out").join("a"), "old content").unwrap();

crate::utils::cargo_bin()
.args(["decompress", "a.gz", "--dir", "out"])
.current_dir(dir)
.write_stdin("o\n")
.assert()
.success();

assert_eq!("new content", fs::read_to_string(dir.join("out").join("a")).unwrap());
}

/// This test ensures the current behavior isn't modified by accident, even
/// if it's not the ideal behavior.
///
Expand Down