[lldb] Remove FileSystem::Initialize from FileCollector

This patch removes the ability to instantiate the LLDB FileSystem class
with a FileCollector. It keeps the ability to collect files, but uses
the FileCollectorFileSystem to do that transparently.

Because the two are intertwined, this patch also removes the
finalization logic which copied the files over out of process.

lldb/include/lldb/Host/FileSystem.h

@@ -16,7 +16,6 @@
 
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/Chrono.h"
-#include "llvm/Support/FileCollector.h"
 #include "llvm/Support/VirtualFileSystem.h"
 
 #include "lldb/lldb-types.h"
@@ -31,12 +30,9 @@ public:
   static const char *DEV_NULL;
   static const char *PATH_CONVERSION_ERROR;
 
-  FileSystem() : m_fs(llvm::vfs::getRealFileSystem()), m_collector(nullptr) {}
-  FileSystem(std::shared_ptr<llvm::FileCollectorBase> collector)
-      : m_fs(llvm::vfs::getRealFileSystem()),
-        m_collector(std::move(collector)) {}
+  FileSystem() : m_fs(llvm::vfs::getRealFileSystem()) {}
   FileSystem(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs)
-      : m_fs(std::move(fs)), m_collector(nullptr) {}
+      : m_fs(std::move(fs)) {}
 
   FileSystem(const FileSystem &fs) = delete;
   FileSystem &operator=(const FileSystem &fs) = delete;
@@ -44,7 +40,6 @@ public:
   static FileSystem &Instance();
 
   static void Initialize();
-  static void Initialize(std::shared_ptr<llvm::FileCollectorBase> collector);
   static void Initialize(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs);
   static void Terminate();
 
@@ -191,15 +186,11 @@ public:
     return m_fs;
   }
 
-  void Collect(const FileSpec &file_spec);
-  void Collect(const llvm::Twine &file);
-
   void SetHomeDirectory(std::string home_directory);
 
 private:
   static llvm::Optional<FileSystem> &InstanceImpl();
   llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> m_fs;
-  std::shared_ptr<llvm::FileCollectorBase> m_collector;
   std::string m_home_directory;
 };
 } // namespace lldb_private

lldb/include/lldb/Utility/Reproducer.h

@@ -225,9 +225,6 @@ struct ReplayOptions {
   bool check_version = true;
 };
 
-llvm::Error Finalize(Loader *loader);
-llvm::Error Finalize(const FileSpec &root);
-
 } // namespace repro
 } // namespace lldb_private
 

lldb/include/lldb/Utility/ReproducerProvider.h

@@ -91,23 +91,6 @@ public:
   }
 };
 
