[VFS] Add a WorkingDirectoryFileSystem

[JDevlieghere]

Summary

This patch implements Duncan's suggestion from D94811. It allows us to extract the working directory logic from the virtual file system implementation. More specifically, it enables handling of purely virtual working directories in the RedirectingFileSystem. Currently, setting a working directory that does not exist in the RedirectingFileSystem's "underlying filesystem", disables falthrough to avoid inconsistencies.

[dexonsmith]

This looks like the direction I was suggesting; may be in a follow up patch one of us can change RealFileSystem to be based on this.

What's missing right now is that the APIs taking paths need to be intercepted, so the paths can be converted to absolute before going to the underlying FS. I'd also prefer if we could put all or most of the guts in a base class that isn't templated, both to reduce code duplication and perhaps sink some of it into the .cpp file.

llvm/include/llvm/Support/VirtualFileSystem.h

771

Please name this BaseT to document a little better that it's a base class.

786

I think this should turn Path into an absolute path. I'd suggest something like:

SmallString<128> CanonicalizedPath;
Path.toVector(CanonicalizedPath);
sys::fs::make_absolute(WorkingDirectory, CanonicalizedPath);
sys::path::remove_dots(CanonicalizedPath);
WorkingDirectory = CanonicalizedPath.str();

remove_dots is important to avoid cd ../x followed by cd ../y from endlessly growing the WD into something like /path/to/../x/../y.

Also, see below, maybe these guts don't need to be templated and can go in the .cpp.

796

I think you also need to override every API that takes a (potentially relative) path:

• status
• openFileForRead
• getBufferForFile
• dir_begin
• getRealPath
• isLocal
• makeAbsolute

I'd suggest splitting out WorkingDirectoryFileSystemBase, which would not inherit from anything, as a non-templated place to put a couple of helpers:

class WorkingDirectoryFileSystemBase {
public:
  // Can be public or protected; doesn't matter since WorkingDirectoryFileSystem
  // inherits privately. Definition should probably go in the .cpp.
  Twine getAdjustedPathTwine(const Twine &Path,
                             SmallVectorImpl<char> &Storage) {
    if (sys::path::is_absolute(Path))
      return Path;
    Path.toVector(Storage);
    sys::fs::make_absolute(WorkingDirectory, Storage);
    return Storage;
  }

  ErrorOr<Status> status(const Twine &Path) {
    SmallString<128> Storage;
    return getWorkingDirectoryUnderlyingFS().status(
        getAdjustedPathTwine(Path, Storage));
  }

  // etc.

protected:
  const FileSystem &getWorkingDirectoryUnderlyingFS() const = 0;
  FileSystem &getWorkingDirectoryUnderlyingFS() {
    return const_cast<FileSystem &>(
        const_cast<const WorkingDirectoryFileSystemBase *>(this)->
            getWorkingDirectoryUnderlyingFS());
  }
private:
  std::string WorkingDirectory;
}

and then you're left with:

template <class BaseT>
class WorkingDirectoryFileSystem
    : WorkingDirectoryFileSystemBase, public BaseT {
public:
  std::error_code setCurrentWorkingDirectory(const Twine &Path) override {
    return WorkingDirectoryFileSystemBase::setCurrentWorkingDirectory(Path);
  }

  std::error_code makeAbsolute(SmallVectorImpl<char> &Path) override {
    return WorkingDirectoryFileSystemBase::makeAbsolute(Path);
  }

  ErrorOr<Status> status(const Twine &Path) override {
    return WorkingDirectoryFileSystemBase::status(Path);
  }

  // etc.

private:
  const FileSystem &getWorkingDirectoryUnderlyingFS() const final {
    return static_cast<BaseT *>(this);
  }
};

or there are other ways to layer it, like passing static_cast<BaseT *> down to WorkingDirectoryFileSystemBase as a parameter, or pulling the calls to getAdjustedPathTwine() up to the templated class.

llvm/unittests/Support/VirtualFileSystemTest.cpp

907–910

You can probably skip assigning EC here, make this two one-line statements, unless you're actually going to check what the error is.

