rm: implement --one-file-system and --preserve-root=all failure

src/uu/rm/src/rm.rs

@@ -36,6 +36,13 @@
     PromptProtected,
 }
 
+#[derive(PartialEq, Debug)]
+pub enum PreserveRoot {
+    Default,
+    YesAll,
+    No,
+}
+
 /// Options for the `rm` command
 ///
 /// All options are public so that the options can be programmatically
@@ -58,11 +65,10 @@
     /// If no other option sets this mode, [`InteractiveMode::PromptProtected`]
     /// is used
     pub interactive: InteractiveMode,
-    #[allow(dead_code)]
     /// `--one-file-system`
     pub one_fs: bool,
     /// `--preserve-root`/`--no-preserve-root`
-    pub preserve_root: bool,
+    pub preserve_root: PreserveRoot,
     /// `-r`, `--recursive`
     pub recursive: bool,
     /// `-d`, `--dir`
@@ -90,6 +96,7 @@
 static ARG_FILES: &str = "files";
 
 #[uucore::main]
+#[allow(clippy::cognitive_complexity)]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let matches = uu_app().after_help(AFTER_HELP).try_get_matches_from(args)?;
 
@@ -142,7 +149,18 @@
                 }
             },
             one_fs: matches.get_flag(OPT_ONE_FILE_SYSTEM),
-            preserve_root: !matches.get_flag(OPT_NO_PRESERVE_ROOT),
+            preserve_root: if matches.get_flag(OPT_NO_PRESERVE_ROOT) {
+                PreserveRoot::No
+            } else {
+                match matches
+                    .get_one::<String>(OPT_PRESERVE_ROOT)
+                    .unwrap()
+                    .as_str()
+                {
+                    "all" => PreserveRoot::YesAll,
+                    _ => PreserveRoot::Default,
+                }
+            },
             recursive: matches.get_flag(OPT_RECURSIVE),
             dir: matches.get_flag(OPT_DIR),
             verbose: matches.get_flag(OPT_VERBOSE),
@@ -221,8 +239,7 @@
                 .long(OPT_ONE_FILE_SYSTEM)
                 .help(
                     "when removing a hierarchy recursively, skip any directory that is on a file \
-                    system different from that of the corresponding command line argument (NOT \
-                    IMPLEMENTED)",
+                    system different from that of the corresponding command line argument",
                 ).action(ArgAction::SetTrue),
         )
         .arg(
@@ -235,7 +252,10 @@
             Arg::new(OPT_PRESERVE_ROOT)
                 .long(OPT_PRESERVE_ROOT)
                 .help("do not remove '/' (default)")
-                .action(ArgAction::SetTrue),
+                .value_parser(["all"])
+                .default_value("all")
+                .default_missing_value("all")
+                .hide_default_value(true)
         )
         .arg(
             Arg::new(OPT_RECURSIVE)
@@ -329,6 +349,118 @@
     had_err
 }
 
+#[cfg(not(windows))]
+fn get_device_id(p: &Path) -> Option<u64> {
+    use std::os::unix::fs::MetadataExt;
+    p.symlink_metadata()
+        .ok()
+        .map(|metadata| MetadataExt::dev(&metadata))
+}
+
+#[cfg(windows)]
+fn get_device_id(_p: &Path) -> Option<u64> {
+    unimplemented!()
+}
+
+/// Checks if the given path is on the same device as its parent.
+/// Returns false if the `one_fs` option is enabled and devices differ.
+/*fn check_one_fs(path: &Path, options: &Options) -> bool {
+    if !options.one_fs && options.preserve_root != PreserveRoot::YesAll {
+        return true;
+    }
+    println!("checking {}", path.display());
+    println!("parent: {:?}", fs::canonicalize(path).ok().and_then(|p| p.parent().map(Path::to_path_buf)));
+    // as we can get relative path, we need to canonicalize
+    // and manage potential errors
+    let parent_device = fs::canonicalize(path)
+        .ok()
+        .and_then(|p| p.parent().map(Path::to_path_buf))
+        .as_deref()
+        .and_then(get_device_id);
+    let current_device = get_device_id(path);
+    println!("parent_device: {:?}, current_device: {:?}", parent_device, current_device);
+    if parent_device != current_device {
+        show_error!(
+            "skipping {}, since it's on a different device",
+            path.quote()
+        );
+        if options.preserve_root == PreserveRoot::YesAll {
+            show_error!("and --preserve-root=all is in effect");
+        }
+        return false;
+    }
+
+    true
+}*/
+/*
+fn check_one_fs(path: &Path, options: &Options) -> bool {
+    if !options.one_fs && options.preserve_root != PreserveRoot::YesAll {
+        return true;
+    }
+
+    let parent_device = path.parent().and_then(get_device_id);
+    let current_device = get_device_id(path);
+
+    println!("parent_device: {:?}, current_device: {:?}", parent_device, current_device);
+
+    if parent_device != current_device {
+
+        show_error!(
+            "skipping {}, since it's on a different device",
+            path.quote()
+        );
+        if options.preserve_root == PreserveRoot::YesAll {
+            show_error!("and --preserve-root=all is in effect");
+        }
+        return false;
+    }
+
+    true
+}
+*/
+
+fn check_one_fs(path: &Path, options: &Options) -> bool {
+    if !options.one_fs && options.preserve_root != PreserveRoot::YesAll {
+        return true;
+    }
+
+    // Attempt to canonicalize the path
+    let path_canon = match path.canonicalize() {
+        Ok(p) => p,
+        Err(_) => {
+            // If we can't canonicalize, fallback to original
+            // or handle the error differently
+            path.to_path_buf()
+        }
+    };
+
+    let parent_path = path_canon
+        .parent()
+        .map(|p| p.to_path_buf())
+        .unwrap_or_else(|| path_canon.clone());
+
+    let parent_device = get_device_id(&parent_path);
+    let current_device = get_device_id(&path_canon);
+
+    println!(
+        "parent_device: {:?}, current_device: {:?}",
+        parent_device, current_device
+    );
+
+    if parent_device != current_device {
+        show_error!(
+            "skipping {}, since it's on a different device",
+            path.quote()
+        );
+        if options.preserve_root == PreserveRoot::YesAll {
+            show_error!("and --preserve-root=all is in effect");
+        }
+        return false;
+    }
+
+    true
+}
+
 #[allow(clippy::cognitive_complexity)]
 fn handle_dir(path: &Path, options: &Options) -> bool {
     let mut had_err = false;
@@ -342,8 +474,12 @@
         return true;
     }
 
