Add llvm::sys::fs::remove_directories()

[zturner]

Summary

We have create_directories() which can create an entire tree,
and remove(), which can remove an empty directory, but not
a remove_directories() which can remove an entire tree.

This patch adds such a function.

[mehdi_amini]

LGTM

llvm/lib/Support/Path.cpp

937

I assume removing entries in a directory does not invalidate the iterator?

[amccarth]

llvm/lib/Support/Path.cpp

933

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?

[mehdi_amini]

llvm/lib/Support/Path.cpp

933

Windows implementation of remove() here: https://github.com/llvm-project/llvm-project/blob/master/llvm/lib/Support/Windows/Path.inc#L251

[zturner]

Updated to use a more robust method on Windows. Also, on non-Windows platforms unfortunately directory_iterator will follow symlinks when statting, so directory symlinks will look like ordinary directories. This is a serious problem for a recursive delete function, because we will walk into the child and delete everything in it.

To err on the side of caution, I've updated directory_iterator and recursive_directory_iterator to make following symlinks opt-out (this way all existing code is unchanged). In the deletion function specifically, I use this opt-out flag so that we do not follow symlinks, and we just delete the link itself.

Although it's not explicitly documented what the behavior of SHFileOperation is with respect to this, and despite the fact that there is a mysterious flag called SH_NORECURSEREPARSE which it claims is "unsupported", I've confirmed that with the code as written, the API does *not* recurse into reparse points (either junctions or symlinks).

[amccarth]

LGTM.

llvm/include/llvm/Support/FileSystem.h

 /// @brief Remove path. Equivalent to POSIX remove().
 ///
 /// @param path Input path.
 /// @returns errc::success if path has been removed or didn't exist, otherwise a
 ///          platform-specific error code. If IgnoreNonExisting is false, also
 ///          returns error if the file didn't exist.
 std::error_code remove(const Twine &path, bool IgnoreNonExisting = true);
 
+/// @brief Recursively delete a directory.
+///
+/// @param path Input path.
+/// @returns errc::success if path has been removed or didn't exist, otherwise a
+///          platform-specific error code.
+std::error_code remove_directories(const Twine &path, bool IgnoreErrors = true);
+
 /// @brief Rename \a from to \a to. Files are renamed as if by POSIX rename().
 ///
 /// @param from The path to rename from.
 /// @param to The path to rename to. This is created.
 std::error_code rename(const Twine &from, const Twine &to);
 
 /// @brief Copy the contents of \a From to \a To.
 ///

llvm/lib/Support/Path.cpp

if (Parent.empty())
     return EC;
 
   if ((EC = create_directories(Parent, IgnoreExisting, Perms)))
       return EC;
 
   return create_directory(P, IgnoreExisting, Perms);
 }
 
+template <typename T>
+static std::error_code remove_directories_impl(const T &Entry,
+                                               bool IgnoreErrors) {
+  std::error_code EC;
+  directory_iterator Begin(Entry, EC);
+  directory_iterator End;
+  while (Begin != End) {
+    auto &Item = *Begin;
+    file_status st;
+    EC = Item.status(st);
+    if (EC && !IgnoreErrors)
+      return EC;
+
+    if (is_directory(st)) {
+      EC = remove_directories_impl(Item, IgnoreErrors);
+      if (EC && !IgnoreErrors)
+        return EC;
+    }
+
+    EC = remove(Item.path());
+    if (EC && !IgnoreErrors)
+      return EC;
+
+    Begin.increment(EC);
+    if (EC && !IgnoreErrors)
+      return EC;
+  }
+  return std::error_code();
+}
+
+std::error_code remove_directories(const Twine &path, bool IgnoreErrors) {
+  auto EC = remove_directories_impl(path, IgnoreErrors);
+  if (EC && !IgnoreErrors)
+    return EC;
+  remove(path);
+  return std::error_code();
+}
+
 std::error_code copy_file(const Twine &From, const Twine &To) {
   int ReadFD, WriteFD;
   if (std::error_code EC = openFileForRead(From, ReadFD))
     return EC;
   if (std::error_code EC = openFileForWrite(To, WriteFD, F_None)) {
     close(ReadFD);
     return EC;
   }

llvm/unittests/Support/Path.cpp

TEST_F(FileSystemTest, DirectoryIteration) {
   }
   EXPECT_EQ(I, E);
   ASSERT_NO_ERROR(fs::remove(Twine(TestDirectory) + "/reclevel/a/b/c"));
   ASSERT_NO_ERROR(fs::remove(Twine(TestDirectory) + "/reclevel/a/b"));
   ASSERT_NO_ERROR(fs::remove(Twine(TestDirectory) + "/reclevel/a"));
   ASSERT_NO_ERROR(fs::remove(Twine(TestDirectory) + "/reclevel"));
 }
 
+TEST_F(FileSystemTest, Remove) {
+  SmallString<64> BaseDir;
+  SmallString<64> Paths[4];
+  int fds[4];
+  ASSERT_NO_ERROR(fs::createUniqueDirectory("fs_remove", BaseDir));
+
+  ASSERT_NO_ERROR(fs::create_directories(Twine(BaseDir) + "/foo/bar/baz"));
+  ASSERT_NO_ERROR(fs::create_directories(Twine(BaseDir) + "/foo/bar/buzz"));
+  ASSERT_NO_ERROR(fs::createUniqueFile(
+      Twine(BaseDir) + "/foo/bar/baz/%%%%%%.tmp", fds[0], Paths[0]));
+  ASSERT_NO_ERROR(fs::createUniqueFile(
+      Twine(BaseDir) + "/foo/bar/baz/%%%%%%.tmp", fds[1], Paths[1]));
+  ASSERT_NO_ERROR(fs::createUniqueFile(
+      Twine(BaseDir) + "/foo/bar/buzz/%%%%%%.tmp", fds[2], Paths[2]));
+  ASSERT_NO_ERROR(fs::createUniqueFile(
+      Twine(BaseDir) + "/foo/bar/buzz/%%%%%%.tmp", fds[3], Paths[3]));
+
+  for (int fd : fds)
+    ::close(fd);
+
+  EXPECT_TRUE(fs::exists(Twine(BaseDir) + "/foo/bar/baz"));
+  EXPECT_TRUE(fs::exists(Twine(BaseDir) + "/foo/bar/buzz"));
+  EXPECT_TRUE(fs::exists(Paths[0]));
+  EXPECT_TRUE(fs::exists(Paths[1]));
+  EXPECT_TRUE(fs::exists(Paths[2]));
+  EXPECT_TRUE(fs::exists(Paths[3]));
+
+  ASSERT_NO_ERROR(fs::remove_directories(BaseDir));
+  ASSERT_FALSE(fs::exists(BaseDir));
+}
+
 const char archive[] = "!<arch>\x0A";
 const char bitcode[] = "\xde\xc0\x17\x0b";
 const char coff_object[] = "\x00\x00......";
 const char coff_bigobj[] = "\x00\x00\xff\xff\x00\x02......"
     "\xc7\xa1\xba\xd1\xee\xba\xa9\x4b\xaf\x20\xfa\xf6\x6a\xa4\xdc\xb8";
 const char coff_import_library[] = "\x00\x00\xff\xff....";
 const char elf_relocatable[] = { 0x7f, 'E', 'L', 'F', 1, 2, 1, 0, 0,
                                  0,    0,   0,   0,   0, 0, 0, 0, 1 };