Skip to content

Conversation

fangli
Copy link

@fangli fangli commented Sep 17, 2023

gh-109503: Fix document for shutil.move on usage of os.rename since it's inaccurate

The document/docstring for shutil.move states:

If the destination is on the current filesystem, then :func:os.rename is used.
Otherwise, src is copied to dst using copy_function and then removed.

which seems to be incorrect. The actual behavior shows below:

  • If src and target are on the same filesystem, os.rename is preferred, which is an atomic operation.
  • But, if os.rename fails due to some reason (e.g. permission denied), then shutil.move will silently fallback to nonatomic copy+remove.

This fallback behavior is crucial in case atomic operation matters. It creates false statement that an operation must be atomic, while in fact it is non-atomic in some cases. This behavior can be found in source code here.

For example

Given all following dirs and files on the SAME filesystem

// 600: rw_ ___ ___
// 700: rwx ___ ___

userA:groupA 700  /dirSrc
userA:groupA 600  /dirSrc/fileSrc

userB:groupB 700  /dirTarget
userA:groupA 600  /dirTarget/fileTarget

When the userA calls:

# Run by userA
shutil.move('/dirSrc/fileSrc', '/dirTarget/fileTarget')
# Move succeeded

Based on current doc, user expects that the move must be atomic since both files are on the same filesystem. But in fact, this operation will fall back to copy+del silently, which is not atomic.


📚 Documentation preview 📚: https://cpython-previews--109507.org.readthedocs.build/

@ghost
Copy link

ghost commented Sep 17, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

Nonatomic move might be used even if the files are
on the same filesystem in some cases.
@serhiy-storchaka serhiy-storchaka added the docs Documentation in the Doc dir label Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review docs Documentation in the Doc dir
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

2 participants