cp: fail when trying to copy to read only file on mac#5261
cp: fail when trying to copy to read only file on mac#5261tertsdiepraam merged 5 commits intouutils:mainfrom
Conversation
|
Cool! Thanks! Since you're on mac, would you be able to figure out if this is at all related to the |
|
Looking at the man page for cp on mac it looks like there is a flag to use clonefile for the copy |
|
Interesting, I can't find |
|
Can't find it online but here what man cp on my mac shows |
|
|
|
reading through reflink behavior it seems like these changes should match whats expected in gnu cp. Seems like |
tertsdiepraam
left a comment
There was a problem hiding this comment.
Hi! Thanks for picking this up again! This is a trickier issue than it might appear, because the whole file removal thing might be wrong...
For example, the linux equivalent first opens the files for reading, before using a syscall. Maybe that's the better way to approach this. For example using fclonefileat instead of clonefile. It's a bit unfortunate I don't have a mac, so I can't check this for myself. Could you maybe check whether that would work?
If not then I think we should merge (a simplified version) of this patch, because it still fixes an important issue, maybe with a comment to state that it is a workaround and that this code should be expanded and rewritten.
|
No problem happy to help out! I can take a look at reimplementing this to match the Linux version better this week. |
|
looking at |
|
Hi! Sorry for not responding for a while, I was very busy... Anyway, let's pick this back up! I think the |
|
Yeah, I think merging this first is good. Should be ready to go |
- Add 10 new test functions covering readonly destination behavior - Tests cover basic readonly copying, flag combinations, and edge cases - Include macOS-specific clonefile behavior tests - Ensure readonly file protection from PR uutils#5261 cannot regress - Tests provide evidence for closing issue uutils#5349
- Add 10 new test functions covering readonly destination behavior - Tests cover basic readonly copying, flag combinations, and edge cases - Include macOS-specific clonefile behavior tests - Ensure readonly file protection from PR uutils#5261 cannot regress - Tests provide evidence for closing issue uutils#5349
- Add 10 new test functions covering readonly destination behavior - Tests cover basic readonly copying, flag combinations, and edge cases - Include macOS-specific clonefile behavior tests - Ensure readonly file protection from PR uutils#5261 cannot regress - Tests provide evidence for closing issue uutils#5349
* feat: add comprehensive readonly file regression tests for cp - Add 10 new test functions covering readonly destination behavior - Tests cover basic readonly copying, flag combinations, and edge cases - Include macOS-specific clonefile behavior tests - Ensure readonly file protection from PR #5261 cannot regress - Tests provide evidence for closing issue #5349 * perf: optimize readonly regression tests with batched I/O operations - Reduce file I/O overhead by batching file operations - Consolidate setup operations to minimize system calls - Improve test execution time from 0.44s to 0.27s (38% improvement) - Maintain comprehensive test coverage for readonly file behavior * fix: remove duplicate tests and trivial comments per PR feedback - Remove test_cp_readonly_dest_regression (duplicate of test_cp_dest_no_permissions) - Remove test_cp_readonly_dest_with_force (duplicate of test_cp_arg_force) - Remove test_cp_readonly_dest_with_remove_destination (duplicate of test_cp_arg_remove_destination) - Remove test_cp_macos_clonefile_readonly (duplicate of test_cp_existing_target) - Remove test_cp_normal_copy_still_works (duplicate of test_cp_existing_target) - Remove trivial performance comments from readonly tests - Keep existing proven tests per maintainer preferences - Keep unique readonly tests that provide additional coverage
* feat: add comprehensive readonly file regression tests for cp - Add 10 new test functions covering readonly destination behavior - Tests cover basic readonly copying, flag combinations, and edge cases - Include macOS-specific clonefile behavior tests - Ensure readonly file protection from PR uutils#5261 cannot regress - Tests provide evidence for closing issue uutils#5349 * perf: optimize readonly regression tests with batched I/O operations - Reduce file I/O overhead by batching file operations - Consolidate setup operations to minimize system calls - Improve test execution time from 0.44s to 0.27s (38% improvement) - Maintain comprehensive test coverage for readonly file behavior * fix: remove duplicate tests and trivial comments per PR feedback - Remove test_cp_readonly_dest_regression (duplicate of test_cp_dest_no_permissions) - Remove test_cp_readonly_dest_with_force (duplicate of test_cp_arg_force) - Remove test_cp_readonly_dest_with_remove_destination (duplicate of test_cp_arg_remove_destination) - Remove test_cp_macos_clonefile_readonly (duplicate of test_cp_existing_target) - Remove test_cp_normal_copy_still_works (duplicate of test_cp_existing_target) - Remove trivial performance comments from readonly tests - Keep existing proven tests per maintainer preferences - Keep unique readonly tests that provide additional coverage
Fixes issue #5257 by first checking if file is read only or not before removing file and trying clonefile again