-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
rm: don't treat symlinks as write-protected #10232
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rm: don't treat symlinks as write-protected #10232
Conversation
f3f691e to
aad33db
Compare
CodSpeed Performance ReportMerging this PR will improve performance by 4.84%Comparing Summary
Performance Changes
Footnotes
|
looks like entirely related to querying symlink metadata, I tried reversing the logic but found the branch needs to be always taken to keep the functionality |
6ef66b5 to
c2a1956
Compare
|
I'm trying to see if theres a way to deduplicate the metadata call so that theres no performance regression, I think it should be possible only have one call |
Re-evaluating after double checking the performance regressions
c2a1956 to
b264abb
Compare
GNU rm does not check for write-protection on symbolic links; it instead prompts to "remove symbolic link" regardless of the link's permissions or its target's status. This change: - Ensures `prompt_file` checks for symlinks specifically using `symlink_metadata`, avoiding the incorrect "write-protected" prompt. - Refactors permission checks into `is_writable_metadata` to allow using the already-fetched metadata, which also optimizes performance by reducing redundant `stat` calls. - Updates `prompt_file_permission_readonly` to operate on metadata directly.
b264abb to
d479e9b
Compare
|
GNU testsuite comparison: |

Detect symlinks in prompt_file() using fs::symlink_metadata() and handle them differently:
Adds a unit test test_symlink_not_write_protected_prompt to verify that a symlink to a read-only file is not treated as a write-protected regular file.
Closes #10222