Comment 1:

Should os.Replace handle the EXDEV case on Unix systems?  (On Unix os.Rename will fail
when trying to move a file to a different mounted file system).

Labels changed: added repo-main, release-go1.5.

Status changed to Accepted.

Comment 2 by MartyMacGyver:

I don't think that (at this point) os.Replace() should have extended functionality
beyond what os.Rename offers (e.g., os.Rename() isn't supposed to work across
filesystems). Atomicity with os.Replace() varies with filesystem - though it would be
implicit for Posix when using the fallthrough to os.Rename(), Windows requires more
finesse. The major functional change versus os.Rename() is handling Windows properly,
where one would use the MoveFileEx API *without* the MOVEFILE_COPY_ALLOWED flag to
maintain parity with Posix-style rename.
For the sake of atomicity I understand why this can't all be done in os.Rename(), but
os.Replace() is meant to loosen that guarantee for select OSes, not remove it. If kept
simple, (and documented as such), devs will know that on Posix hosts this will be
exactly the same as os.Rename, and on Windows hosts this will do its best but is
inherently non-Posix in nature (versus doing nothing at all in the case of such
non-Posix hosts).
** When one starts adding functionality such as the ability to move across filesystems
it quickly complicates things while breaking the guarantee of atomicity across the
board. If one wants cross-filesystem functionality (where atomic (or nearly-atomic)
behavior is not expected at all), one would use os.Copy() or os.Move() - which are
notable in their absence in Go. That's another wheel that people seem to reinvent a lot,
but it's outside of the scope of this issue except for the point that once you start
adding that level of functionality you are effectively creating those functions as well.
Better to keep that extended effort separate.

Comment 3:

What package and function name do you propose?

Comment 4 by MartyMacGyver:

Package os, function signature the same as for Rename(), e.g.,
    func Replace(oldpath, newpath string) error

I don't think there's consensus on a plan here (or any pending CL), and Go 1.5 was frozen as of last Friday. So I'm bumping this to milestone Go 1.6.

Note that commit 92c5736 did change os.Rename's behavior on Windows, though:

commit 92c57363e0b4d193c4324e2af6902fe56b7524a0
Author: Daniel Theophanes <kardianos@gmail.com>
Date:   Thu Feb 26 12:10:11 2015 -0800

    os: windows Rename should overwrite destination file.

    Rename now uses MoveFileEx which was previously not available to
    use because it is not supported on Windows 2000.

This bug was originally created asking for an os.Replace with consistent behavior on Windows, because #3366 (fix os.Rename) was closed as impossible.

I'm changing the topic of this bug to fix os.Rename, effectively replacing #3366. I don't believe package os should be inventing new functionality, but I do think that it should strive for consistent functionality in what's there. (There is for example another open bug for making os.RemoveAll work with read-only files on Windows.)

Based on http://stackoverflow.com/questions/167414/is-an-atomic-file-rename-with-overwrite-possible-on-windows it looks like it may be possible to just fix os.Rename on Windows. In particular, it sounds like ReplaceFile is the right call to make.

If we decide that it is truly impossible to implement os.Rename on Windows then we could fall back to a discussion of a new function, but I'd rather take all the existing code using os.Rename and make it start working than make it obsolete.

This is difficult to determine the correct winapi to call here. There are typically three winapi calls suggested:

• MoveFileEx
  • https://msdn.microsoft.com/en-us/library/windows/desktop/aa365240(v=vs.85).aspx
• ReplaceFile
  • https://msdn.microsoft.com/en-us/library/windows/desktop/aa365512(v=vs.85).aspx
• MoveFileTransacted
  • https://msdn.microsoft.com/en-us/library/windows/desktop/aa365241(v=vs.85).aspx

MoveFileEx is what go currently has in go1.5 and tip. When this bug was written go used "MoveFile".

MoveFileTransacted has two strikes against it: 1) It is only supported on Vista+ and 2) MS says it may remove all the transacted APIs, including this one.

Is ReplaceFile atomic? ReplaceFile has error code ERROR_UNABLE_TO_MOVE_REPLACEMENT which states:

The replacement file could not be renamed. If lpBackupFileName was specified, the replaced and replacement files retain their original file names. Otherwise, the replaced file no longer exists and the replacement file exists under its original name.

It also only works on files. However a research paper from MS claims that ReplaceFile is atomic: http://research.microsoft.com/pubs/64525/tr-2006-45.pdf . I suspect that this is a mistake.

MoveFileEx has many flags. Of interest here are two in particular:
MOVEFILE_REPLACE_EXISTING and MOVEFILE_COPY_ALLOWED. Currently in go we only use MOVEFILE_REPLACE_EXISTING and thus Rename will move if attempted between different volumes. However only the MOVEFILE_COPY_ALLOWED flag suggests multiple operations are used.

I think what we currently do is the correct thing to do on Windows.

I'm changing the topic of this bug to fix os.Rename, effectively replacing #3366.

@kardianos didn't you fix issue #3366 with CL 6140? If I am not mistaken, you have even added new TestRenameOverwriteDest that tests the problem raised in issue #3366.

@rsc what exactly do you want us to do?

Alex

@alexbrainman That was my impression as well. I just wanted to be through in my analysis.

The original report says:

In Windows this does not work correctly as designed and fails when replacing an existing file.

I interpreted the original report as saying that

os.Create("x")
os.Create("y")
os.Rename("x", "y")

fails on Windows because y already exists.

If that is true, then I think we should fix that. If it's not true, then I think there is nothing to do here. I admit that I took the report at face value and did not investigate whether that is true.

I agree @alexbrainman that TestRenameOverwriteDest seems to suggest this example already works. Given that, I think we can close the bug. Sorry for the confusion.