Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix warning for zip with additional formats #239

Merged
merged 6 commits into from
Mar 3, 2022
Merged
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
80 changes: 42 additions & 38 deletions src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@ use crate::{
warning, Opts, QuestionAction, QuestionPolicy, Subcommand,
};

// Message used to advice the user that .zip archives have limitations that require it to load everything into memory at once
// and this can lead to out-of-memory scenarios for archives that are big enough.
const ZIP_IN_MEMORY_LIMITATION_WARNING: &str =
"\tThere is a limitation for .zip archives with extra extensions. (e.g. <file>.zip.gz)
\tThe design of .zip makes it impossible to compress via stream, so it must be done entirely in memory.
\tBy compressing .zip with extra compression formats, you can run out of RAM if the file is too large!";

// Used in BufReader and BufWriter to perform less syscalls
const BUFFER_CAPACITY: usize = 1024 * 64;

Expand Down Expand Up @@ -163,8 +170,14 @@ pub fn run(
let compress_result =
compress_files(files, formats, output_file, &output_path, question_policy, file_visibility_policy);

// If any error occurred, delete incomplete file
if compress_result.is_err() {
if let Ok(true) = compress_result {
// this is only printed once, so it doesn't result in much text. On the other hand,
// having a final status message is important especially in an accessibility context
// as screen readers may not read a commands exit code, making it hard to reason
// about whether the command succeeded without such a message
info!(accessible, "Successfully compressed '{}'.", to_utf(&output_path));
} else {
// If Ok(false) or Err() occurred, delete incomplete file
// Print an extra alert message pointing out that we left a possibly
// CORRUPTED FILE at `output_path`
if let Err(err) = fs::remove_file(&output_path) {
Expand All @@ -173,12 +186,6 @@ pub fn run(
eprintln!(" Compression failed and we could not delete '{}'.", to_utf(&output_path),);
eprintln!(" Error:{reset} {}{red}.{reset}\n", err, reset = *colors::RESET, red = *colors::RED);
}
} else {
// this is only printed once, so it doesn't result in much text. On the other hand,
// having a final status message is important especially in an accessibility context
// as screen readers may not read a commands exit code, making it hard to reason
// about whether the command succeeded without such a message
info!(accessible, "Successfully compressed '{}'.", to_utf(&output_path));
}

compress_result?;
Expand Down Expand Up @@ -284,21 +291,24 @@ pub fn run(
// files are the list of paths to be compressed: ["dir/file1.txt", "dir/file2.txt"]
// formats contains each format necessary for compression, example: [Tar, Gz] (in compression order)
// output_file is the resulting compressed file name, example: "compressed.tar.gz"
//
// Returns Ok(true) if compressed all files successfully, and Ok(false) if user opted to skip files
fn compress_files(
files: Vec<PathBuf>,
formats: Vec<Extension>,
output_file: fs::File,
output_dir: &Path,
question_policy: QuestionPolicy,
file_visibility_policy: FileVisibilityPolicy,
) -> crate::Result<()> {
) -> crate::Result<bool> {
// The next lines are for displaying the progress bar
// If the input files contain a directory, then the total size will be underestimated
let (total_input_size, precise) = files
.iter()
.map(|f| (f.metadata().expect("file exists").len(), f.is_file()))
.fold((0, true), |(total_size, and_precise), (size, precise)| (total_size + size, and_precise & precise));
//NOTE: canonicalize is here to avoid a weird bug:

// NOTE: canonicalize is here to avoid a weird bug:
// > If output_file_path is a nested path and it exists and the user overwrite it
// >> output_file_path.exists() will always return false (somehow)
// - canonicalize seems to fix this
Expand Down Expand Up @@ -360,16 +370,14 @@ fn compress_files(
writer.flush()?;
}
Zip => {
eprintln!("{orange}[WARNING]{reset}", orange = *colors::ORANGE, reset = *colors::RESET);
eprintln!(
"\tThere is a limitation for .zip archives with extra extensions. (e.g. <file>.zip.gz)\
\n\tThe design of .zip makes it impossible to compress via stream, so it must be done entirely in memory.\
\n\tBy compressing .zip with extra compression formats, you can run out of RAM if the file is too large!"
);
if formats.len() > 1 {
eprintln!("{orange}[WARNING]{reset}", orange = *colors::ORANGE, reset = *colors::RESET);
eprintln!("{}", ZIP_IN_MEMORY_LIMITATION_WARNING);

// give user the option to continue compressing after warning is shown
if !user_wants_to_continue(output_dir, question_policy, QuestionAction::Compression)? {
return Ok(());
// give user the option to continue compressing after warning is shown
if !user_wants_to_continue(output_dir, question_policy, QuestionAction::Compression)? {
return Ok(false);
}
}

let mut vec_buffer = io::Cursor::new(vec![]);
Expand Down Expand Up @@ -400,7 +408,7 @@ fn compress_files(
}
}

Ok(())
Ok(true)
}

// Decompress a file
Expand Down Expand Up @@ -523,16 +531,14 @@ fn decompress_file(
};
}
Zip => {
eprintln!("{orange}[WARNING]{reset}", orange = *colors::ORANGE, reset = *colors::RESET);
eprintln!(
"\tThere is a limitation for .zip archives with extra extensions. (e.g. <file>.zip.gz)\
\n\tThe design of .zip makes it impossible to compress via stream, so it must be done entirely in memory.\
\n\tBy compressing .zip with extra compression formats, you can run out of RAM if the file is too large!"
);
if formats.len() > 1 {
eprintln!("{orange}[WARNING]{reset}", orange = *colors::ORANGE, reset = *colors::RESET);
eprintln!("{}", ZIP_IN_MEMORY_LIMITATION_WARNING);

// give user the option to continue decompressing after warning is shown
if !user_wants_to_continue(input_file_path, question_policy, QuestionAction::Decompression)? {
return Ok(());
// give user the option to continue decompressing after warning is shown
if !user_wants_to_continue(input_file_path, question_policy, QuestionAction::Decompression)? {
return Ok(());
}
}

let mut vec = vec![];
Expand Down Expand Up @@ -620,16 +626,14 @@ fn list_archive_contents(
let files: Box<dyn Iterator<Item = crate::Result<FileInArchive>>> = match formats[0] {
Tar => Box::new(crate::archive::tar::list_archive(tar::Archive::new(reader))),
Zip => {
eprintln!("{orange}[WARNING]{reset}", orange = *colors::ORANGE, reset = *colors::RESET);
eprintln!(
"\tThere is a limitation for .zip archives with extra extensions. (e.g. <file>.zip.gz)\
\n\tThe design of .zip makes it impossible to compress via stream, so it must be done entirely in memory.\
\n\tBy compressing .zip with extra compression formats, you can run out of RAM if the file is too large!"
);
if formats.len() > 1 {
eprintln!("{orange}[WARNING]{reset}", orange = *colors::ORANGE, reset = *colors::RESET);
eprintln!("{}", ZIP_IN_MEMORY_LIMITATION_WARNING);

// give user the option to continue decompressing after warning is shown
if !user_wants_to_continue(archive_path, question_policy, QuestionAction::Decompression)? {
return Ok(());
// give user the option to continue decompressing after warning is shown
if !user_wants_to_continue(archive_path, question_policy, QuestionAction::Decompression)? {
return Ok(());
}
}

let mut vec = vec![];
Expand Down