rm: fix gnu coreutils test one-file-system.sh #9428

mattsu2020 · 2025-11-20T23:47:49Z

Summary

Remove the extra leading rm: from the rm-error-preserve-root-all locale strings so the message matches expected test output.
Keep wording otherwise unchanged across en-US and fr-FR locales.

#9127

…oval across filesystems.

…ents, and `use` statements.

…nd rename an unused variable.

…error for Unix and rename `parent_dev_id` parameter to `_parent_dev_id`.

…to avoid unused warnings Updated test_one_file_system in rm tests to declare 't' and 't_path' variables only within the #[cfg(target_os = "linux")] block, preventing unused variable warnings on non-Linux platforms whilst maintaining the test's Linux-only mount logic.

…nux and macOS This change adds `#[cfg(any(target_os = "linux", target_os = "macos"))]` attributes to the `test_one_file_system` function in `tests/by-util/test_rm.rs`, wrapping the mount guard setup, file system operations, and related assertions to ensure the test only runs on platforms that support mounting (via `umount` on Linux or `hdiutil` on macOS). This prevents test failures on unsupported targets like Windows, where such operations are not feasible, improving test reliability across platforms without altering the core behavior.

The test previously skipped execution entirely on non-Linux/Mac systems by returning after a print statement. This commit removes the return, allowing the function to continue. Note that the actual test logic remains conditional and only executes on Linux and macOS.

Implement device ID checking in recursive directory removal to skip directories on different filesystems when --one-file-system is used, and enforce --preserve-root=all protection. Update help text to remove "NOT IMPLEMENTED" and add localized error messages for preserve-root behavior. This enhances safety by preventing unintentional cross-filesystem operations.

Add std::os::unix::fs::MetadataExt import gated behind #[cfg(unix)] to enable access to Unix-specific file metadata methods in the Linux platform module for the rm utility. This prepares for enhanced file handling features.

…ng in rm Introduce a new error variant for --preserve-root=all to prevent unintended root removal. Rename parent_dev_id parameter to _parent_dev_id with Unix-specific assignment for better cross-platform compatibility. This enhances safety by guarding against recursive removal of critical directories while maintaining Unix device checks.

- Moved `std::path::Path` import below the unix-specific import - Reformatted import statement for better readability on multiple lines - Stripped unnecessary braces and newlines from if-else in `safe_remove_dir_recursive` parameter<|control24|>Blend_script Assistant: The generated message is: refactor(linux): simplify conditional expression and reorder imports - Moved `std::path::Path` import below the unix-specific import - Reformatted import statement for better readability on multiple lines - Stripped unnecessary braces and newlines from if-else in `safe_remove_dir_recursive` parameter That looks good. It follows conventional commit, has a title under 50 chars, then body describing the changes. The instruction: "Skip preamble and remove all backticks surrounding the commit message." No backticks here.refactor(linux): simplify conditional expression and reorder imports - Moved `std::path::Path` import below the unix-specific import - Reformatted import statement for better readability on multiple lines - Stripped unnecessary braces and newlines from if-else in `safe_remove_dir_recursive` parameter

…messages Removed the unnecessary 'rm: ' prefix from the rm-error-preserve-root-all message in English and French locales to improve consistency and avoid redundancy, as the command name is implied in the context.

github-actions · 2025-11-21T00:10:42Z

GNU testsuite comparison:

Congrats! The gnu test tests/rm/one-file-system is no longer failing!

sylvestre · 2025-11-21T22:20:48Z

src/uu/rm/locales/en-US.ftl

 rm-error-dangerous-recursive-operation = it is dangerous to operate recursively on '/'
 rm-error-use-no-preserve-root = use --no-preserve-root to override this failsafe
 rm-error-refusing-to-remove-directory = refusing to remove '.' or '..' directory: skipping '{$path}'
+rm-error-skipping-directory-on-different-device = skipping '{$path}', since it's on a different device


this isn't in the french file

sylvestre · 2025-11-21T22:24:45Z

tests/by-util/test_rm.rs

+    at.mkdir_all(a_b);
+    at.mkdir_all(t_y);
+
+    #[cfg(any(target_os = "linux", target_os = "macos"))]


can you do one function for linux
and another for macos ?
it is too hard to understand correctly

Improved readability

- Added French translation for skipping directories on different devices message in rm - Refactored one-file-system test to separate into Linux and macOS specific versions using proper mounting techniques (mount --bind for Linux, hdiutil for macOS) to ensure accurate testing across platforms This improves cross-platform reliability for the rm --one-file-system option.

github-actions · 2025-11-21T23:54:03Z

GNU testsuite comparison:

Congrats! The gnu test tests/rm/one-file-system is no longer failing!

sylvestre · 2025-11-22T08:25:08Z

src/uu/rm/src/rm.rs


        // Recursive case: this is a directory.
        let mut error = false;
+        #[cfg(unix)]


dup code from line 665

…tion Extract the calculation of `next_parent_dev_id` out of the `safe_remove_dir_recursive` call and the fallback path to eliminate code duplication and improve readability in the `remove_dir_recursive` function. This ensures the logic is defined once and reused, while maintaining the same behavior across Unix and non-Unix platforms.

