[PATCH] D30676: Add llvm::sys::fs::remove_directories()

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: llvm/lib/Support/Path.cpp:933
+
+    EC = remove(Item.path());
+    if (EC && !IgnoreErrors)
----------------
amccarth wrote:
> This is the usual Posix approach, so I'm concerned it won't be reliable on Windows.
> 
> When you delete files from a directory on Windows, they are marked for deletion and actually deleted later, typically milliseconds later, but it can actually be even longer.  So when you try to delete the directory, it may not actually be empty yet.
> 
> The correct pattern on Windows is to rename the files out of directory (to some temp location) and then delete them.  The rename, if on the same volume, is synchronous, so you know that the directory is actually empty, even if the delete hasn't happened yet.  Is this what the implementation of remove does on Windows?
Windows implementation of remove() here: https://github.com/llvm-project/llvm-project/blob/master/llvm/lib/Support/Windows/Path.inc#L251


https://reviews.llvm.org/D30676