From 9581fcf688c175673ebd4f51341469aead8bf949 Mon Sep 17 00:00:00 2001 From: Marvin Hofmann <644950+reggaemuffin@users.noreply.github.com> Date: Mon, 5 Apr 2021 21:18:47 +0100 Subject: [PATCH] rm: add verbose output and trim multiple slashes (#1988) * rm: add verbose output and trim multiple slashes Uses the normalize_path used in cargo to strip duplicate slashes With a link to a std rfc https://github.com/rust-lang/rfcs/issues/2208 This fixes https://github.com/uutils/coreutils/issues/1829 This also touches https://github.com/uutils/coreutils/issues/1768 but does not attempt to fully solve it --- src/uu/rm/Cargo.toml | 3 +- src/uu/rm/src/rm.rs | 16 ++++-- src/uucore/src/lib/features/fs.rs | 90 +++++++++++++++++++++++++++++++ tests/by-util/test_rm.rs | 29 ++++++++++ 4 files changed, 133 insertions(+), 5 deletions(-) diff --git a/src/uu/rm/Cargo.toml b/src/uu/rm/Cargo.toml index 961a8036c..9974111aa 100644 --- a/src/uu/rm/Cargo.toml +++ b/src/uu/rm/Cargo.toml @@ -18,7 +18,8 @@ path = "src/rm.rs" clap = "2.33" walkdir = "2.2" remove_dir_all = "0.5.1" -uucore = { version=">=0.0.8", package="uucore", path="../../uucore" } + +uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["fs"] } uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } [[bin]] diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index 033a1a4aa..09671768b 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -16,7 +16,7 @@ use std::collections::VecDeque; use std::fs; use std::io::{stderr, stdin, BufRead, Write}; use std::ops::BitOr; -use std::path::Path; +use std::path::{Path, PathBuf}; use walkdir::{DirEntry, WalkDir}; #[derive(Eq, PartialEq, Clone, Copy)] @@ -251,7 +251,7 @@ 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) { - if options.interactive != InteractiveMode::Always { + if options.interactive != InteractiveMode::Always && !options.verbose { // we need the extra crate because apparently fs::remove_dir_all() does not function // correctly on Windows if let Err(e) = remove_dir_all(path) { @@ -311,7 +311,7 @@ fn remove_dir(path: &Path, options: &Options) -> bool { match fs::remove_dir(path) { Ok(_) => { if options.verbose { - println!("removed directory '{}'", path.display()); + println!("removed directory '{}'", normalize(path).display()); } } Err(e) => { @@ -349,7 +349,7 @@ fn remove_file(path: &Path, options: &Options) -> bool { match fs::remove_file(path) { Ok(_) => { if options.verbose { - println!("removed '{}'", path.display()); + println!("removed '{}'", normalize(path).display()); } } Err(e) => { @@ -370,6 +370,14 @@ fn prompt_file(path: &Path, is_dir: bool) -> bool { } } +fn normalize(path: &Path) -> PathBuf { + // copied from https://github.com/rust-lang/cargo/blob/2e4cfc2b7d43328b207879228a2ca7d427d188bb/src/cargo/util/paths.rs#L65-L90 + // both projects are MIT https://github.com/rust-lang/cargo/blob/master/LICENSE-MIT + // for std impl progress see rfc https://github.com/rust-lang/rfcs/issues/2208 + // TODO: replace this once that lands + uucore::fs::normalize_path(path) +} + fn prompt(msg: &str) -> bool { let _ = stderr().write_all(msg.as_bytes()); let _ = stderr().flush(); diff --git a/src/uucore/src/lib/features/fs.rs b/src/uucore/src/lib/features/fs.rs index adf4f6f82..a72d6ea82 100644 --- a/src/uucore/src/lib/features/fs.rs +++ b/src/uucore/src/lib/features/fs.rs @@ -60,6 +60,37 @@ pub enum CanonicalizeMode { Missing, } +// copied from https://github.com/rust-lang/cargo/blob/2e4cfc2b7d43328b207879228a2ca7d427d188bb/src/cargo/util/paths.rs#L65-L90 +// both projects are MIT https://github.com/rust-lang/cargo/blob/master/LICENSE-MIT +// for std impl progress see rfc https://github.com/rust-lang/rfcs/issues/2208 +// replace this once that lands +pub fn normalize_path(path: &Path) -> PathBuf { + let mut components = path.components().peekable(); + let mut ret = if let Some(c @ Component::Prefix(..)) = components.peek().cloned() { + components.next(); + PathBuf::from(c.as_os_str()) + } else { + PathBuf::new() + }; + + for component in components { + match component { + Component::Prefix(..) => unreachable!(), + Component::RootDir => { + ret.push(component.as_os_str()); + } + Component::CurDir => {} + Component::ParentDir => { + ret.pop(); + } + Component::Normal(c) => { + ret.push(c); + } + } + } + ret +} + fn resolve>(original: P) -> IOResult { const MAX_LINKS_FOLLOWED: u32 = 255; let mut followed = 0; @@ -266,3 +297,62 @@ pub fn display_permissions_unix(mode: u32) -> String { result } + +#[cfg(test)] +mod tests { + // Note this useful idiom: importing names from outer (for mod tests) scope. + use super::*; + + struct NormalizePathTestCase<'a> { + path: &'a str, + test: &'a str, + } + + const NORMALIZE_PATH_TESTS: [NormalizePathTestCase; 8] = [ + NormalizePathTestCase { + path: "./foo/bar.txt", + test: "foo/bar.txt", + }, + NormalizePathTestCase { + path: "bar/../foo/bar.txt", + test: "foo/bar.txt", + }, + NormalizePathTestCase { + path: "foo///bar.txt", + test: "foo/bar.txt", + }, + NormalizePathTestCase { + path: "foo///bar", + test: "foo/bar", + }, + NormalizePathTestCase { + path: "foo//./bar", + test: "foo/bar", + }, + NormalizePathTestCase { + path: "/foo//./bar", + test: "/foo/bar", + }, + NormalizePathTestCase { + path: r"C:/you/later/", + test: "C:/you/later", + }, + NormalizePathTestCase { + path: "\\networkshare/a//foo//./bar", + test: "\\networkshare/a/foo/bar", + }, + ]; + + #[test] + fn test_normalize_path() { + for test in NORMALIZE_PATH_TESTS.iter() { + let path = Path::new(test.path); + let normalized = normalize_path(path); + assert_eq!( + test.test + .replace("/", std::path::MAIN_SEPARATOR.to_string().as_str()), + normalized.to_str().expect("Path is not valid utf-8!") + ); + } + } +} diff --git a/tests/by-util/test_rm.rs b/tests/by-util/test_rm.rs index 149d509c5..0d77d9b01 100644 --- a/tests/by-util/test_rm.rs +++ b/tests/by-util/test_rm.rs @@ -263,3 +263,32 @@ fn test_rm_no_operand() { ucmd.fails() .stderr_is("rm: error: missing an argument\nrm: error: for help, try 'rm --help'\n"); } + +#[test] +fn test_rm_verbose_slash() { + let (at, mut ucmd) = at_and_ucmd!(); + let dir = "test_rm_verbose_slash_directory"; + let file_a = &format!("{}/test_rm_verbose_slash_file_a", dir); + + at.mkdir(dir); + at.touch(file_a); + + let file_a_normalized = &format!( + "{}{}test_rm_verbose_slash_file_a", + dir, + std::path::MAIN_SEPARATOR + ); + + ucmd.arg("-r") + .arg("-f") + .arg("-v") + .arg(&format!("{}///", dir)) + .succeeds() + .stdout_only(format!( + "removed '{}'\nremoved directory '{}'\n", + file_a_normalized, dir + )); + + assert!(!at.dir_exists(dir)); + assert!(!at.file_exists(file_a)); +}