bpo-28356: Document os.rename() behavior on Windows for differing filesystems #27376

rhyn0 · 2021-07-26T19:53:11Z

https://bugs.python.org/issue28356

zooba · 2021-07-28T18:32:06Z

Doc/library/os.rst

@@ -2292,6 +2292,8 @@ features:
   will fail with an :exc:`OSError` subclass in a number of cases:

   On Windows, if *dst* exists a :exc:`FileExistsError` is always raised.
+   The operation will fail if *src* and *dst* are on different filesystems. Use


I'd say it "may" fail, and move this out of the "if dst exists" section (which is introduced in the preceding paragraph).

Hi Zooba,

Thanks for the comment. The original (bpo-28356) talks about this, the main point of this documentation update is to alert users that it automatically will fail when moving across different disk drives (file systems).

This is something that is explained in errors in Python 3.8.10 but is missed in the documentation. See my terminal

As for the other point, I read it as being grouped together with Windows, as the next paragraph talks about Unix systems. I could rewrite the whole Windows paragraph, but I thought it better to tack it on to existing content.

I will leave it alone for now unless there is more discussion about this on either the bpo or in this thread.

I am not a Windows dev, but @eryksun also said on the issue that it will always fail because of the specific Windows API call used in the implementation. On the other hand, perhaps we should say "may" so we can get away with changing the behavior in the future if we want.

