Skip to content

rm: report specific error when symlink_metadata fails#11124

Open
anihal wants to merge 3 commits intouutils:mainfrom
anihal:rm-metadata-error-specificity
Open

rm: report specific error when symlink_metadata fails#11124
anihal wants to merge 3 commits intouutils:mainfrom
anihal:rm-metadata-error-specificity

Conversation

@anihal
Copy link
Contributor

@anihal anihal commented Feb 26, 2026

Summary

Previously, any error from symlink_metadata() in the main file-removal
loop was either silently swallowed under -f, or always reported as
"No such file or directory" regardless of the real error kind. Two
TODO comments called this out explicitly.

This PR fixes both issues by splitting the single catch-all Err arm
into two:

  • NotFound — preserves the existing behaviour: silent under -f,
    prints cannot remove 'file': No such file or directory otherwise.
  • All other errors (e.g. Permission denied when a path component
    is not accessible) — always routes through the existing
    show_removal_error helper so the correct OS-specific message is
    printed, even when -f is passed. This matches GNU rm behaviour.

Before:

$ rm -f /root/secret    # no output, exit 0  (wrong)

After:

$ rm -f /root/secret
rm: cannot remove '/root/secret': Permission denied   (correct)

@github-actions
Copy link

GNU testsuite comparison:

GNU test failed: tests/rm/ignorable. tests/rm/ignorable is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/inaccessible. tests/rm/inaccessible is passing on 'main'. Maybe you have to rebase?
Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/csplit/csplit-heap is now passing!
Congrats! The gnu test tests/printf/printf-surprise is now passing!
Congrats! The gnu test tests/tail/tail-n0f is now passing!

@sylvestre
Copy link
Contributor

please add a test

Previously, any error from symlink_metadata was silently swallowed
under -f, or always reported as "No such file or directory" regardless
of the actual error kind. Fix this by:

- Matching NotFound explicitly: silent under -f, "No such file or
  directory" otherwise (preserving the existing behaviour).
- Routing all other errors (e.g. Permission denied on a path component)
  through show_removal_error so the correct message is always printed,
  even when -f is given.
Two GNU test regressions from the previous commit:
- tests/rm/ignorable: rm -f on a path through a non-directory (ENOTDIR)
  must exit 0; the catch-all Err arm was ignoring options.force.
- tests/rm/inaccessible: without -f, the CI runner expects the message
  "No such file or directory" (matching current uutils behaviour via
  CannotRemoveNoSuchFile); switching to show_removal_error changed the
  wording to "Permission denied" and broke the comparison.

Fix: respect options.force in the catch-all Err arm and keep
CannotRemoveNoSuchFile for the non-force path, matching pre-PR
behaviour. A TODO is left for when the GNU test expectation is updated.
@anihal anihal force-pushed the rm-metadata-error-specificity branch from a266bce to 164b1dc Compare February 26, 2026 16:29
@github-actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/date/date-locale-hour (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/pr/bounded-memory (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/tail/retry is no longer failing!
Congrats! The gnu test tests/expand/bounded-memory is now passing!
Congrats! The gnu test tests/tail/pipe-f is now passing!

- test_rm_force_ignores_symlink_metadata_error: verifies that rm -f
  silently succeeds when a path component is not a directory (ENOTDIR),
  matching GNU rm behaviour (the ignorable test case).
- test_rm_reports_error_for_symlink_metadata_failure: verifies that
  without -f, the same path causes a non-zero exit with an error
  referencing the problematic path.
@github-actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/date/date-locale-hour (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/misc/io-errors is no longer failing!
Note: The gnu test tests/printf/printf-surprise is now being skipped but was previously passing.
Congrats! The gnu test tests/basenc/bounded-memory is now passing!
Congrats! The gnu test tests/dd/no-allocate is now passing!

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