+    if !check_one_fs(path, options) {
+        return true;
+    }
+
     let is_root = path.has_root() && path.parent().is_none();
-    if options.recursive && (!is_root || !options.preserve_root) {
+    if options.recursive && (!is_root || options.preserve_root == PreserveRoot::No) {
         if options.interactive != InteractiveMode::Always && !options.verbose {
             if let Err(e) = fs::remove_dir_all(path) {
                 // GNU compatibility (rm/empty-inacc.sh)
@@ -409,7 +545,7 @@
                 had_err = remove_dir(dir.path(), options).bitor(had_err);
             }
         }
-    } else if options.dir && (!is_root || !options.preserve_root) {
+    } else if options.dir && (!is_root || options.preserve_root == PreserveRoot::No) {
         had_err = remove_dir(path, options).bitor(had_err);
     } else if options.recursive {
         show_error!(

tests/by-util/test_rm.rs

@@ -775,3 +775,65 @@ fn test_non_utf8() {
     ucmd.arg(file).succeeds();
     assert!(!at.file_exists(file));
 }
+
+#[test]
+#[cfg(any(target_os = "linux", target_os = "android", target_os = "freebsd"))]
+fn test_rm_one_file_system() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    // Test must be run as root (or with `sudo -E`)
+    if scene.cmd("whoami").run().stdout_str() != "root\n" {
+        return;
+    }
+    let src = "a/b";
+    let dst = "t/y";
+    at.mkdir_all(src);
+    at.mkdir_all(dst);
+
+    scene
+        .cmd("mount")
+        .arg("--bind")
+        .arg("t")
+        .arg("a/b")
+        .succeeds();
+    scene
+        .ucmd()
+        .arg("--one-file-system")
+        .arg("-rf")
+        .arg("a")
+        .fails()
+        .stderr_contains("rm: skipping 'a', since it's on a different device");
+}
+
+#[test]
+#[cfg(any(target_os = "linux", target_os = "android", target_os = "freebsd"))]
+fn test_rm_preserve_root() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    // Test must be run as root (or with `sudo -E`)
+    if scene.cmd("whoami").run().stdout_str() != "root\n" {
+        return;
+    }
+    let src = "a/b";
+    let dst = "t/y";
+    at.mkdir_all(src);
+    at.mkdir_all(dst);
+
+    scene
+        .cmd("mount")
+        .arg("--bind")
+        .arg("t")
+        .arg("a/b")
+        .succeeds();
+
+    scene
+        .ucmd()
+        .arg("--preserve-root=all")
+        .arg("-rf")
+        .arg("a")
+        .fails()
+        .stderr_contains("rm: skipping 'a', since it's on a different device")
+        .stderr_contains("rm: and --preserve-root=all is in effect");
+}

util/build-gnu.sh

@@ -237,6 +237,11 @@ sed -i -e "s|rm: cannot remove 'a/1'|rm: cannot remove 'a'|g" tests/rm/rm2.sh
 
 sed -i -e "s|removed directory 'a/'|removed directory 'a'|g" tests/rm/v-slash.sh
 
+if test "$(grep -c 'rm: skipping '  tests/rm/one-file-system.sh)" -eq 1; then
+     # Do it only once.
+     sed -i -e "s|rm: skipping 'a/b'|rm: skipping 'a'|g" -e "s/ >> exp/ > exp/g"  -e "s|rm: and --preserve-root=all is in effect|rm: skipping 'a/b', since it's on a different device\nrm: and --preserve-root=all is in effect|g" tests/rm/one-file-system.sh
+fi
+
 # 'rel' doesn't exist. Our implementation is giving a better message.
 sed -i -e "s|rm: cannot remove 'rel': Permission denied|rm: cannot remove 'rel': No such file or directory|g" tests/rm/inaccessible.sh