Skip to content

Conversation

@mattsu2020
Copy link
Contributor

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

mattsu2020 and others added 16 commits November 20, 2025 17:11
…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
Copy link

GNU testsuite comparison:

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

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't in the french file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

at.mkdir_all(a_b);
at.mkdir_all(t_y);

#[cfg(any(target_os = "linux", target_os = "macos"))]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link

GNU testsuite comparison:

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


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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dup code from line 665

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix is

mattsu2020 and others added 2 commits November 22, 2025 18:30
…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
Copy link

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
Copy link

github-actions bot commented Dec 1, 2025

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
Copy link

github-actions bot commented Dec 2, 2025

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
Copy link

codspeed-hq bot commented Dec 15, 2025

CodSpeed Performance Report

Merging #9428 will not alter performance

Comparing mattsu2020:rm_compatibility (1525f6a) with main (cc103ec)

Summary

✅ 127 untouched
⏩ 6 skipped1

Footnotes

  1. 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
Copy link

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
Copy link

GNU testsuite comparison:

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants