[FileCollector] Add a method to add a whole directory and it contents.

[JDevlieghere]

Summary

Extend the FileCollector's API with addDirectory which adds a directory and its contents to the VFS mapping.

[arphaman]

llvm/lib/Support/FileCollector.cpp

78

It looks like the is_regular_file check doesn't take the previously used sys::fs::file_type::directory_file and sys::fs::file_type::symlink_file file types into account. Is that intentional?

[JDevlieghere]

Rebase & fix missing file types.

[arphaman]

LGTM with one comment that needs to be fixed

llvm/lib/Support/FileCollector.cpp

232

Could you also propagate the error to addDirectory and return early from this function if EC is set?

[thakis]

This breaks tests on Windows: http://45.33.8.238/win/11469/step_11.txt

Please take a look and revert for now if investigating takes a while.

[thakis]

Also fails here:
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/31579/steps/test-check-all/logs/FAIL%3A%20LLVM-Unit%3A%3AFileCollectorTest.addDirectory
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/22835/steps/test-check-all/logs/FAIL%3A%20LLVM-Unit%3A%3AFileCollectorTest.addDirectory
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-win-fast/builds/13422/steps/test-check-llvm-unit/logs/FAIL%3A%20LLVM-Unit%3A%3AFileCollectorTest.addDirectory
http://lab.llvm.org:8011/builders/llvm-clang-win-x-armv7l/builds/6232/steps/test-check-llvm/logs/FAIL%3A%20LLVM-Unit%3A%3AFileCollectorTest.addDirectory
http://lab.llvm.org:8011/builders/llvm-clang-win-x-aarch64/builds/6270/steps/test-check-llvm/logs/FAIL%3A%20LLVM-Unit%3A%3AFileCollectorTest.addDirectory
http://lab.llvm.org:8011/builders/lld-x86_64-win7/builds/41289/steps/test-check-all/logs/FAIL%3A%20LLVM-Unit%3A%3AFileCollectorTest.addDirectory

llvm/include/llvm/Support/FileCollector.h

 
 namespace llvm {
 
 /// Collects files into a directory and generates a mapping that can be used by
 /// the VFS.
 class FileCollector {
 public:
   FileCollector(std::string Root, std::string OverlayRoot);
+  virtual ~FileCollector() = default;
 
-  void addFile(const Twine &file);
+  virtual void addFile(const Twine &file);
+  virtual void addDirectory(const Twine &dir);
 
   /// Write the yaml mapping (for the VFS) to the given file.
   std::error_code writeMapping(StringRef mapping_file);
 
   /// Copy the files into the root directory.
   ///
   /// When StopOnError is true (the default) we abort as soon as one file
   /// cannot be copied. This is relatively common, for example when a file was
bool markAsSeen(StringRef Path) {
     if (Path.empty())
       return false;
     return Seen.insert(Path).second;
   }
 
   bool getRealPath(StringRef SrcPath, SmallVectorImpl<char> &Result);
 
   void addFileToMapping(StringRef VirtualPath, StringRef RealPath) {
+    if (sys::fs::is_directory(VirtualPath))
+      VFSWriter.addDirectoryMapping(VirtualPath, RealPath);
+    else
       VFSWriter.addFileMapping(VirtualPath, RealPath);
   }
 
 protected:
   /// Synchronizes adding files.
   std::mutex Mutex;
 
   /// The root directory where files are copied.
   std::string Root;

llvm/lib/Support/FileCollector.cpp

 
 void FileCollector::addFile(const Twine &file) {
   std::lock_guard<std::mutex> lock(Mutex);
   std::string FileStr = file.str();
   if (markAsSeen(FileStr))
     addFileImpl(FileStr);
 }
 
+void FileCollector::addDirectory(const Twine &dir) {
+  assert(sys::fs::is_directory(dir));
+  std::error_code EC;
+  sys::fs::recursive_directory_iterator Iter(dir, EC);
+  sys::fs::recursive_directory_iterator End;
+  addFile(dir); // Add the directory in case it's empty.
+  for (; Iter != End && !EC; Iter.increment(EC)) {
+    if (Iter->type() == sys::fs::file_type::regular_file ||
+        Iter->type() == sys::fs::file_type::directory_file ||
+        Iter->type() == sys::fs::file_type::symlink_file) {
+      addFile(Iter->path());
+    }
+  }
+}
+
 void FileCollector::addFileImpl(StringRef SrcPath) {
   // We need an absolute src path to append to the root.
   SmallString<256> AbsoluteSrc = SrcPath;
   sys::fs::make_absolute(AbsoluteSrc);
 
   // Canonicalize src to a native path to avoid mixed separator styles.
   sys::path::native(AbsoluteSrc);
 
openFileForRead(const Twine &Path) override {
     auto Result = FS->openFileForRead(Path);
     if (Result && *Result)
       Collector->addFile(Path);
     return Result;
   }
 
   llvm::vfs::directory_iterator dir_begin(const llvm::Twine &Dir,
                                           std::error_code &EC) override {
-    auto It = FS->dir_begin(Dir, EC);
-    if (EC)
-      return It;
     // Collect everything that's listed in case the user needs it.
-    Collector->addFile(Dir);
-    for (; !EC && It != llvm::vfs::directory_iterator(); It.increment(EC)) {
-      if (It->type() == sys::fs::file_type::regular_file ||
-          It->type() == sys::fs::file_type::directory_file ||
-          It->type() == sys::fs::file_type::symlink_file) {
-        Collector->addFile(It->path());
-      }
-    }
-    if (EC)
-      return It;
+    Collector->addDirectory(Dir);
+
     // Return a new iterator.
     return FS->dir_begin(Dir, EC);
   }
 
   std::error_code getRealPath(const Twine &Path,
                               SmallVectorImpl<char> &Output) const override {
     auto EC = FS->getRealPath(Path, Output);
     if (!EC) {

llvm/unittests/Support/FileCollectorTest.cpp

TEST(FileCollectorTest, addFile) {
   EXPECT_TRUE(FileCollector.hasSeen("/path/to/a"));
   EXPECT_TRUE(FileCollector.hasSeen("/path/to/b"));
   EXPECT_TRUE(FileCollector.hasSeen("/path/to/c"));
 
   // Make sure we've only seen the added files.
   EXPECT_FALSE(FileCollector.hasSeen("/path/to/d"));
 }
 
+TEST(FileCollectorTest, addDirectory) {
+  ScopedDir file_root("file_root", true);
+  ScopedFile a(file_root + "/aaa");
+  ScopedFile b(file_root + "/bbb");
+  ScopedFile c(file_root + "/ccc");
+
+  std::string root_fs = std::string(file_root.Path.str());
+  TestingFileCollector FileCollector(root_fs, root_fs);
+
+  FileCollector.addDirectory(file_root.Path);
+
+  // Make sure the root is correct.
+  EXPECT_EQ(FileCollector.Root, root_fs);
+
+  // Make sure we've seen all the added files.
+  EXPECT_TRUE(FileCollector.hasSeen(a.Path));
+  EXPECT_TRUE(FileCollector.hasSeen(b.Path));
+  EXPECT_TRUE(FileCollector.hasSeen(c.Path));
+
+  // Make sure we've only seen the added files.
+  EXPECT_FALSE(FileCollector.hasSeen("/file_root/ddd"));
+}
+
 TEST(FileCollectorTest, copyFiles) {
   ScopedDir file_root("file_root", true);
   ScopedFile a(file_root + "/aaa");
   ScopedFile b(file_root + "/bbb");
   ScopedFile c(file_root + "/ccc");
 
   // Create file collector and add files.
   ScopedDir root("copy_files_root", true);