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 fatal error if read fails when compressing #297

Merged
merged 6 commits into from
Oct 16, 2022
Merged
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
17 changes: 10 additions & 7 deletions src/commands/compress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,16 @@ use crate::{
QuestionAction, QuestionPolicy, BUFFER_CAPACITY,
};

// Compress files into an `output_file`
//
// - `files`: is the list of paths to be compressed: ["dir/file1.txt", "dir/file2.txt"]
// - `extensions`: contains each compression format necessary for compressing, 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
/// Compress files into `output_file`.
///
/// # Arguments:
/// - `files`: is the list of paths to be compressed: ["dir/file1.txt", "dir/file2.txt"]
/// - `extensions`: is a list of compression formats for compressing, example: [Tar, Gz] (in compression order)
/// - `output_file` is the resulting compressed file name, example: "archive.tar.gz"
///
/// # Return value
/// - Returns `Ok(true)` if compressed all files normally.
/// - Returns `Ok(false)` if user opted to abort compression mid-way.
pub fn compress_files(
files: Vec<PathBuf>,
extensions: Vec<Extension>,
Expand Down
28 changes: 12 additions & 16 deletions src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use std::{
path::{Path, PathBuf},
};

use fs_err as fs;
use utils::colors;

use crate::{
Expand Down Expand Up @@ -183,22 +182,19 @@ pub fn run(
// 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) {
// If Ok(false) or Err() occurred, delete incomplete file at `output_path`
//
// if deleting fails, print an extra alert message pointing
// out that we left a possibly CORRUPTED file at `output_path`
if utils::remove_file_or_dir(&output_path).is_err() {
eprintln!("{red}FATAL ERROR:\n", red = *colors::RED);
eprintln!(" Please manually delete '{}'.", to_utf(&output_path));
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
);
eprintln!(" Ouch failed to delete the file '{}'.", to_utf(&output_path));
eprintln!(" Please delete it manually.");
eprintln!(" This file is corrupted if compression didn't finished.");

if compress_result.is_err() {
eprintln!(" Compression failed for reasons below.");
}
}
}

Expand Down
9 changes: 7 additions & 2 deletions src/utils/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,18 @@ pub fn clear_path(path: &Path, question_policy: QuestionPolicy) -> crate::Result
return Ok(false);
}

remove_file_or_dir(path)?;

Ok(true)
}

pub fn remove_file_or_dir(path: &Path) -> crate::Result<()> {
if path.is_dir() {
fs::remove_dir_all(path)?;
} else if path.is_file() {
fs::remove_file(path)?;
}

Ok(true)
Ok(())
}

/// Creates a directory at the path, if there is nothing there.
Expand Down
3 changes: 2 additions & 1 deletion src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ mod question;
pub use file_visibility::FileVisibilityPolicy;
pub use formatting::{nice_directory_display, pretty_format_list_of_paths, strip_cur_dir, to_utf};
pub use fs::{
cd_into_same_dir_as, clear_path, create_dir_if_non_existent, dir_is_empty, is_symlink, try_infer_extension,
cd_into_same_dir_as, clear_path, create_dir_if_non_existent, dir_is_empty, is_symlink, remove_file_or_dir,
try_infer_extension,
};
pub use question::{
ask_to_create_file, user_wants_to_continue, user_wants_to_overwrite, QuestionAction, QuestionPolicy,
Expand Down
6 changes: 2 additions & 4 deletions src/utils/question.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use super::{strip_cur_dir, to_utf};
use crate::{
accessible::is_running_in_accessible_mode,
error::{Error, Result},
utils::colors,
utils::{self, colors},
};

#[derive(Debug, PartialEq, Eq, Clone, Copy)]
Expand Down Expand Up @@ -59,9 +59,7 @@ pub fn ask_to_create_file(path: &Path, question_policy: QuestionPolicy) -> Resul
Ok(w) => Ok(Some(w)),
Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {
if user_wants_to_overwrite(path, question_policy)? {
if path.is_dir() {
fs::remove_dir_all(path)?;
}
utils::remove_file_or_dir(path)?;
Ok(Some(fs::File::create(path)?))
} else {
Ok(None)
Expand Down