Reimplement Sys.rename under Win32 to be more POSIX

[xavierleroy]

Currently, in the MSVC and Mingw ports of OCaml, Sys.rename src dst fails if a file named dst already exists. This is unlike the other ports, where dst is replaced by src in this case. This PR reimplements Sys.rename on top of the MoveFileEx Win32 system call so that the destination file, if it exists already, is replaced by the source file.

The "replace existing file" behavior is more useful especially in cases where one wants to prepare data in a temporary file with a unique name, then give it its final name, which can match an existing file. See MPR 4991 and MPR 7472 for possible uses within the OCaml compiler itself.

The "replace existing file" behavior is even more useful if it is guaranteed to be atomic. POSIX gives such guarantees for the rename function, but MSDN gives no atomicity guarantees for MoveFileEx nor for ReplaceFile. It is believed that MoveFileEx is atomic on a journaling file system such as NTFS and if no copy of contents is requested. Same beliefs for ReplaceFile, except that the latter is not really usable because it expects the destination name to exist already, and fails otherwise. In the end we cannot guarantee atomicity of Sys.rename but we've made a best effort.

[alainfrisch]

• What happens when the target file is an existing directory? Is the directory removed?

• MSDN says that "If a file named lpNewFileName exists, the function replaces its contents with the contents of the lpExistingFileName file, ...". Does that mean that only the contents of the file is changed, but its other properties are kept from the old file named lpNewFileName?

• Do you know if the behavior w.r.t. symlinks is the same as POSIX?

Same beliefs for ReplaceFile, except that the latter fails when the destination name does not exist, which is not what we want.

You meant "when the destination name exists", right?

In the end we cannot guarantee atomicity of Sys.rename but we've made a best effort.

I think this is fine. Even under Unix, NFS mounts don't guarantee atomicity for rename.

[xavierleroy]

What happens when the target file is an existing directory? Is the directory removed?

My guess and my hope is that it fails in this case, just like POSIX rename fails. I'll test but not until tomorrow. You're welcome to try!

MSDN says that "If a file named lpNewFileName exists, the function replaces its contents with the contents of the lpExistingFileName file, ...". Does that mean that only the contents of the file is changed, but its other properties are kept from the old file named lpNewFileName?

Quite possibly. There is similar wording for ReplaceFile that makes it sound like extended attributes (ACLs, etc) of the destination are kept. Again, testing should tell.

Do you know if the behavior w.r.t. symlinks is the same as POSIX?

I don't know. Testing should tell.

Same beliefs for ReplaceFile, except that the latter fails when the destination name does not exist, which is not what we want.
You meant "when the destination name exists", right?

I meant that ReplaceFile fails when it is told to replace a file DST with a file SRC and the file DST doesn't exist already. True to its name... but rather useless in my opinion.

[alainfrisch]

There is similar wording for ReplaceFile that makes it sound like extended attributes (ACLs, etc) of the destination are kept.

I confirmed, it would be worth mentioning that in the doc-comment for the function, as there could be implications in term of security (among others).

[alainfrisch]

What about dealing with Unix.rename as well?

[fdopen]

Sys.rename is now also "more POSIX" in another aspect: It's not longer possible to move a file to a different volume with Sys.rename, instead it will fail with EXDEV like under POSIX. Microsoft's rename adds MOVEFILE_COPY_ALLOWED - as does Unix.rename.
This difference between Sys.rename and Unix.rename is neither intuitive nor documented.

[xavierleroy]

@fdopen : you're right, the previous implementation of Sys.rename under Win32 would do a copy if old and new names are on different file systems. I wasn't aware of this. I guess it's best to keep this behavior in the new implementation, for backward compatibility. Commit [41f5ed5] does this.

@alainfrisch : Unix.rename already did "the right thing" under Windows because it uses MoveFileEx if available. Since MoveFileEx is now available on all Windows versions we support, commit [4f4e1ef] simplifies the implementation but should not change the behavior. Some tests added in [01433ad].

@alainfrisch : commit [3cd2a99] documents Sys.rename in more details.

Can I get my formal review now?

[alainfrisch]

@xavierleroy Do you know why Unix.rename does not release the runtime lock (Sys.rename does it)?

[alainfrisch]

LGTM. Extra possible tweaks: (i) release the runtime lock in Win32's version of Unix.rename; (ii) bring the documentation of Unix.rename up-to-date with Sys.rename (or refer to it).

[xavierleroy]

The Unix implementation of Unix.rename does release the runtime lock, but not the Win32 implementation. Originally neither implementations would release the runtime lock because rename() doesn't block. Then some people noticed that on an NFS file system operations such as rename() can take a long time. Then they added runtime lock operations but only for the Unix implementations.

[xavierleroy]

Documentation of Unix.rename improved. This is ready for merging.

[alainfrisch]

Then some people noticed that on an NFS file system operations such as rename() can take a long time.

The Win32 version can copy files so it can also take a long time. Ok, perhaps something for later (or people can use Sys.rename...).

[alainfrisch]

Just for the records: it seems that even this new implementations doesn't allow renaming A to B if B exists and is currently opened by any process (even with FILE_SHARE_DELETE). If anyone knows a way to make this works, I'm interested!