[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

242–243

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

 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/VirtualFileSystem.h"
 
 #include <mutex>
 
 namespace llvm {
-
+class FileCollectorFileSystem;
 /// 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);
+  std::error_code writeMapping(StringRef MappingFile);
 
   /// 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
   /// removed after it was added to the mapping.
   std::error_code copyFiles(bool StopOnError = true);
 
   /// Create a VFS that collects all the paths that might be looked at by the
   /// file system accesses.
   static IntrusiveRefCntPtr<vfs::FileSystem>
   createCollectorVFS(IntrusiveRefCntPtr<vfs::FileSystem> BaseFS,
                      std::shared_ptr<FileCollector> Collector);
 
 private:
-  void addFileImpl(StringRef SrcPath);
+  friend FileCollectorFileSystem;
 
   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:
+  void addFileImpl(StringRef SrcPath);
+
+  llvm::vfs::directory_iterator
+  addDirectoryImpl(const llvm::Twine &Dir,
+                   IntrusiveRefCntPtr<vfs::FileSystem> FS, std::error_code &EC);
+
   /// Synchronizes adding files.
   std::mutex Mutex;
 
   /// The root directory where files are copied.
   std::string Root;
 
   /// The root directory where the VFS overlay lives.
   std::string OverlayRoot;

llvm/lib/Support/FileCollector.cpp

if (DirWithSymlink == SymlinkMap.end()) {
     RealPath = DirWithSymlink->second;
   }
 
   sys::path::append(RealPath, FileName);
   Result.swap(RealPath);
   return true;
 }
 
-void FileCollector::addFile(const Twine &file) {
+void FileCollector::addFile(const Twine &File) {
   std::lock_guard<std::mutex> lock(Mutex);
-  std::string FileStr = file.str();
+  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;
+  addDirectoryImpl(Dir, vfs::getRealFileSystem(), EC);
+}
+
 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);
 
   // Remove redundant leading "./" pieces and consecutive separators.
   AbsoluteSrc = sys::path::remove_leading_dotslash(AbsoluteSrc);
void FileCollector::addFileImpl(StringRef SrcPath) {
   // Always map a canonical src path to its real path into the YAML, by doing
   // this we map different virtual src paths to the same entry in the VFS
   // overlay, which is a way to emulate symlink inside the VFS; this is also
   // needed for correctness, not doing that can lead to module redefinition
   // errors.
   addFileToMapping(VirtualPath, DstPath);
 }
 
+llvm::vfs::directory_iterator
+FileCollector::addDirectoryImpl(const llvm::Twine &Dir,
+                                IntrusiveRefCntPtr<vfs::FileSystem> FS,
+                                std::error_code &EC) {
+  auto It = FS->dir_begin(Dir, EC);
+  if (EC)
+    return It;
+  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) {
+      addFile(It->path());
+    }
+  }
+  if (EC)
+    return It;
+  // Return a new iterator.
+  return FS->dir_begin(Dir, EC);
+}
+
 /// Set the access and modification time for the given file from the given
 /// status object.
 static std::error_code
 copyAccessAndModificationTime(StringRef Filename,
                               const sys::fs::file_status &Stat) {
   int FD;
 
   if (auto EC =
for (auto &entry : VFSWriter.getMappings()) {
     }
 
     // Copy over modification time.
     copyAccessAndModificationTime(entry.RPath, Stat);
   }
   return {};
 }
 
-std::error_code FileCollector::writeMapping(StringRef mapping_file) {
+std::error_code FileCollector::writeMapping(StringRef MappingFile) {
   std::lock_guard<std::mutex> lock(Mutex);
 
   VFSWriter.setOverlayDir(OverlayRoot);
   VFSWriter.setCaseSensitivity(isCaseSensitivePath(OverlayRoot));
   VFSWriter.setUseExternalNames(false);
 
   std::error_code EC;
-  raw_fd_ostream os(mapping_file, EC, sys::fs::OF_Text);
+  raw_fd_ostream os(MappingFile, EC, sys::fs::OF_Text);
   if (EC)
     return EC;
 
   VFSWriter.write(os);
 
   return {};
 }
 
-namespace {
+namespace llvm {
 
 class FileCollectorFileSystem : public vfs::FileSystem {
 public:
   explicit FileCollectorFileSystem(IntrusiveRefCntPtr<vfs::FileSystem> FS,
                                    std::shared_ptr<FileCollector> Collector)
       : FS(std::move(FS)), Collector(std::move(Collector)) {}
 
   llvm::ErrorOr<llvm::vfs::Status> status(const Twine &Path) override {
     auto Result = FS->status(Path);
     if (Result && Result->exists())
       Collector->addFile(Path);
     return Result;
   }
 
   llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
   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;
-    // Return a new iterator.
-    return FS->dir_begin(Dir, EC);
+    return Collector->addDirectoryImpl(Dir, FS, EC);
   }
 
   std::error_code getRealPath(const Twine &Path,
                               SmallVectorImpl<char> &Output) const override {
     auto EC = FS->getRealPath(Path, Output);
     if (!EC) {
       Collector->addFile(Path);
       if (Output.size() > 0)
std::error_code setCurrentWorkingDirectory(const llvm::Twine &Path) override {
     return FS->setCurrentWorkingDirectory(Path);
   }
 
 private:
   IntrusiveRefCntPtr<vfs::FileSystem> FS;
   std::shared_ptr<FileCollector> Collector;
 };
 
-} // end anonymous namespace
+} // namespace llvm
 
 IntrusiveRefCntPtr<vfs::FileSystem>
 FileCollector::createCollectorVFS(IntrusiveRefCntPtr<vfs::FileSystem> BaseFS,
                                   std::shared_ptr<FileCollector> Collector) {
   return new FileCollectorFileSystem(std::move(BaseFS), std::move(Collector));
 }

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);