962

I think we also to test that the WorkingDirFS will correctly convert relative paths to absolute paths in the other APIs, like status().

[JDevlieghere]

Superseded by D95188

llvm/include/llvm/Support/VirtualFileSystem.h

 #define LLVM_SUPPORT_VIRTUALFILESYSTEM_H
 
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Chrono.h"
+#include "llvm/Support/Errc.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SourceMgr.h"
 #include <cassert>
 #include <cstdint>
 #include <ctime>
 #include <memory>
public:
 
   void dump(raw_ostream &OS) const;
   void dumpEntry(raw_ostream &OS, Entry *E, int NumSpaces = 0) const;
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
   LLVM_DUMP_METHOD void dump() const;
 #endif
 };
 
+template <typename T> class WorkingDirectoryFileSystem : public T {
+public:
+  template <class... ArgsT>
+  WorkingDirectoryFileSystem(ArgsT... Args)
+      : T(Args...), WorkingDirectory(), EC() {
+    llvm::ErrorOr<std::string> WD = T::getCurrentWorkingDirectory();
+    if (WD)
+      WorkingDirectory = *WD;
+    else
+      EC = WD.getError();
+  };
+
+  std::error_code setCurrentWorkingDirectory(const Twine &Path) override {
+    if (!this->exists(Path))
+      return llvm::errc::no_such_file_or_directory;
+    WorkingDirectory = Path.str();
+    EC = {};
+    return {};
+  }
+
+  llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override {
+    if (EC)
+      return EC;
+    return WorkingDirectory;
+  }
+
+private:
+  std::string WorkingDirectory;
+  std::error_code EC;
+};
+
 /// Collect all pairs of <virtual path, real path> entries from the
 /// \p YAMLFilePath. This is used by the module dependency collector to forward
 /// the entries into the reproducer output VFS YAML file.
 void collectVFSFromYAML(
     std::unique_ptr<llvm::MemoryBuffer> Buffer,
     llvm::SourceMgr::DiagHandlerTy DiagHandler, StringRef YAMLFilePath,
     SmallVectorImpl<YAMLVFSEntry> &CollectedEntries,
     void *DiagContext = nullptr,

llvm/lib/Support/VirtualFileSystem.cpp

 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Chrono.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Debug.h"
-#include "llvm/Support/Errc.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/SourceMgr.h"

llvm/unittests/Support/VirtualFileSystemTest.cpp

void addSymlink(StringRef Path) {
     vfs::Status S(Path, UniqueID(FSID, FileID++),
                   std::chrono::system_clock::now(), 0, 0, 0,
                   sys::fs::file_type::symlink_file, sys::fs::all_all);
     addEntry(Path, S);
   }
 };
 
 class ErrorDummyFileSystem : public DummyFileSystem {
+public:
+  ErrorDummyFileSystem(bool InvalidInitialWorkingDirectory = false)
+      : InvalidInitialWorkingDirectory(InvalidInitialWorkingDirectory) {}
   std::error_code setCurrentWorkingDirectory(const Twine &Path) override {
     return llvm::errc::no_such_file_or_directory;
   }
+  llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override {
+    if (InvalidInitialWorkingDirectory)
+      return llvm::errc::no_such_file_or_directory;
+    return "//working/directory";
+  }
+
+private:
+  bool InvalidInitialWorkingDirectory;
 };
 
 /// Replace back-slashes by front-slashes.
 std::string getPosixPath(std::string S) {
   SmallString<128> Result;
   llvm::sys::path::native(S, Result, llvm::sys::path::Style::posix);
   return std::string(Result.str());
 }
TEST(ProxyFileSystemTest, Basic) {
   ASSERT_FALSE(PFS.getRealPath("a", Path));
   ASSERT_EQ("/a", Path);
 
   bool Local = true;
   ASSERT_FALSE(PFS.isLocal("/a", Local));
   EXPECT_FALSE(Local);
 }
 
