[PATCH] D49860: [dsymutil] Simplify temporary file handling.

[Jeremy Morse via Phabricator via llvm-commits]

jmorse accepted this revision.
jmorse added a comment.
This revision is now accepted and ready to land.

LGTM, I get 1 Unsupported (due to Darwin) and 44 successful passes when running lit over the dsymutil\X86\ tests, Windows 10 with a MSVC 2015 build environment. No failures.

Not clear what to do about having MoveFileEx move between volumes, see inline comment, it might best to add the "copy allowed" flag in a follow up commit that's reviewed by someone more familiar with LLVMs filesystem handling.



================
Comment at: llvm/lib/Support/Path.cpp:1163
+    RenameEC = sys::fs::copy_file(TmpName, Name);
+    // If we can't rename or copy, discard the temporary file.
+    if (RenameEC)
----------------
JDevlieghere wrote:
> Should we have something similar for Windows?
The Windows implementation uses MoveFileEx and explicitly supports that kind of rename:

    https://docs.microsoft.com/en-us/windows/desktop/api/winbase/nf-winbase-movefileexa

But it wants a flag, "MOVEFILE_COPY_ALLOWED" in the call to MoveFileEx in that case. Adding that to the MoveFileExW call in Windows' Path.inc would be sufficient: off the top of my head I can't imagine circumstances where this interfered with other LLVM code.


Repository:
  rL LLVM

https://reviews.llvm.org/D49860