chmod: report the real error when a target's metadata is inaccessible - #13637
Conversation
Sorry, something went wrong.
|
GNU testsuite comparison: Note: The gnu test tests/seq/seq-epipe is now being skipped but was previously passing. |
Sorry, something went wrong.
|
a few jobs are failing |
Sorry, something went wrong.
chmod used Path::exists() to decide whether a target exists, but exists() returns false on any metadata error, so a file whose parent directory lacks search permission was reported as "No such file or directory" instead of "Permission denied". Use try_exists() to distinguish a genuine ENOENT from a permission error, matching GNU. Fixes uutils#9789.
Merging this PR will improve performance by 22.62%⚠️ Different runtime environments detected
⚡ 1 improved benchmark Performance Changes
Tip Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent. Comparing 0xfandom:chmod-inaccessible-permission-denied (6dadcce) with main (a730551) Footnotes
|
Sorry, something went wrong.
Satisfy clippy::unreadable_literal (0o100644 -> 0o100_644) so the Style and Lint job passes.
|
Thanks for the heads up. The failing job was clippy's unreadable_literal on the new test — I'd written the octal mode as 0o100644. Pushed a fixup that changes it to 0o100_644, and clippy is clean locally now. The CodSpeed report flags a ~3.5% change on du_wide_tree, but that's an unrelated du benchmark this PR doesn't touch, and the report itself notes it compared across different runtime environments. Should be noise. Let me know if you'd like anything else adjusted. |
Sorry, something went wrong.
|
Thanks! |
Sorry, something went wrong.
Fixes #9789.
chmod used Path::exists() to decide whether a target exists. Path::exists() returns false for any metadata access error, including permission errors, so a file whose parent directory lacks search permission was reported as:
instead of GNU's:
This switches the non-recursive path to Path::try_exists(), which returns Ok(false) only for a genuine ENOENT and Err(..) for a permission error. A permission error is now reported via the existing ChmodError::PermissionDenied variant, whose message already matches GNU. Any other unexpected metadata error falls through to the normal chmod attempt so it can surface a precise message.
Added a regression test (test_chmod_inaccessible_file_reports_permission_denied) for the non-recursive case; it fails on the old Path::exists() code (prints "No such file or directory") and passes with the fix. The existing permission-denied tests only covered the recursive (-R) path.