+TEST(WorkingDirectoryFileSystemTest, Basic) {
+  IntrusiveRefCntPtr<ErrorDummyFileSystem> ErrorFS(new ErrorDummyFileSystem());
+  ErrorFS->addDirectory("//root/");
+  ErrorFS->addDirectory("//root/foo");
+  ErrorFS->addRegularFile("//root/foo/a");
+  ErrorFS->addRegularFile("//root/foo/b");
+
+  std::error_code EC;
+
+  // The ErrorDummyFileSystem rejects every path.
+  EC = ErrorFS->setCurrentWorkingDirectory("//root/");
+  ASSERT_TRUE(EC);
+  EC = ErrorFS->setCurrentWorkingDirectory("//root/foo");
+  ASSERT_TRUE(EC);
+
+  typedef llvm::vfs::WorkingDirectoryFileSystem<ErrorDummyFileSystem>
+      WorkingDirDummyFileSystem;
+  IntrusiveRefCntPtr<WorkingDirDummyFileSystem> WorkingDirFS(
+      new WorkingDirDummyFileSystem());
+  WorkingDirFS->addDirectory("//root/");
+  WorkingDirFS->addDirectory("//root/foo");
+  WorkingDirFS->addRegularFile("//root/foo/a");
+  WorkingDirFS->addRegularFile("//root/foo/b");
+
+  ASSERT_EQ("//working/directory", *WorkingDirFS->getCurrentWorkingDirectory());
+
+  // The WorkingDirectoryFileSystem accepts valid paths.
+  EC = WorkingDirFS->setCurrentWorkingDirectory("//root/");
+  ASSERT_FALSE(EC);
+  ASSERT_EQ("//root/", *WorkingDirFS->getCurrentWorkingDirectory());
+
+  EC = WorkingDirFS->setCurrentWorkingDirectory("//root/foo");
+  ASSERT_FALSE(EC);
+  ASSERT_EQ("//root/foo", *WorkingDirFS->getCurrentWorkingDirectory());
+  ASSERT_TRUE(WorkingDirFS->exists("a"));
+  ASSERT_TRUE(WorkingDirFS->exists("b"));
+
+  // The WorkingDirectoryFileSystem rejects invalid paths.
+  EC = WorkingDirFS->setCurrentWorkingDirectory("//root/bar");
+  ASSERT_TRUE(EC);
+  ASSERT_EQ("//root/foo", *WorkingDirFS->getCurrentWorkingDirectory());
+}
+
+TEST(WorkingDirectoryFileSystemTest, InvalidInitialWorkingDirectory) {
+  typedef llvm::vfs::WorkingDirectoryFileSystem<ErrorDummyFileSystem>
+      WorkingDirDummyFileSystem;
+  IntrusiveRefCntPtr<WorkingDirDummyFileSystem> WorkingDirFS(
+      new WorkingDirDummyFileSystem(/*InvalidInitialWorkingDirectory*/ true));
+  WorkingDirFS->addDirectory("//root/");
+
+  std::error_code EC;
+
+  // If the wrapped FS returned an error code from getCurrentWorkingDirectory,
+  // WorkingDirectoryFileSystem will do the same until the working directory is
+  // changed.
+  llvm::ErrorOr<std::string> WD = WorkingDirFS->getCurrentWorkingDirectory();
+  ASSERT_FALSE(WD);
+  ASSERT_EQ(llvm::errc::no_such_file_or_directory, WD.getError());
+
+  // The error code is cleared after setting the working directory to a valid
+  // path.
+  EC = WorkingDirFS->setCurrentWorkingDirectory("//root/");
+  ASSERT_FALSE(EC);
+  ASSERT_EQ("//root/", *WorkingDirFS->getCurrentWorkingDirectory());
+}
+
 class InMemoryFileSystemTest : public ::testing::Test {
 protected:
   llvm::vfs::InMemoryFileSystem FS;
   llvm::vfs::InMemoryFileSystem NormalizedFS;
 
   InMemoryFileSystemTest()
       : FS(/*UseNormalizedPaths=*/false),
         NormalizedFS(/*UseNormalizedPaths=*/true) {}