From 9907ebcf364049fcab6ef038ec4d72ab1fcc2671 Mon Sep 17 00:00:00 2001 From: Spyros Roum Date: Fri, 15 Oct 2021 00:33:17 +0300 Subject: [PATCH 1/5] Properly detect if we are compressing a partially compressed file --- src/commands.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/commands.rs b/src/commands.rs index 05db7a637..182261e77 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -163,12 +163,24 @@ pub fn run(command: Command, flags: &oof::Flags) -> crate::Result<()> { fn compress_files( files: Vec, - formats: Vec, + mut formats: Vec, output_file: fs::File, _flags: &oof::Flags, ) -> crate::Result<()> { let file_writer = BufWriter::with_capacity(BUFFER_CAPACITY, output_file); + if files.len() == 1 { + // It's possible the file is already partially compressed so we don't want to compress it again + // `ouch compress file.tar.gz file.tar.gz.xz` should produce `file.tar.gz.xz` and not `file.tar.gz.tar.gz.xz` + let cur_extensions = extension::extensions_from_path(&files[0]); + + // If the input is a subset at the start of `formats` then remove the extensions + if cur_extensions.len() < formats.len() && cur_extensions.iter().zip(&formats).all(|(inp, out)| inp == out) { + let drain_iter = formats.drain(..cur_extensions.len()); + drop(drain_iter); // Remove the extensions from `formats` + } + } + if let [Tar | Tgz | Zip] = *formats.as_slice() { match formats[0] { Tar => { From 123ccddd91cd508512752007dbc3e5e3f488ab4a Mon Sep 17 00:00:00 2001 From: Spyros Roum Date: Fri, 15 Oct 2021 02:44:15 +0300 Subject: [PATCH 2/5] Move the check to `run` function --- src/commands.rs | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/src/commands.rs b/src/commands.rs index 182261e77..ae46e5aa3 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -41,7 +41,7 @@ pub fn run(command: Command, flags: &oof::Flags) -> crate::Result<()> { match command { Command::Compress { files, output_path } => { // Formats from path extension, like "file.tar.gz.xz" -> vec![Tar, Gzip, Lzma] - let formats = extension::extensions_from_path(&output_path); + let mut formats = extension::extensions_from_path(&output_path); if formats.is_empty() { let reason = FinalError::with_title(format!("Cannot compress to '{}'.", to_utf(&output_path))) @@ -57,7 +57,7 @@ pub fn run(command: Command, flags: &oof::Flags) -> crate::Result<()> { } if matches!(&formats[0], Bzip | Gzip | Lzma) && represents_several_files(&files) { - // This piece of code creates a sugestion for compressing multiple files + // This piece of code creates a suggestion for compressing multiple files // It says: // Change from file.bz.xz // To file.tar.bz.xz @@ -102,6 +102,22 @@ pub fn run(command: Command, flags: &oof::Flags) -> crate::Result<()> { } let output_file = fs::File::create(&output_path)?; + + if files.len() == 1 { + // It's possible the file is already partially compressed so we don't want to compress it again + // `ouch compress file.tar.gz file.tar.gz.xz` should produce `file.tar.gz.xz` and not `file.tar.gz.tar.gz.xz` + let input_extensions = extension::extensions_from_path(&files[0]); + + // If the input is a sublist at the start of `formats` then remove the extensions + // Note: If input_extensions is empty this counts as true + if !input_extensions.is_empty() + && input_extensions.len() < formats.len() + && input_extensions.iter().zip(&formats).all(|(inp, out)| inp == out) + { + let drain_iter = formats.drain(..input_extensions.len()); + drop(drain_iter); // Remove the extensions from `formats` + } + } let compress_result = compress_files(files, formats, output_file, flags); // If any error occurred, delete incomplete file @@ -163,24 +179,12 @@ pub fn run(command: Command, flags: &oof::Flags) -> crate::Result<()> { fn compress_files( files: Vec, - mut formats: Vec, + formats: Vec, output_file: fs::File, _flags: &oof::Flags, ) -> crate::Result<()> { let file_writer = BufWriter::with_capacity(BUFFER_CAPACITY, output_file); - if files.len() == 1 { - // It's possible the file is already partially compressed so we don't want to compress it again - // `ouch compress file.tar.gz file.tar.gz.xz` should produce `file.tar.gz.xz` and not `file.tar.gz.tar.gz.xz` - let cur_extensions = extension::extensions_from_path(&files[0]); - - // If the input is a subset at the start of `formats` then remove the extensions - if cur_extensions.len() < formats.len() && cur_extensions.iter().zip(&formats).all(|(inp, out)| inp == out) { - let drain_iter = formats.drain(..cur_extensions.len()); - drop(drain_iter); // Remove the extensions from `formats` - } - } - if let [Tar | Tgz | Zip] = *formats.as_slice() { match formats[0] { Tar => { From aa65743e4ebdfe8c5b96aaef53bdafd4715e81df Mon Sep 17 00:00:00 2001 From: Spyros Roum Date: Fri, 15 Oct 2021 02:44:34 +0300 Subject: [PATCH 3/5] Add some `info!` for the user --- src/commands.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/commands.rs b/src/commands.rs index ae46e5aa3..1e221efcb 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -114,6 +114,11 @@ pub fn run(command: Command, flags: &oof::Flags) -> crate::Result<()> { && input_extensions.len() < formats.len() && input_extensions.iter().zip(&formats).all(|(inp, out)| inp == out) { + info!( + "Partial compression detected. Compressing {} into {}", + to_utf(files[0].as_path().file_name().unwrap()), + to_utf(&output_path) + ); let drain_iter = formats.drain(..input_extensions.len()); drop(drain_iter); // Remove the extensions from `formats` } From baf23fa685922bf7a16d3ace76b43ce763914e1b Mon Sep 17 00:00:00 2001 From: Spyros Roum Date: Fri, 15 Oct 2021 02:45:07 +0300 Subject: [PATCH 4/5] Use `represents_several_files` instead of checking len of `files` --- src/commands.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands.rs b/src/commands.rs index 1e221efcb..32c60fc6d 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -103,7 +103,7 @@ pub fn run(command: Command, flags: &oof::Flags) -> crate::Result<()> { let output_file = fs::File::create(&output_path)?; - if files.len() == 1 { + if !represents_several_files(&files) { // It's possible the file is already partially compressed so we don't want to compress it again // `ouch compress file.tar.gz file.tar.gz.xz` should produce `file.tar.gz.xz` and not `file.tar.gz.tar.gz.xz` let input_extensions = extension::extensions_from_path(&files[0]); From 05f82d3aee2659254d99af7869ec85dc0210d3e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20M=2E=20Bezerra?= Date: Fri, 15 Oct 2021 05:32:44 -0300 Subject: [PATCH 5/5] Adding unwrap safety for file_name --- src/commands.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/commands.rs b/src/commands.rs index 32c60fc6d..fa90b0164 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -114,6 +114,11 @@ pub fn run(command: Command, flags: &oof::Flags) -> crate::Result<()> { && input_extensions.len() < formats.len() && input_extensions.iter().zip(&formats).all(|(inp, out)| inp == out) { + // Safety: + // We checked above that input_extensions isn't empty, so files[0] has a extension. + // + // Path::extension says: "if there is no file_name, then there is no extension". + // Using DeMorgan's law: "if there is extension, then there is file_name". info!( "Partial compression detected. Compressing {} into {}", to_utf(files[0].as_path().file_name().unwrap()),