fs::rename fails when dest exists only on windows

[brson]

This code fails on windows, not on linux:

use std::fs;

fn main() {
    let src = "testdir1";
    let dest = "testdir2";
    fs::create_dir(src).unwrap();
    fs::create_dir(dest).unwrap();
    fs::rename(src, dest).unwrap();
}

with

thread '<main>' panicked at 'called `Result::unwrap()` on an `Err` value: Error { repr: Os { code: 5, message: "Access is denied." } }', ../src/libcore\result.rs:741

[alexcrichton]

I opened this awhile ago as #15836 and closed that as "not a bug" because we're just exporting the system's behavior. I'm personally inclined to say that this is something we shouldn't handle in the standard library (but may wish to document)

[nodakai]

FYI, Java 7 introduced java.nio.file.Files.move() rather than fixing an decades-old API java.io.File.renameTo()

[retep998]

Interestingly, the option MOVEFILE_REPLACE_EXISTING states This value cannot be used if lpNewFileName or lpExistingFileName names a directory. so even with that you wouldn't be able to overwrite an existing directory. That said since RemoveDirectory only works on empty directories, a really easy way to workaround this is if the first move fails, attempt to remove the directory and if that succeeds then attempt the move again.

[alexcrichton]

I'd be wary of making a core operation like this do more than one I/O operation as it'd be tough to ensure that it remains "transactional".

We already attempt to make Windows/Unix behave the same by passing the flag @retep998 mentioned, and the remaining behavior is something I'd prefer to just document.

[retep998]

I'm not suggesting that the standard library do that, documenting the current behavior should be sufficient. It's just a workaround that users can easily use right now if they do want that sort of behavior on Windows.

[barosl]

I tested MoveFileEx with MOVEFILE_REPLACE_EXISTING on Windows, and the explanation on MSDN seems to be a bit different from the actual implementation. lpNewFileName must not be a directory, yes, but lpExistingFileName can be a directory. So replacing a file with a directory using MoveFileEx is entirely possible (!). Note that the rename POSIX API demands both from and to are of the same type, which prevents such renaming/replacing.