Merge pull request #8567 from sylvestre/rm-progress

rm: add the --progress option like with cp & mv
This commit is contained in:
Daniel Hofstetter 2025-10-22 11:14:14 +02:00 committed by GitHub
commit e2634e5c48
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 234 additions and 22 deletions

1
Cargo.lock generated
View file

@ -3880,6 +3880,7 @@ dependencies = [
"clap",
"codspeed-divan-compat",
"fluent",
"indicatif",
"libc",
"tempfile",
"thiserror 2.0.17",

View file

@ -43,6 +43,10 @@ packages.
`mv` can display a progress bar when the `-g`/`--progress` flag is set.
## `rm`
`rm` can display a progress bar when the `-g`/`--progress` flag is set.
## `hashsum`
This utility does not exist in GNU coreutils. `hashsum` is a utility that

View file

@ -22,6 +22,7 @@ thiserror = { workspace = true }
clap = { workspace = true }
uucore = { workspace = true, features = ["fs", "parser", "safe-traversal"] }
fluent = { workspace = true }
indicatif = { workspace = true }
[target.'cfg(unix)'.dependencies]
libc = { workspace = true }

View file

@ -28,6 +28,10 @@ rm-help-preserve-root = do not remove '/' (default)
rm-help-recursive = remove directories and their contents recursively
rm-help-dir = remove empty directories
rm-help-verbose = explain what is being done
rm-help-progress = display a progress bar. Note: this feature is not supported by GNU coreutils.
# Progress messages
rm-progress-removing = Removing
# Error messages
rm-error-missing-operand = missing operand

View file

@ -28,6 +28,10 @@ rm-help-preserve-root = ne pas supprimer '/' (par défaut)
rm-help-recursive = supprimer les répertoires et leur contenu récursivement
rm-help-dir = supprimer les répertoires vides
rm-help-verbose = expliquer ce qui est fait
rm-help-progress = afficher une barre de progression. Note : cette fonctionnalité n'est pas supportée par GNU coreutils.
# Messages de progression
rm-progress-removing = Suppression
# Messages d'erreur
rm-error-missing-operand = opérande manquant

View file

@ -7,6 +7,7 @@
// spell-checker:ignore fstatat unlinkat
use indicatif::ProgressBar;
use std::ffi::OsStr;
use std::fs;
use std::path::Path;
@ -28,7 +29,11 @@ pub fn is_readable(path: &Path) -> bool {
}
/// Remove a single file using safe traversal
pub fn safe_remove_file(path: &Path, options: &Options) -> Option<bool> {
pub fn safe_remove_file(
path: &Path,
options: &Options,
progress_bar: Option<&ProgressBar>,
) -> Option<bool> {
let parent = path.parent()?;
let file_name = path.file_name()?;
@ -36,6 +41,10 @@ pub fn safe_remove_file(path: &Path, options: &Options) -> Option<bool> {
match dir_fd.unlink_at(file_name, false) {
Ok(_) => {
// Update progress bar for file removal
if let Some(pb) = progress_bar {
pb.inc(1);
}
verbose_removed_file(path, options);
Some(false)
}
@ -51,7 +60,11 @@ pub fn safe_remove_file(path: &Path, options: &Options) -> Option<bool> {
}
/// Remove an empty directory using safe traversal
pub fn safe_remove_empty_dir(path: &Path, options: &Options) -> Option<bool> {
pub fn safe_remove_empty_dir(
path: &Path,
options: &Options,
progress_bar: Option<&ProgressBar>,
) -> Option<bool> {
let parent = path.parent()?;
let dir_name = path.file_name()?;
@ -59,6 +72,10 @@ pub fn safe_remove_empty_dir(path: &Path, options: &Options) -> Option<bool> {
match dir_fd.unlink_at(dir_name, true) {
Ok(_) => {
// Update progress bar for directory removal
if let Some(pb) = progress_bar {
pb.inc(1);
}
verbose_removed_directory(path, options);
Some(false)
}
@ -172,12 +189,16 @@ pub fn remove_dir_with_special_cases(path: &Path, options: &Options, error_occur
}
}
pub fn safe_remove_dir_recursive(path: &Path, options: &Options) -> bool {
pub fn safe_remove_dir_recursive(
path: &Path,
options: &Options,
progress_bar: Option<&ProgressBar>,
) -> bool {
// Base case 1: this is a file or a symbolic link.
// Use lstat to avoid race condition between check and use
match fs::symlink_metadata(path) {
Ok(metadata) if !metadata.is_dir() => {
return remove_file(path, options);
return remove_file(path, options, progress_bar);
}
Ok(_) => {}
Err(e) => {

View file

@ -7,9 +7,10 @@
use clap::builder::{PossibleValue, ValueParser};
use clap::{Arg, ArgAction, Command, parser::ValueSource};
use indicatif::{ProgressBar, ProgressStyle};
use std::ffi::{OsStr, OsString};
use std::fs::{self, Metadata};
use std::io::{IsTerminal, stdin};
use std::io::{self, IsTerminal, stdin};
use std::ops::BitOr;
#[cfg(unix)]
use std::os::unix::ffi::OsStrExt;
@ -70,7 +71,7 @@ fn verbose_removed_directory(path: &Path, options: &Options) {
/// Helper function to show error with context and return error status
fn show_removal_error(error: std::io::Error, path: &Path) -> bool {
if error.kind() == std::io::ErrorKind::PermissionDenied {
if error.kind() == io::ErrorKind::PermissionDenied {
show_error!("cannot remove {}: Permission denied", path.quote());
} else {
let e =
@ -158,6 +159,8 @@ pub struct Options {
pub dir: bool,
/// `-v`, `--verbose`
pub verbose: bool,
/// `-g`, `--progress`
pub progress: bool,
#[doc(hidden)]
/// `---presume-input-tty`
/// Always use `None`; GNU flag for testing use only
@ -174,6 +177,7 @@ impl Default for Options {
recursive: false,
dir: false,
verbose: false,
progress: false,
__presume_input_tty: None,
}
}
@ -189,6 +193,7 @@ static OPT_PROMPT_ALWAYS: &str = "prompt-always";
static OPT_PROMPT_ONCE: &str = "prompt-once";
static OPT_RECURSIVE: &str = "recursive";
static OPT_VERBOSE: &str = "verbose";
static OPT_PROGRESS: &str = "progress";
static PRESUME_INPUT_TTY: &str = "-presume-input-tty";
static ARG_FILES: &str = "files";
@ -241,6 +246,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
recursive: matches.get_flag(OPT_RECURSIVE),
dir: matches.get_flag(OPT_DIR),
verbose: matches.get_flag(OPT_VERBOSE),
progress: matches.get_flag(OPT_PROGRESS),
__presume_input_tty: if matches.get_flag(PRESUME_INPUT_TTY) {
Some(true)
} else {
@ -359,6 +365,13 @@ pub fn uu_app() -> Command {
.help(translate!("rm-help-verbose"))
.action(ArgAction::SetTrue),
)
.arg(
Arg::new(OPT_PROGRESS)
.short('g')
.long(OPT_PROGRESS)
.help(translate!("rm-help-progress"))
.action(ArgAction::SetTrue),
)
// From the GNU source code:
// This is solely for testing.
// Do not document.
@ -383,6 +396,70 @@ pub fn uu_app() -> Command {
)
}
/// Creates a progress bar for rm operations if conditions are met.
/// Returns Some(ProgressBar) if `total_files` > 0, None otherwise.
fn create_progress_bar(files: &[&OsStr], recursive: bool) -> Option<ProgressBar> {
let total_files = count_files(files, recursive);
if total_files == 0 {
return None;
}
Some(
ProgressBar::new(total_files)
.with_style(
ProgressStyle::with_template(
"{msg}: [{elapsed_precise}] {wide_bar} {pos:>7}/{len:7} files",
)
.unwrap(),
)
.with_message(translate!("rm-progress-removing")),
)
}
/// Count the total number of files and directories to be deleted.
/// This function recursively counts all files and directories that will be processed.
/// Files are not deduplicated when appearing in multiple sources. If `recursive` is set to `false`, the
/// directories in `paths` will be ignored.
fn count_files(paths: &[&OsStr], recursive: bool) -> u64 {
let mut total = 0;
for p in paths {
let path = Path::new(p);
if let Ok(md) = fs::symlink_metadata(path) {
if md.is_dir() && !is_symlink_dir(&md) {
if recursive {
total += count_files_in_directory(path);
}
} else {
total += 1;
}
}
// If we can't access the file, skip it for counting
// This matches the behavior where -f suppresses errors for missing files
}
total
}
/// A helper for `count_files` specialized for directories.
fn count_files_in_directory(p: &Path) -> u64 {
let entries_count = fs::read_dir(p)
.map(|entries| {
entries
.filter_map(Result::ok)
.map(|entry| {
let file_type = entry.file_type();
match file_type {
Ok(ft) if ft.is_dir() => count_files_in_directory(&entry.path()),
Ok(_) => 1,
Err(_) => 0,
}
})
.sum()
})
.unwrap_or(0);
1 + entries_count
}
// TODO: implement one-file-system (this may get partially implemented in walkdir)
/// Remove (or unlink) the given files
///
@ -393,17 +470,27 @@ pub fn uu_app() -> Command {
pub fn remove(files: &[&OsStr], options: &Options) -> bool {
let mut had_err = false;
// Check if any files actually exist before creating progress bar
let mut progress_bar: Option<ProgressBar> = None;
let mut any_files_processed = false;
for filename in files {
let file = Path::new(filename);
had_err = match file.symlink_metadata() {
Ok(metadata) => {
// Create progress bar on first successful file metadata read
if options.progress && progress_bar.is_none() {
progress_bar = create_progress_bar(files, options.recursive);
}
any_files_processed = true;
if metadata.is_dir() {
handle_dir(file, options)
handle_dir(file, options, progress_bar.as_ref())
} else if is_symlink_dir(&metadata) {
remove_dir(file, options)
remove_dir(file, options, progress_bar.as_ref())
} else {
remove_file(file, options)
remove_file(file, options, progress_bar.as_ref())
}
}
@ -426,6 +513,13 @@ pub fn remove(files: &[&OsStr], options: &Options) -> bool {
.bitor(had_err);
}
// Only finish progress bar if it was created and files were processed
if let Some(pb) = progress_bar {
if any_files_processed {
pb.finish();
}
}
had_err
}
@ -487,7 +581,11 @@ fn is_writable(_path: &Path) -> bool {
/// directory, remove all of its entries recursively and then remove the
/// directory itself. In case of an error, print the error message to
/// `stderr` and return `true`. If there were no errors, return `false`.
fn remove_dir_recursive(path: &Path, options: &Options) -> bool {
fn remove_dir_recursive(
path: &Path,
options: &Options,
progress_bar: Option<&ProgressBar>,
) -> bool {
// Base case 1: this is a file or a symbolic link.
//
// The symbolic link case is important because it could be a link to
@ -495,7 +593,7 @@ fn remove_dir_recursive(path: &Path, options: &Options) -> bool {
// avoids an infinite recursion in the case of a link to the current
// directory, like `ln -s . link`.
if !path.is_dir() || path.is_symlink() {
return remove_file(path, options);
return remove_file(path, options, progress_bar);
}
// Base case 2: this is a non-empty directory, but the user
@ -510,7 +608,7 @@ fn remove_dir_recursive(path: &Path, options: &Options) -> bool {
// Use secure traversal on Linux for all recursive directory removals
#[cfg(target_os = "linux")]
{
safe_remove_dir_recursive(path, options)
safe_remove_dir_recursive(path, options, progress_bar)
}
// Fallback for non-Linux or use fs::remove_dir_all for very long paths
@ -534,7 +632,7 @@ fn remove_dir_recursive(path: &Path, options: &Options) -> bool {
// Recursive case: this is a directory.
let mut error = false;
match fs::read_dir(path) {
Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => {
Err(e) if e.kind() == io::ErrorKind::PermissionDenied => {
// This is not considered an error.
}
Err(_) => error = true,
@ -543,7 +641,8 @@ fn remove_dir_recursive(path: &Path, options: &Options) -> bool {
match entry {
Err(_) => error = true,
Ok(entry) => {
let child_error = remove_dir_recursive(&entry.path(), options);
let child_error =
remove_dir_recursive(&entry.path(), options, progress_bar);
error = error || child_error;
}
}
@ -585,7 +684,7 @@ fn remove_dir_recursive(path: &Path, options: &Options) -> bool {
}
}
fn handle_dir(path: &Path, options: &Options) -> bool {
fn handle_dir(path: &Path, options: &Options, progress_bar: Option<&ProgressBar>) -> bool {
let mut had_err = false;
let path = clean_trailing_slashes(path);
@ -599,9 +698,9 @@ fn handle_dir(path: &Path, options: &Options) -> bool {
let is_root = path.has_root() && path.parent().is_none();
if options.recursive && (!is_root || !options.preserve_root) {
had_err = remove_dir_recursive(path, options);
had_err = remove_dir_recursive(path, options, progress_bar);
} else if options.dir && (!is_root || !options.preserve_root) {
had_err = remove_dir(path, options).bitor(had_err);
had_err = remove_dir(path, options, progress_bar).bitor(had_err);
} else if options.recursive {
show_error!("{}", RmError::DangerousRecursiveOperation);
show_error!("{}", RmError::UseNoPreserveRoot);
@ -620,7 +719,7 @@ fn handle_dir(path: &Path, options: &Options) -> bool {
/// Remove the given directory, asking the user for permission if necessary.
///
/// Returns true if it has encountered an error.
fn remove_dir(path: &Path, options: &Options) -> bool {
fn remove_dir(path: &Path, options: &Options, progress_bar: Option<&ProgressBar>) -> bool {
// Ask the user for permission.
if !prompt_dir(path, options) {
return false;
@ -638,21 +737,31 @@ fn remove_dir(path: &Path, options: &Options) -> bool {
// Use safe traversal on Linux for empty directory removal
#[cfg(target_os = "linux")]
{
if let Some(result) = safe_remove_empty_dir(path, options) {
if let Some(result) = safe_remove_empty_dir(path, options, progress_bar) {
return result;
}
}
// Update progress bar for directory removal
if let Some(pb) = progress_bar {
pb.inc(1);
}
// Fallback method for non-Linux or when safe traversal is unavailable
remove_dir_with_feedback(path, options)
}
fn remove_file(path: &Path, options: &Options) -> bool {
fn remove_file(path: &Path, options: &Options, progress_bar: Option<&ProgressBar>) -> bool {
if prompt_file(path, options) {
// Update progress bar before removing the file
if let Some(pb) = progress_bar {
pb.inc(1);
}
// Use safe traversal on Linux for individual file removal
#[cfg(target_os = "linux")]
{
if let Some(result) = safe_remove_file(path, options) {
if let Some(result) = safe_remove_file(path, options, progress_bar) {
return result;
}
}
@ -663,7 +772,7 @@ fn remove_file(path: &Path, options: &Options) -> bool {
verbose_removed_file(path, options);
}
Err(e) => {
if e.kind() == std::io::ErrorKind::PermissionDenied {
if e.kind() == io::ErrorKind::PermissionDenied {
// GNU compatibility (rm/fail-eacces.sh)
show_error!(
"{}",

View file

@ -1149,3 +1149,71 @@ fn test_rm_directory_not_writable() {
assert!(!at.dir_exists("b/c")); // Should be removed
assert!(!at.dir_exists("b/d")); // Should be removed
}
#[test]
fn test_progress_flag_short() {
let (at, mut ucmd) = at_and_ucmd!();
let file = "test_rm_progress_file";
at.touch(file);
// Test that -g flag is accepted
ucmd.arg("-g").arg(file).succeeds();
assert!(!at.file_exists(file));
}
#[test]
fn test_progress_flag_long() {
let (at, mut ucmd) = at_and_ucmd!();
let file = "test_rm_progress_file";
at.touch(file);
// Test that --progress flag is accepted
ucmd.arg("--progress").arg(file).succeeds();
assert!(!at.file_exists(file));
}
#[test]
fn test_progress_with_recursive() {
let (at, mut ucmd) = at_and_ucmd!();
at.mkdir("test_dir");
at.touch("test_dir/file1");
at.touch("test_dir/file2");
at.mkdir("test_dir/subdir");
at.touch("test_dir/subdir/file3");
// Test progress with recursive removal
ucmd.arg("-rg").arg("test_dir").succeeds();
assert!(!at.dir_exists("test_dir"));
}
#[test]
fn test_progress_with_verbose() {
let (at, mut ucmd) = at_and_ucmd!();
let file = "test_rm_progress_verbose_file";
at.touch(file);
// Test that progress and verbose work together
ucmd.arg("-gv").arg(file).succeeds().stdout_contains(file);
assert!(!at.file_exists(file));
}
#[test]
fn test_progress_no_output_on_error() {
let nonexistent_file = "this_file_does_not_exist";
// Test that progress bar is not shown when file doesn't exist
new_ucmd!()
.arg("--progress")
.arg(nonexistent_file)
.fails()
.stderr_contains("cannot remove")
.stderr_contains("No such file or directory");
}