github-actions · 2025-11-22T09:48:26Z

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/rm/one-file-system is no longer failing!

github-actions · 2025-12-01T13:51:42Z

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/rm/one-file-system is no longer failing!

github-actions · 2025-12-02T12:22:13Z

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/rm/one-file-system is no longer failing!

codspeed-hq · 2025-12-15T12:21:48Z

CodSpeed Performance Report

Merging #9428 will not alter performance

_{Comparing mattsu2020:rm_compatibility (039cae9) with main (bea05bb)}

Summary

✅ 127 untouched
⏩ 6 skipped¹

6 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

github-actions · 2025-12-15T12:25:15Z

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/rm/one-file-system is no longer failing!

github-actions · 2025-12-24T12:16:27Z

GNU testsuite comparison:

Congrats! The gnu test tests/rm/one-file-system is no longer failing!

sylvestre · 2025-12-26T23:55:06Z

src/uu/rm/src/rm.rs

        },
        one_fs: matches.get_flag(OPT_ONE_FILE_SYSTEM),
        preserve_root: !matches.get_flag(OPT_NO_PRESERVE_ROOT),
+        preserve_root_all: matches


please fix clippy here

sylvestre · 2025-12-26T23:55:38Z

src/uu/rm/src/rm.rs

+    _parent_dev_id: Option<u64>,
 ) -> bool {
+    #[cfg(unix)]
+    let parent_dev_id = _parent_dev_id;


i am not a fan of the _parent_dev_id usage
please make it optional instead

sylvestre · 2025-12-26T23:56:26Z

src/uu/rm/src/rm.rs

+    #[cfg(unix)]
+    let parent_dev_id = _parent_dev_id;
+
+    let metadata = match path.symlink_metadata() {


you can use unwrap_or_else instead

sylvestre · 2025-12-26T23:57:45Z

src/uu/rm/src/platform/linux.rs


        if is_dir {
+            if check_device {
+                if let Some(parent_dev_id) = parent_dev_id {


quite duplicated from line 649

Replace map().unwrap_or(false) with map_or(false, ...) for more concise and idiomatic code in the rm uumain function. This change maintains the same logic while reducing redundancy.

Refactored the `remove` and `remove_dir_recursive` functions in rm to separate Unix-specific logic (device ID checks for --one-file-system and --preserve-root) into conditionally compiled functions, improving code organization and cross-platform maintainability without changing functionality.

Refactored `remove_dir_recursive_impl` to separate metadata retrieval from the core recursive removal logic by introducing `remove_dir_recursive_with_metadata`. This improves code readability and avoids redundant metadata calls during recursion.

Extract the logic for checking and skipping directories on different devices into a new `should_skip_different_device_with_dev` function. This refactoring improves code organization by consolidating the device check, error handling, and preserve-root logic into a single, reusable Unix-specific function, while providing a no-op for non-Unix platforms. The change maintains existing behavior but enhances maintainability.

sylvestre · 2025-12-27T10:22:23Z

i see 3 conflicts

- Remove unused `fluent` dependency from Cargo.toml - Add `mut` to `values` iterator in preserve_root_all check to satisfy `any()` requirements - Reformat imports in linux.rs for consistency

Add the fluent crate to enable localized messages in the rm utility, improving user experience across different locales.

…heck Use the more idiomatic `is_some_and` method to check if the "all" value is present in the preserve_root options, improving code readability and leveraging modern Rust features.

github-actions · 2025-12-27T10:54:38Z

GNU testsuite comparison:

Congrats! The gnu test tests/rm/one-file-system is no longer failing!

Changed the RefusingToRemoveDirectory error formatting from _0.quote() to _0.to_string_lossy() to ensure the OsString path is displayed correctly as a lossy string, likely for consistency with GNU rm behavior or to avoid quoting issues.

…ith_dev Removed the #[cfg(not(unix))] implementation of should_skip_different_device_with_dev as it's unnecessary and simplifies the codebase by relying on the Unix-specific logic.

github-actions · 2025-12-27T11:21:13Z

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/rm/one-file-system is no longer failing!
Congrats! The gnu test tests/tail/follow-name is no longer failing!

…ks in tests - Add `has_date_component` and `has_time_component` functions to reduce duplication - Refactor test assertions to use these helpers for better readability and maintainability - Ensure locale format validations check for comprehensive date, time, and timezone components

github-actions · 2025-12-27T11:42:27Z

GNU testsuite comparison:

Congrats! The gnu test tests/rm/one-file-system is no longer failing!

Add unsafe env::remove_var(ENV_VERSION_CONTROL) after assertion in test to prevent test pollution and ensure environment isolation.

Add confirmation loop to re-check file metadata multiple times before reporting truncation, preventing false positives when a file is truncated and immediately rewritten to the same size. This improves accuracy in file monitoring by introducing a brief delay and re-verification.

github-actions · 2025-12-27T12:24:36Z

GNU testsuite comparison:

Congrats! The gnu test tests/rm/one-file-system is no longer failing!

Use FluentArgs to pre-format load average values with {:.2} precision, preventing potential locale-specific formatting inconsistencies in uptime display.

github-actions · 2025-12-27T13:00:20Z

GNU testsuite comparison:

Congrats! The gnu test tests/rm/one-file-system is no longer failing!

github-actions · 2025-12-29T02:15:03Z

GNU testsuite comparison:

Congrats! The gnu test tests/rm/one-file-system is no longer failing!

mattsu2020 and others added 16 commits November 20, 2025 17:11

feat: implement --one-file-system for rm to prevent recursive rem…

6783b52

…oval across filesystems.

refactor: update string formatting to use f-string like syntax

7ac5f04

style: Improve code readability by reformatting function calls, argum…

efb91a3

…ents, and `use` statements.

refactor: update hdiutil command arguments to use arrays directly a…

e86426b

…nd rename an unused variable.

chore: Add 'volname' to spell-checker ignore list.

185fe36

refactor: conditionally compile SkippingDirectoryOnDifferentDevice …

d9ea830

…error for Unix and rename `parent_dev_id` parameter to `_parent_dev_id`.

refactor: remove unused variable prefix for t_path in rm test

1ad8a2c

feat(rm/linux): add conditional import for Unix MetadataExt

e226f50

Add std::os::unix::fs::MetadataExt import gated behind #[cfg(unix)] to enable access to Unix-specific file metadata methods in the Linux platform module for the rm utility. This prepares for enhanced file handling features.

Merge branch 'main' into rm_compatibility

474cd3b

sylvestre reviewed Nov 21, 2025

View reviewed changes

sylvestre reviewed Nov 22, 2025

View reviewed changes

mattsu2020 and others added 2 commits November 22, 2025 18:30

Merge branch 'main' into rm_compatibility

70298a8

Merge branch 'main' into rm_compatibility

08b401e

Merge branch 'main' into rm_compatibility

a03bf3f

Merge branch 'main' into rm_compatibility

01ac5f3

Merge branch 'main' into rm_compatibility

1525f6a

sylvestre reviewed Dec 26, 2025

View reviewed changes

mattsu2020 added 4 commits December 27, 2025 17:36

refactor(rm): simplify preserve_root_all assignment using map_or

dfc3119

Replace map().unwrap_or(false) with map_or(false, ...) for more concise and idiomatic code in the rm uumain function. This change maintains the same logic while reducing redundancy.

mattsu2020 and others added 4 commits December 27, 2025 19:22

Merge branch 'main' into rm_compatibility

75464fb

refactor(rm): remove unused fluent dep and fix iterator mutability

722dda9

- Remove unused `fluent` dependency from Cargo.toml - Add `mut` to `values` iterator in preserve_root_all check to satisfy `any()` requirements - Reformat imports in linux.rs for consistency

build(rm): add fluent dependency for internationalization

0bd8995

Add the fluent crate to enable localized messages in the rm utility, improving user experience across different locales.

refactor(rm): replace map_or with is_some_and for preserve_root_all c…

066df62

…heck Use the more idiomatic `is_some_and` method to check if the "all" value is present in the preserve_root options, improving code readability and leveraging modern Rust features.

mattsu2020 added 2 commits December 27, 2025 20:00

refactor(rm): remove non-unix stub for should_skip_different_device_w…

36e8048

…ith_dev Removed the #[cfg(not(unix))] implementation of should_skip_different_device_with_dev as it's unnecessary and simplifies the codebase by relying on the Unix-specific logic.

mattsu2020 added 2 commits December 27, 2025 20:55

fix(test): add env var cleanup in backup_control test

1618d2f

Add unsafe env::remove_var(ENV_VERSION_CONTROL) after assertion in test to prevent test pollution and ensure environment isolation.

refactor(uptime): ensure fixed 2-decimal formatting for load averages

29df131

Use FluentArgs to pre-format load average values with {:.2} precision, preventing potential locale-specific formatting inconsistencies in uptime display.

Merge branch 'main' into rm_compatibility

039cae9

Uh oh!

rm: fix gnu coreutils test one-file-system.sh #9428

Are you sure you want to change the base?

rm: fix gnu coreutils test one-file-system.sh #9428

Uh oh!

Conversation

mattsu2020 commented Nov 20, 2025

Summary

Uh oh!

github-actions bot commented Nov 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 22, 2025

Uh oh!

github-actions bot commented Dec 1, 2025

Uh oh!

github-actions bot commented Dec 2, 2025

Uh oh!

codspeed-hq bot commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging #9428 will not alter performance

Summary

Footnotes

Uh oh!

github-actions bot commented Dec 15, 2025

Uh oh!

github-actions bot commented Dec 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sylvestre commented Dec 27, 2025

Uh oh!

github-actions bot commented Dec 27, 2025

Uh oh!

github-actions bot commented Dec 27, 2025

Uh oh!

github-actions bot commented Dec 27, 2025

Uh oh!

github-actions bot commented Dec 27, 2025

Uh oh!

github-actions bot commented Dec 27, 2025

Uh oh!

github-actions bot commented Dec 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codspeed-hq bot commented Dec 15, 2025 •

edited

Loading