-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
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 #5261
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good! Do you want to pick up that followup PR as well? I made a new issue for this, if you're interested in picking this up just respond there.
Fixes issue #5257 by first checking if file is read only or not before removing file and trying clonefile again