As for the placement, the current docs (https://docs.python.org/3.10/library/os.html#os.rename) aren't very clear. The Unix paragraph has a bunch of other information beyond behavior if dst already exists. I'd actually suggest removing the "If dst exists, the operation will fail with an OSError subclass in a number of cases:" sentence completely. That way, we just have one paragraph talking about Windows oddities and one talking about Unix oddities.

The implementation of os.rename() in Windows calls MoveFileExW() without the flag MOVEFILE_COPY_ALLOWED. The documentation states that "[i]f the destination is on another drive, you must [emphasis added] set the MOVEFILE_COPY_ALLOWED flag in dwFlags". This flag is not used by os.rename() or os.replace() because the destination is not supposed to be a copied file that has a new ID in the system.

POSIX rename() allows a system to support links across file systems, but the inode number -- and presumably the (dev, ino) tuple that uniquely identifies a file in the system -- must remain the same. I haven't seen this implemented. I've only ever seen rename fail with EXDEV in this case.

That said, using "may fail" is fine. A new version of NTFS and the Windows I/O manager could implement a type of reparse point that uses the FILE_ID_INFO, which in theory could be used to rename and hard link across two NTFS file systems.

Yeah, there's a difference between "will always fail with the current implementation" and "will always fail for all time". This is the first case, but if we write it in the docs, we've made it the second case.

Better to leave ourselves open here, in case we (or Microsoft) decide to enable it later. Then we don't need to deprecate the function for two releases before changing it.

github-actions · 2021-09-02T00:05:06Z

This PR is stale because it has been open for 30 days with no activity.

JelleZijlstra · 2022-04-03T22:48:18Z

Misc/NEWS.d/next/Documentation/2021-07-26-19-57-21.bpo-28356._F5a04.rst

@@ -0,0 +1 @@
+The documentation about os.rename() used on Windows systems updated to reflect behavior about moving across filesystems. Patch by Ryan Ozawa


Suggested change

The documentation about os.rename() used on Windows systems updated to reflect behavior about moving across filesystems. Patch by Ryan Ozawa

Document the behavior of :func:`os.rename` when moving across filesystems on Windows. Patch by Ryan Ozawa.

Shorter, and NEWS entries should be in the imperative

JelleZijlstra · 2022-04-03T22:54:53Z

Doc/library/os.rst

@@ -2292,6 +2292,8 @@ features:
   will fail with an :exc:`OSError` subclass in a number of cases:

   On Windows, if *dst* exists a :exc:`FileExistsError` is always raised.
+   The operation will fail if *src* and *dst* are on different filesystems. Use


I am not a Windows dev, but @eryksun also said on the issue that it will always fail because of the specific Windows API call used in the implementation. On the other hand, perhaps we should say "may" so we can get away with changing the behavior in the future if we want.

As for the placement, the current docs (https://docs.python.org/3.10/library/os.html#os.rename) aren't very clear. The Unix paragraph has a bunch of other information beyond behavior if dst already exists. I'd actually suggest removing the "If dst exists, the operation will fail with an OSError subclass in a number of cases:" sentence completely. That way, we just have one paragraph talking about Windows oddities and one talking about Unix oddities.

slateny · 2022-12-06T08:02:38Z

@rhyn0 Would you be interested in updating the PR per the reviews?

zooba · 2022-12-06T17:33:14Z

This doesn't require a NEWS entry at all, it can be removed rather than updated.

slateny · 2022-12-24T18:24:29Z

Doc/library/os.rst

@@ -2292,6 +2292,8 @@ features:
   will fail with an :exc:`OSError` subclass in a number of cases:


I think it's better to keep this sentence since explicitly stating that all the related exceptions below are subclasses of OSError could be helpful for easily catching the exceptions, but I can remove it nonetheless if wanted here.

slateny · 2022-12-24T18:26:35Z

Doc/library/os.rst

@@ -2292,6 +2292,8 @@ features:
   will fail with an :exc:`OSError` subclass in a number of cases:

   On Windows, if *dst* exists a :exc:`FileExistsError` is always raised.
+   The operation may fail if *src* and *dst* are on different filesystems. Use
+   :func:`shutil.move` to support moves to a different filesystem.


I believe since this statement is Windows-specific, it's fine to keep it under the Windows section despite being slightly unrelated to the general case of dst existing. But let me know if it's preferred to open up a new paragraph above/below (or if removing the sentence on dst exists is preferred).

zooba · 2023-01-09T21:42:48Z

Thanks for all the changes!

miss-islington · 2023-01-09T21:43:29Z

Thanks @rhyn0 for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

miss-islington · 2023-01-09T21:43:29Z

Thanks @rhyn0 for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

bedevere-bot · 2023-01-09T21:43:40Z

GH-100896 is a backport of this pull request to the 3.11 branch.

bedevere-bot · 2023-01-09T21:43:45Z

GH-100897 is a backport of this pull request to the 3.10 branch.

…umes (pythonGH-27376) (cherry picked from commit e098137) Co-authored-by: Ryan Ozawa <[email protected]>

…umes (GH-27376) (cherry picked from commit e098137) Co-authored-by: Ryan Ozawa <[email protected]>

add initial changes to os.rename for Windows behavior

7683169

bedevere-bot added the awaiting review label Jul 26, 2021

the-knights-who-say-ni added the CLA signed label Jul 26, 2021

bedevere-bot added the docs Documentation in the Doc dir label Jul 26, 2021

blurb-it bot and others added 2 commits July 26, 2021 19:57

📜🤖 Added by blurb_it.

b278005

Remove typo

2883af8

zooba reviewed Jul 28, 2021

View reviewed changes

github-actions bot added the stale Stale PR or inactive for long period of time. label Sep 2, 2021

JelleZijlstra reviewed Apr 3, 2022

View reviewed changes

ezio-melotti removed the CLA signed label Jul 13, 2022

github-actions bot removed the stale Stale PR or inactive for long period of time. label Aug 11, 2022

zooba added the skip news label Dec 6, 2022

Change "will" to "may"; remove news

536d241

slateny reviewed Dec 24, 2022

View reviewed changes

zooba merged commit e098137 into python:main Jan 9, 2023

bedevere-bot removed the awaiting review label Jan 9, 2023

zooba added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Jan 9, 2023

bedevere-bot removed the needs backport to 3.11 only security fixes label Jan 9, 2023

bedevere-bot removed the needs backport to 3.10 only security fixes label Jan 9, 2023

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 9, 2023

bpo-28356: Document os.rename() behavior on Windows for differing vol…

f7c10a5

…umes (pythonGH-27376) (cherry picked from commit e098137) Co-authored-by: Ryan Ozawa <[email protected]>

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 9, 2023

bpo-28356: Document os.rename() behavior on Windows for differing vol…

88d0f0e

…umes (pythonGH-27376) (cherry picked from commit e098137) Co-authored-by: Ryan Ozawa <[email protected]>

miss-islington added a commit that referenced this pull request Jan 9, 2023

bpo-28356: Document os.rename() behavior on Windows for differing vol…

5c4afdf

…umes (GH-27376) (cherry picked from commit e098137) Co-authored-by: Ryan Ozawa <[email protected]>

miss-islington added a commit that referenced this pull request Jan 9, 2023

bpo-28356: Document os.rename() behavior on Windows for differing vol…

4e096ec

…umes (GH-27376) (cherry picked from commit e098137) Co-authored-by: Ryan Ozawa <[email protected]>

stephan mannequin mentioned this pull request May 11, 2023

[easy doc] Document os.rename() behavior on Windows when src and dst are on different filesystems #72542

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-28356: Document os.rename() behavior on Windows for differing filesystems #27376

bpo-28356: Document os.rename() behavior on Windows for differing filesystems #27376

rhyn0 commented Jul 26, 2021 •

edited by bedevere-bot

Loading

zooba Jul 28, 2021

rhyn0 Aug 2, 2021

JelleZijlstra Apr 3, 2022

eryksun Apr 4, 2022

zooba Apr 4, 2022

github-actions bot commented Sep 2, 2021

JelleZijlstra Apr 3, 2022

JelleZijlstra Apr 3, 2022

slateny commented Dec 6, 2022

zooba commented Dec 6, 2022

slateny Dec 24, 2022

slateny Dec 24, 2022

zooba commented Jan 9, 2023

miss-islington commented Jan 9, 2023

miss-islington commented Jan 9, 2023

bedevere-bot commented Jan 9, 2023

bedevere-bot commented Jan 9, 2023

		@@ -0,0 +1 @@
		The documentation about os.rename() used on Windows systems updated to reflect behavior about moving across filesystems. Patch by Ryan Ozawa

	The documentation about os.rename() used on Windows systems updated to reflect behavior about moving across filesystems. Patch by Ryan Ozawa
	Document the behavior of :func:`os.rename` when moving across filesystems on Windows. Patch by Ryan Ozawa.

		@@ -2292,6 +2292,8 @@ features:
		will fail with an :exc:`OSError` subclass in a number of cases:

bpo-28356: Document os.rename() behavior on Windows for differing filesystems #27376

bpo-28356: Document os.rename() behavior on Windows for differing filesystems #27376

Conversation

rhyn0 commented Jul 26, 2021 • edited by bedevere-bot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 2, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slateny commented Dec 6, 2022

zooba commented Dec 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zooba commented Jan 9, 2023

miss-islington commented Jan 9, 2023

miss-islington commented Jan 9, 2023

bedevere-bot commented Jan 9, 2023

bedevere-bot commented Jan 9, 2023

rhyn0 commented Jul 26, 2021 •

edited by bedevere-bot

Loading