Skip to content
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

LocalFileSystem clobbers file permissions after cp_file #1524

Open
russell-horvath opened this issue Feb 6, 2024 · 4 comments · May be fixed by #1535
Open

LocalFileSystem clobbers file permissions after cp_file #1524

russell-horvath opened this issue Feb 6, 2024 · 4 comments · May be fixed by #1535

Comments

@russell-horvath
Copy link

Hi,

I am trying to copy a local file with the following permissions:

-rwxrwxr-x 1 rhorvath rhorvath 7994 

after copying the file it becomes:

-rw-rw-r--  1 rhorvath rhorvath 7994

I have narrowed the reason for this down to the cp_file function in LocalFileSystem where shutil.copyfile(path1, path2) doesn't copy the permission mode. This could be solved by using shutil.copy() and preserving the source file's permission mode.

@martindurant
Copy link
Member

could be solved by using shutil.copy() and preserving the source file's permission mode

Am I right in remembering that this does not do atomic operations, or that it operates differently across discs?

copyfile: "in the most efficient way possible"
copy: "data and mode bits" (may copy into t directory, not to the specific path given)

@Skylion007
Copy link
Contributor

This really should use shutil.copy2: https://docs.python.org/3/library/shutil.html#shutil.copy2

https://stackoverflow.com/questions/20873723/is-pythons-shutil-copyfile-atomic copyfile does not do atomic operations either so nothing will be lost. I think the issue you are worried about is the semantics if the destination path is a directory, but that can be handled with an isdir check.

Alternatively, if you really want to keep shutil.copyfile call, just add a copystat call as well.

shutil.copyfile and shutil.copy can both copy between disks, while the relevant os calls cannot.

@martindurant
Copy link
Member

can be handled with an isdir check

We would rather not add checks if possible - it turns out that things can go really slowly with bulk file operations.

Would you like to propose the PR?

Skylion007 added a commit to Skylion007/filesystem_spec that referenced this issue Mar 5, 2024
@Skylion007 Skylion007 linked a pull request Mar 5, 2024 that will close this issue
@Skylion007
Copy link
Contributor

This stackoverflow post gives a good behavior of what the different shutil calls do:
https://stackoverflow.com/a/44996087

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 a pull request may close this issue.

3 participants