From 2689f05a575f7f4b1a0a3edc96a899a1849ae231 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 21 Oct 2023 08:28:20 +0200 Subject: [PATCH 01/11] rm: implement --one-file-system --- src/uu/rm/src/rm.rs | 44 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index 4fc37a1300f..b7370e81468 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -53,7 +53,6 @@ pub struct Options { /// 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` @@ -216,8 +215,7 @@ pub fn uu_app() -> Command { .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( @@ -322,10 +320,50 @@ pub fn remove(files: &[&OsStr], options: &Options) -> bool { had_err } +#[cfg(not(windows))] +fn get_device_id(p: &Path) -> Option { + use std::os::unix::fs::MetadataExt; + p.symlink_metadata() + .ok() + .map(|metadata| MetadataExt::dev(&metadata)) +} + +#[cfg(windows)] +fn get_device_id(_p: &Path) -> Option { + 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; + } + + let parent_device = path.parent().and_then(get_device_id); + let current_device = get_device_id(path); + + if parent_device != current_device { + show_error!( + "skipping {}, since it's on a different device", + path.quote() + ); + return false; + } + + true +} + #[allow(clippy::cognitive_complexity)] fn handle_dir(path: &Path, options: &Options) -> bool { let mut had_err = false; + if options.one_fs { + if !check_one_fs(path) { + return true; + } + } + let is_root = path.has_root() && path.parent().is_none(); if options.recursive && (!is_root || !options.preserve_root) { if options.interactive != InteractiveMode::Always && !options.verbose { From 38749f0e95c5b02bf48f267cd2fb0ec90e0842f1 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 22 Oct 2023 09:33:59 +0200 Subject: [PATCH 02/11] rm: implement --one-file-system --- src/uu/rm/src/rm.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index b7370e81468..b879ea0af3a 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -340,9 +340,14 @@ fn check_one_fs(path: &Path, options: &Options) -> bool { return true; } - let parent_device = path.parent().and_then(get_device_id); + // 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); - if parent_device != current_device { show_error!( "skipping {}, since it's on a different device", From 4b8606809069ddbe6841c9e92ab716e6ba909b69 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 22 Oct 2023 09:34:38 +0200 Subject: [PATCH 03/11] rm: also add --preserve-root=all failure --- src/uu/rm/src/rm.rs | 42 +++++++++++++++++++++------ tests/by-util/test_rm.rs | 62 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 9 deletions(-) diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index b879ea0af3a..a6d81b3b582 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -31,6 +31,13 @@ pub enum InteractiveMode { 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 @@ -56,7 +63,7 @@ pub struct Options { /// `--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` @@ -84,6 +91,7 @@ static PRESUME_INPUT_TTY: &str = "-presume-input-tty"; 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)?; @@ -136,7 +144,19 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } }, 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::(OPT_PRESERVE_ROOT) + .unwrap() + .as_str() + { + "all" => PreserveRoot::YesAll, + "default" => PreserveRoot::Default, + _ => PreserveRoot::Default, + } + }, recursive: matches.get_flag(OPT_RECURSIVE), dir: matches.get_flag(OPT_DIR), verbose: matches.get_flag(OPT_VERBOSE), @@ -228,7 +248,10 @@ pub fn uu_app() -> Command { Arg::new(OPT_PRESERVE_ROOT) .long(OPT_PRESERVE_ROOT) .help("do not remove '/' (default)") - .action(ArgAction::SetTrue), + .value_parser(["default", "all"]) + .default_value("default") + .default_missing_value("default") + .hide_default_value(true) ) .arg( Arg::new(OPT_RECURSIVE) @@ -353,6 +376,9 @@ fn check_one_fs(path: &Path, options: &Options) -> bool { "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; } @@ -363,14 +389,12 @@ fn check_one_fs(path: &Path, options: &Options) -> bool { fn handle_dir(path: &Path, options: &Options) -> bool { let mut had_err = false; - if options.one_fs { - if !check_one_fs(path) { - 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) @@ -436,7 +460,7 @@ fn handle_dir(path: &Path, options: &Options) -> bool { 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!("could not remove directory {}", path.quote()); diff --git a/tests/by-util/test_rm.rs b/tests/by-util/test_rm.rs index 5125c746da7..5930a5688e7 100644 --- a/tests/by-util/test_rm.rs +++ b/tests/by-util/test_rm.rs @@ -698,3 +698,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"); +} From e979ffdc2f5037587c47b6307e6a369a20443370 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 21 Oct 2023 19:32:18 +0200 Subject: [PATCH 04/11] rm: adjust a gnu test --- util/build-gnu.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/util/build-gnu.sh b/util/build-gnu.sh index dbddc8a315e..e895c24a66f 100755 --- a/util/build-gnu.sh +++ b/util/build-gnu.sh @@ -210,6 +210,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 From 4a84af208f9b2f77ab5ecc0ac6d04b77168aec76 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 22 Oct 2023 22:13:41 +0200 Subject: [PATCH 05/11] don't accept default as arg Co-authored-by: Daniel Hofstetter --- src/uu/rm/src/rm.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index a6d81b3b582..4fe8947b76f 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -248,7 +248,7 @@ pub fn uu_app() -> Command { Arg::new(OPT_PRESERVE_ROOT) .long(OPT_PRESERVE_ROOT) .help("do not remove '/' (default)") - .value_parser(["default", "all"]) + .value_parser(["all"]) .default_value("default") .default_missing_value("default") .hide_default_value(true) From 8320b8a65c5b32ace808f60b7cd6c1c03f44bddd Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 22 Oct 2023 22:14:27 +0200 Subject: [PATCH 06/11] all are default --- src/uu/rm/src/rm.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index 4fe8947b76f..af483132db5 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -153,7 +153,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .as_str() { "all" => PreserveRoot::YesAll, - "default" => PreserveRoot::Default, _ => PreserveRoot::Default, } }, From d665a4c0f1459a6dd2986ec542a42a09ce6de10f Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Mon, 25 Dec 2023 23:00:45 +0100 Subject: [PATCH 07/11] fix the default --- src/uu/rm/src/rm.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index af483132db5..a299a2e6e82 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -248,8 +248,8 @@ pub fn uu_app() -> Command { .long(OPT_PRESERVE_ROOT) .help("do not remove '/' (default)") .value_parser(["all"]) - .default_value("default") - .default_missing_value("default") + .default_value("all") + .default_missing_value("all") .hide_default_value(true) ) .arg( From 70e71af8a782c9a802e85ebf487515baabe76c80 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 26 Dec 2023 01:02:30 +0100 Subject: [PATCH 08/11] try to compare the root results too --- .github/workflows/GnuTests.yml | 124 +++++++++++++++++++-------------- 1 file changed, 72 insertions(+), 52 deletions(-) diff --git a/.github/workflows/GnuTests.yml b/.github/workflows/GnuTests.yml index 3ce6e8be27b..87c97ff2e70 100644 --- a/.github/workflows/GnuTests.yml +++ b/.github/workflows/GnuTests.yml @@ -204,6 +204,7 @@ jobs: ## Compare test failures VS reference have_new_failures="" REF_LOG_FILE='${{ steps.vars.outputs.path_reference }}/test-logs/test-suite.log' + ROOT_REF_LOG_FILE='${{ steps.vars.outputs.path_reference }}/test-logs/test-suite-root.log' REF_SUMMARY_FILE='${{ steps.vars.outputs.path_reference }}/test-summary/gnu-result.json' REPO_DEFAULT_BRANCH='${{ steps.vars.outputs.repo_default_branch }}' path_UUTILS='${{ steps.vars.outputs.path_UUTILS }}' @@ -223,68 +224,87 @@ jobs: rm -f ${COMMENT_LOG} touch ${COMMENT_LOG} - if test -f "${REF_LOG_FILE}"; then - echo "Reference SHA1/ID: $(sha1sum -- "${REF_SUMMARY_FILE}")" - REF_ERROR=$(sed -n "s/^ERROR: \([[:print:]]\+\).*/\1/p" "${REF_LOG_FILE}" | sort) - NEW_ERROR=$(sed -n "s/^ERROR: \([[:print:]]\+\).*/\1/p" '${{ steps.vars.outputs.path_GNU_tests }}/test-suite.log' | sort) - REF_FAILING=$(sed -n "s/^FAIL: \([[:print:]]\+\).*/\1/p" "${REF_LOG_FILE}" | sort) - NEW_FAILING=$(sed -n "s/^FAIL: \([[:print:]]\+\).*/\1/p" '${{ steps.vars.outputs.path_GNU_tests }}/test-suite.log' | sort) - for LINE in ${REF_FAILING} - do - if ! grep -Fxq ${LINE}<<<"${NEW_FAILING}"; then - if ! grep ${LINE} ${IGNORE_INTERMITTENT} + compare_tests() { + local new_log_file=$1 + local ref_log_file=$2 + local test_type=$3 # "standard" or "root" + + if test -f "${ref_log_file}"; then + echo "Reference ${test_type} test log SHA1/ID: $(sha1sum -- "${ref_log_file}")" + REF_ERROR=$(sed -n "s/^ERROR: \([[:print:]]\+\).*/\1/p" "${ref_log_file}"| sort) + NEW_ERROR=$(sed -n "s/^ERROR: \([[:print:]]\+\).*/\1/p" "${new_log_file}" | sort) + REF_FAILING=$(sed -n "s/^FAIL: \([[:print:]]\+\).*/\1/p" "${ref_log_file}"| sort) + NEW_FAILING=$(sed -n "s/^FAIL: \([[:print:]]\+\).*/\1/p" "${new_log_file}" | sort) + + # Compare failing and error tests + for LINE in ${NEW_FAILING} + do + if ! grep -Fxq ${LINE}<<<"${REF_FAILING}" then - MSG="Congrats! The gnu test ${LINE} is no longer failing!" - echo "::warning ::$MSG" - echo $MSG >> ${COMMENT_LOG} - else - MSG="Skipping an intermittent issue ${LINE}" - echo "::warning ::$MSG" - echo $MSG >> ${COMMENT_LOG} - echo "" + if ! grep ${LINE} ${IGNORE_INTERMITTENT} + then + MSG="GNU test failed: ${LINE}. ${LINE} is passing on '${REPO_DEFAULT_BRANCH}'. Maybe you have to rebase?" + echo "::error ::$MSG" + echo $MSG >> ${COMMENT_LOG} + have_new_failures="true" + else + MSG="Skip an intermittent issue ${LINE}" + echo "::warning ::$MSG" + echo $MSG >> ${COMMENT_LOG} + echo "" + fi fi - fi - done - for LINE in ${NEW_FAILING} - do - if ! grep -Fxq ${LINE}<<<"${REF_FAILING}" - then - if ! grep ${LINE} ${IGNORE_INTERMITTENT} + done + + for LINE in ${REF_FAILING} + do + if ! grep -Fxq ${LINE}<<<"${NEW_FAILING}" + then + if ! grep ${LINE} ${IGNORE_INTERMITTENT} + then + MSG="Congrats! The gnu test ${LINE} is no longer failing!" + echo "::warning ::$MSG" + echo $MSG >> ${COMMENT_LOG} + else + MSG="Skipping an intermittent issue ${LINE}" + echo "::warning ::$MSG" + echo $MSG >> ${COMMENT_LOG} + echo "" + fi + fi + done + + for LINE in ${NEW_ERROR} + do + if ! grep -Fxq ${LINE}<<<"${REF_ERROR}" then - MSG="GNU test failed: ${LINE}. ${LINE} is passing on '${{ steps.vars.outputs.repo_default_branch }}'. Maybe you have to rebase?" + MSG="GNU test error: ${LINE}. ${LINE} is passing on '${REPO_DEFAULT_BRANCH}'. Maybe you have to rebase?" echo "::error ::$MSG" echo $MSG >> ${COMMENT_LOG} have_new_failures="true" - else - MSG="Skip an intermittent issue ${LINE}" + fi + done + + for LINE in ${REF_ERROR} + do + if ! grep -Fxq ${LINE}<<<"${NEW_ERROR}" + then + MSG="Congrats! The gnu test ${LINE} is no longer ERROR!" echo "::warning ::$MSG" echo $MSG >> ${COMMENT_LOG} - echo "" fi - fi - done - for LINE in ${REF_ERROR} - do - if ! grep -Fxq ${LINE}<<<"${NEW_ERROR}"; then - MSG="Congrats! The gnu test ${LINE} is no longer ERROR!" - echo "::warning ::$MSG" - echo $MSG >> ${COMMENT_LOG} - fi - done - for LINE in ${NEW_ERROR} - do - if ! grep -Fxq ${LINE}<<<"${REF_ERROR}" - then - MSG="GNU test error: ${LINE}. ${LINE} is passing on '${{ steps.vars.outputs.repo_default_branch }}'. Maybe you have to rebase?" - echo "::error ::$MSG" - echo $MSG >> ${COMMENT_LOG} - have_new_failures="true" - fi - done + done + else + echo "::warning ::Skipping ${test_type} test failure comparison; no prior reference test logs are available." + fi + } + + # Compare standard tests + compare_tests '${{ steps.vars.outputs.path_GNU_tests }}/test-suite.log' "${REF_LOG_FILE}" "standard" + + # Compare root tests + compare_tests '${{ steps.vars.outputs.path_GNU_tests }}/test-suite-root.log' "${ROOT_REF_LOG_FILE}" "root" - else - echo "::warning ::Skipping test failure comparison; no prior reference test logs are available." - fi if test -n "${have_new_failures}" ; then exit -1 ; fi - name: Upload comparison log (for GnuComment workflow) if: success() || failure() # run regardless of prior step success/failure From 4d94eb65794d724e82648f4e3a32d0119bb1c763 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 26 Dec 2023 12:25:01 +0100 Subject: [PATCH 09/11] Add debug info in the logs --- .github/workflows/GnuTests.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/GnuTests.yml b/.github/workflows/GnuTests.yml index 87c97ff2e70..63e1cd85c67 100644 --- a/.github/workflows/GnuTests.yml +++ b/.github/workflows/GnuTests.yml @@ -230,12 +230,16 @@ jobs: local test_type=$3 # "standard" or "root" if test -f "${ref_log_file}"; then - echo "Reference ${test_type} test log SHA1/ID: $(sha1sum -- "${ref_log_file}")" + echo "Reference ${test_type} test log SHA1/ID: $(sha1sum -- "${ref_log_file}") - ${test_type}" REF_ERROR=$(sed -n "s/^ERROR: \([[:print:]]\+\).*/\1/p" "${ref_log_file}"| sort) NEW_ERROR=$(sed -n "s/^ERROR: \([[:print:]]\+\).*/\1/p" "${new_log_file}" | sort) REF_FAILING=$(sed -n "s/^FAIL: \([[:print:]]\+\).*/\1/p" "${ref_log_file}"| sort) NEW_FAILING=$(sed -n "s/^FAIL: \([[:print:]]\+\).*/\1/p" "${new_log_file}" | sort) - + echo "Detailled information:" + echo "REF_ERROR = ${REF_ERROR}" + echo "NEW_ERROR = ${NEW_ERROR}" + echo "REF_FAILING = ${REF_FAILING}" + echo "NEW_FAILING = ${NEW_FAILING}" # Compare failing and error tests for LINE in ${NEW_FAILING} do From 577e2bfa22395bacc2ebdfa1611ccff0594ad530 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 22 Dec 2024 16:28:25 +0100 Subject: [PATCH 10/11] fix rustfmt --- src/uu/rm/src/rm.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index 15bc22d6cec..eac299309d2 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -398,7 +398,7 @@ fn handle_dir(path: &Path, options: &Options) -> bool { if !check_one_fs(path, options) { return true; } - + let path = clean_trailing_slashes(path); if path_is_current_or_parent_directory(path) { show_error!( From 9fc1a681ec5ee0983f5b44b61f270d8c0213349f Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Mon, 23 Dec 2024 22:59:45 +0100 Subject: [PATCH 11/11] improve --- src/uu/rm/src/rm.rs | 82 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 76 insertions(+), 6 deletions(-) diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index eac299309d2..e79ab2744f1 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -364,11 +364,12 @@ fn get_device_id(_p: &Path) -> Option { /// 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 { +/*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) @@ -377,6 +378,75 @@ fn check_one_fs(path: &Path, options: &Options) -> bool { .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", @@ -395,10 +465,6 @@ fn check_one_fs(path: &Path, options: &Options) -> bool { fn handle_dir(path: &Path, options: &Options) -> bool { let mut had_err = false; - if !check_one_fs(path, options) { - return true; - } - let path = clean_trailing_slashes(path); if path_is_current_or_parent_directory(path) { show_error!( @@ -408,6 +474,10 @@ fn handle_dir(path: &Path, options: &Options) -> bool { 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 == PreserveRoot::No) { if options.interactive != InteractiveMode::Always && !options.verbose {