-class FlushingFileCollector : public llvm::FileCollectorBase {
-public:
-  FlushingFileCollector(llvm::StringRef files_path, llvm::StringRef dirs_path,
-                        std::error_code &ec);
-
-protected:
-  void addFileImpl(llvm::StringRef file) override;
-
-  llvm::vfs::directory_iterator
-  addDirectoryImpl(const llvm::Twine &dir,
-                   llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> vfs,
-                   std::error_code &dir_ec) override;
-
-  llvm::Optional<llvm::raw_fd_ostream> m_files_os;
-  llvm::Optional<llvm::raw_fd_ostream> m_dirs_os;
-};
-
 class FileProvider : public Provider<FileProvider> {
 public:
   struct Info {
@@ -116,25 +99,24 @@ public:
   };
 
   FileProvider(const FileSpec &directory) : Provider(directory) {
-    std::error_code ec;
-    m_collector = std::make_shared<FlushingFileCollector>(
-        directory.CopyByAppendingPathComponent("files.txt").GetPath(),
-        directory.CopyByAppendingPathComponent("dirs.txt").GetPath(), ec);
-    if (ec)
-      m_collector.reset();
+    m_collector = std::make_shared<llvm::FileCollector>(
+        directory.CopyByAppendingPathComponent("root").GetPath(),
+        directory.GetPath());
   }
 
-  std::shared_ptr<llvm::FileCollectorBase> GetFileCollector() {
+  std::shared_ptr<llvm::FileCollector> GetFileCollector() {
     return m_collector;
   }
 
+  void Keep() override;
+
   void RecordInterestingDirectory(const llvm::Twine &dir);
   void RecordInterestingDirectoryRecursive(const llvm::Twine &dir);
 
   static char ID;
 
 private:
-  std::shared_ptr<FlushingFileCollector> m_collector;
+  std::shared_ptr<llvm::FileCollector> m_collector;
 };
 
 /// Provider for the LLDB version number.

lldb/source/API/SBReproducer.cpp

@@ -110,20 +110,7 @@ const char *SBReproducer::Replay(const char *path,
 
 const char *SBReproducer::Finalize(const char *path) {
   LLDB_INSTRUMENT_VA(path)
-  static std::string error;
-
-  repro::Loader *loader = repro::Reproducer::Instance().GetLoader();
-  if (!loader) {
-    error = "unable to get replay loader.";
-    return error.c_str();
-  }
-
-  if (auto e = repro::Finalize(loader)) {
-    error = llvm::toString(std::move(e));
-    return error.c_str();
-  }
-
-  return nullptr;
+  return "Reproducer finalize has been removed";
 }
 
 bool SBReproducer::Generate() {

lldb/source/Commands/CommandObjectReproducer.cpp

@@ -191,10 +191,6 @@ protected:
     auto &r = Reproducer::Instance();
     if (auto generator = r.GetGenerator()) {
       generator->Keep();
-      if (llvm::Error e = repro::Finalize(r.GetReproducerPath())) {
-        SetError(result, std::move(e));
-        return result.Succeeded();
-      }
     } else {
       result.AppendErrorWithFormat("Unable to get the reproducer generator");
       return false;

lldb/source/Host/common/FileSystem.cpp

@@ -49,11 +49,6 @@ void FileSystem::Initialize() {
   InstanceImpl().emplace();
 }
 
-void FileSystem::Initialize(std::shared_ptr<FileCollectorBase> collector) {
-  lldbassert(!InstanceImpl() && "Already initialized.");
-  InstanceImpl().emplace(collector);
-}
-
 void FileSystem::Initialize(IntrusiveRefCntPtr<vfs::FileSystem> fs) {
   lldbassert(!InstanceImpl() && "Already initialized.");
   InstanceImpl().emplace(fs);
@@ -281,8 +276,6 @@ void FileSystem::Resolve(FileSpec &file_spec) {
 std::shared_ptr<DataBufferLLVM>
 FileSystem::CreateDataBuffer(const llvm::Twine &path, uint64_t size,
                              uint64_t offset) {
-  Collect(path);
-
   const bool is_volatile = !IsLocal(path);
   std::unique_ptr<llvm::WritableMemoryBuffer> buffer;
   if (size == 0) {
@@ -430,8 +423,6 @@ static mode_t GetOpenMode(uint32_t permissions) {
 Expected<FileUP> FileSystem::Open(const FileSpec &file_spec,
                                   File::OpenOptions options,
                                   uint32_t permissions, bool should_close_fd) {
-  Collect(file_spec.GetPath());
-
   const int open_flags = GetOpenFlags(options);
   const mode_t open_mode =
       (open_flags & O_CREAT) ? GetOpenMode(permissions) : 0;
@@ -451,20 +442,6 @@ Expected<FileUP> FileSystem::Open(const FileSpec &file_spec,
   return std::move(file);
 }
 
-void FileSystem::Collect(const FileSpec &file_spec) {
-  Collect(file_spec.GetPath());
-}
-
-void FileSystem::Collect(const llvm::Twine &file) {
-  if (!m_collector)
-    return;
-
-  if (llvm::sys::fs::is_directory(file))
-    m_collector->addDirectory(file);
-  else
-    m_collector->addFile(file);
-}
-
 void FileSystem::SetHomeDirectory(std::string home_directory) {
   m_home_directory = std::move(home_directory);
 }

lldb/source/Host/posix/FileSystemPosix.cpp

@@ -72,11 +72,9 @@ Status FileSystem::ResolveSymbolicLink(const FileSpec &src, FileSpec &dst) {
 }
 
 FILE *FileSystem::Fopen(const char *path, const char *mode) {
-  Collect(path);
   return llvm::sys::RetryAfterSignal(nullptr, ::fopen, path, mode);
 }
 
 int FileSystem::Open(const char *path, int flags, int mode) {
-  Collect(path);
   return llvm::sys::RetryAfterSignal(-1, ::open, path, flags, mode);
 }

lldb/source/Initialization/SystemInitializerCommon.cpp

@@ -43,12 +43,15 @@ SystemInitializerCommon::~SystemInitializerCommon() = default;
 /// Initialize the FileSystem based on the current reproducer mode.
 static llvm::Error InitializeFileSystem() {
   auto &r = repro::Reproducer::Instance();
+
   if (repro::Generator *g = r.GetGenerator()) {
     repro::VersionProvider &vp = g->GetOrCreate<repro::VersionProvider>();
     vp.SetVersion(lldb_private::GetVersion());
 
     repro::FileProvider &fp = g->GetOrCreate<repro::FileProvider>();
-    FileSystem::Initialize(fp.GetFileCollector());
+
+    FileSystem::Initialize(llvm::FileCollector::createCollectorVFS(
+        llvm::vfs::getRealFileSystem(), fp.GetFileCollector()));
 
     fp.RecordInterestingDirectory(
         g->GetOrCreate<repro::WorkingDirectoryProvider>().GetDirectory());

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

@@ -2723,7 +2723,6 @@ bool ScriptInterpreterPythonImpl::LoadScriptingModule(
   } else {
     FileSpec module_file(pathname);
     FileSystem::Instance().Resolve(module_file);
-    FileSystem::Instance().Collect(module_file);
 
     fs::file_status st;
     std::error_code ec = status(module_file.GetPath(), st);

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp

@@ -409,7 +409,6 @@ Module *SymbolFileDWARFDebugMap::GetModuleByCompUnitInfo(
       FileSpec oso_file(oso_path);
       ConstString oso_object;
       if (FileSystem::Instance().Exists(oso_file)) {
-        FileSystem::Instance().Collect(oso_file);
         // The modification time returned by the FS can have a higher precision
         // than the one from the CU.
         auto oso_mod_time = std::chrono::time_point_cast<std::chrono::seconds>(

lldb/source/Utility/Reproducer.cpp

@@ -136,7 +136,6 @@ Generator::~Generator() {
   if (!m_done) {
     if (m_auto_generate) {
       Keep();
-      llvm::cantFail(Finalize(GetRoot()));
     } else {
       Discard();
     }
@@ -229,58 +228,3 @@ bool Loader::HasFile(StringRef file) {
   auto it = std::lower_bound(m_files.begin(), m_files.end(), file.str());
   return (it != m_files.end()) && (*it == file);
 }
-
-static llvm::Error addPaths(StringRef path,
-                            function_ref<void(StringRef)> callback) {
-  auto buffer = llvm::MemoryBuffer::getFile(path);
-  if (!buffer)
-    return errorCodeToError(buffer.getError());
-
-  SmallVector<StringRef, 0> paths;
-  (*buffer)->getBuffer().split(paths, '\0');
-  for (StringRef p : paths) {
-    if (!p.empty() && llvm::sys::fs::exists(p))
-      callback(p);
-  }
-
-  return errorCodeToError(llvm::sys::fs::remove(path));
-}
-
-llvm::Error repro::Finalize(Loader *loader) {
-  if (!loader)
-    return make_error<StringError>("invalid loader",
-                                   llvm::inconvertibleErrorCode());
-
-  FileSpec reproducer_root = loader->GetRoot();
-  std::string files_path =
-      reproducer_root.CopyByAppendingPathComponent("files.txt").GetPath();
-  std::string dirs_path =
-      reproducer_root.CopyByAppendingPathComponent("dirs.txt").GetPath();
-
-  FileCollector collector(
-      reproducer_root.CopyByAppendingPathComponent("root").GetPath(),
-      reproducer_root.GetPath());
-
-  if (Error e =
-          addPaths(files_path, [&](StringRef p) { collector.addFile(p); }))
-    return e;
-
-  if (Error e =
-          addPaths(dirs_path, [&](StringRef p) { collector.addDirectory(p); }))
-    return e;
-
-  FileSpec mapping =
-      reproducer_root.CopyByAppendingPathComponent(FileProvider::Info::file);
-  if (auto ec = collector.copyFiles(/*StopOnError=*/false))
-    return errorCodeToError(ec);
-  collector.writeMapping(mapping.GetPath());
-
-  return llvm::Error::success();
-}
-
-llvm::Error repro::Finalize(const FileSpec &root) {
-  Loader loader(root);
-  if (Error e = loader.LoadIndex())
-    return e;
-  return Finalize(&loader);
-}

lldb/source/Utility/ReproducerProvider.cpp

@@ -45,40 +45,6 @@ void VersionProvider::Keep() {
   os << m_version << "\n";
 }
 
-FlushingFileCollector::FlushingFileCollector(llvm::StringRef files_path,
-                                             llvm::StringRef dirs_path,
-                                             std::error_code &ec) {
-  auto clear = llvm::make_scope_exit([this]() {
-    m_files_os.reset();
-    m_dirs_os.reset();
-  });
-  m_files_os.emplace(files_path, ec, llvm::sys::fs::OF_Append);
-  if (ec)
-    return;
-  m_dirs_os.emplace(dirs_path, ec, llvm::sys::fs::OF_Append);
-  if (ec)
-    return;
-  clear.release();
-}
-
-void FlushingFileCollector::addFileImpl(StringRef file) {
-  if (m_files_os) {
-    *m_files_os << file << '\0';
-    m_files_os->flush();
-  }
-}
-
-llvm::vfs::directory_iterator
-FlushingFileCollector::addDirectoryImpl(const Twine &dir,
-                                        IntrusiveRefCntPtr<vfs::FileSystem> vfs,
-                                        std::error_code &dir_ec) {
-  if (m_dirs_os) {
-    *m_dirs_os << dir << '\0';
-    m_dirs_os->flush();
-  }
-  return vfs->dir_begin(dir, dir_ec);
-}
-
 void FileProvider::RecordInterestingDirectory(const llvm::Twine &dir) {
   if (m_collector)
     m_collector->addFile(dir);
@@ -89,6 +55,13 @@ void FileProvider::RecordInterestingDirectoryRecursive(const llvm::Twine &dir) {
     m_collector->addDirectory(dir);
 }
 
+void FileProvider::Keep() {
+  if (m_collector) {
+    FileSpec file = GetRoot().CopyByAppendingPathComponent(Info::file);
+    m_collector->writeMapping(file.GetPath());
+  }
+}
+
 llvm::Expected<std::unique_ptr<ProcessInfoRecorder>>
 ProcessInfoRecorder::Create(const FileSpec &filename) {
   std::error_code ec;