Reapply [VFS] Allow multiple RealFileSystem instances with independent CWDs.

[sammccall]

Summary

This reverts commit r351091.
The original mac breakages are addressed by ensuring the root directory
we're working from is fully symlink-resolved before starting.

[sammccall]

Fix silliness in test helper: only canonicalize unique dir names, don't clobber input.

[ilya-biryukov]

LGTM

include/llvm/Support/VirtualFileSystem.h

public:
   /// \param Path A path that is modified to be an absolute path.
   /// \returns success if \a path has been made absolute, otherwise a
   ///          platform-specific error_code.
   std::error_code makeAbsolute(SmallVectorImpl<char> &Path) const;
 };
 
 /// Gets an \p vfs::FileSystem for the 'real' file system, as seen by
 /// the operating system.
+/// The working directory is linked to the process's working directory.
+/// (This is usually thread-hostile).
 IntrusiveRefCntPtr<FileSystem> getRealFileSystem();
 
+/// Create an \p vfs::FileSystem for the 'real' file system, as seen by
+/// the operating system.
+/// It has its own working directory, independent of (but initially equal to)
+/// that of the process.
+std::unique_ptr<FileSystem> createPhysicalFileSystem();
+
 /// A file system that allows overlaying one \p AbstractFileSystem on top
 /// of another.
 ///
 /// Consists of a stack of >=1 \p FileSystem objects, which are treated as being
 /// one merged file system. When there is a directory that exists in more than
 /// one file system, the \p OverlayFileSystem contains a directory containing
 /// the union of their contents.  The attributes (permissions, etc.) of the
 /// top-most (most recently added) directory are used.  When there is a file

lib/Support/VirtualFileSystem.cpp

 std::error_code RealFile::close() {
   std::error_code EC = sys::Process::SafelyCloseFileDescriptor(FD);
   FD = -1;
   return EC;
 }
 
 namespace {
 
-/// The file system according to your operating system.
+/// A file system according to your operating system.
+/// This may be linked to the process's working directory, or maintain its own.
+///
+/// Currently, its own working directory is emulated by storing the path and
+/// sending absolute paths to llvm::sys::fs:: functions.
+/// A more principled approach would be to push this down a level, modelling
+/// the working dir as an llvm::sys::fs::WorkingDir or similar.
+/// This would enable the use of openat()-style functions on some platforms.
 class RealFileSystem : public FileSystem {
 public:
+  explicit RealFileSystem(bool LinkCWDToProcess) {
+    if (!LinkCWDToProcess) {
+      SmallString<128> PWD, RealPWD;
+      if (llvm::sys::fs::current_path(PWD))
+        return; // Awful, but nothing to do here.
+      if (llvm::sys::fs::real_path(PWD, RealPWD))
+        WD = {PWD, PWD};
+      else
+        WD = {PWD, RealPWD};
+    }
+  }
+
   ErrorOr<Status> status(const Twine &Path) override;
   ErrorOr<std::unique_ptr<File>> openFileForRead(const Twine &Path) override;
   directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override;
 
   llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override;
   std::error_code setCurrentWorkingDirectory(const Twine &Path) override;
   std::error_code isLocal(const Twine &Path, bool &Result) override;
   std::error_code getRealPath(const Twine &Path,
                               SmallVectorImpl<char> &Output) const override;
 
 private:
-  mutable std::mutex CWDMutex;
-  mutable std::string CWDCache;
+  // If this FS has its own working dir, use it to make Path absolute.
+  // The returned twine is safe to use as long as both Storage and Path live.
+  Twine adjustPath(const Twine &Path, SmallVectorImpl<char> &Storage) const {
+    if (!WD)
+      return Path;
+    Path.toVector(Storage);
+    sys::fs::make_absolute(WD->Resolved, Storage);
+    return Storage;
+  }
+
+  struct WorkingDirectory {
+    // The current working directory, without symlinks resolved. (echo $PWD).
+    SmallString<128> Specified;
+    // The current working directory, with links resolved. (readlink .).
+    SmallString<128> Resolved;
+  };
+  Optional<WorkingDirectory> WD;
 };
 
 } // namespace
 
 ErrorOr<Status> RealFileSystem::status(const Twine &Path) {
+  SmallString<256> Storage;
   sys::fs::file_status RealStatus;
-  if (std::error_code EC = sys::fs::status(Path, RealStatus))
+  if (std::error_code EC =
+          sys::fs::status(adjustPath(Path, Storage), RealStatus))
     return EC;
   return Status::copyWithNewName(RealStatus, Path.str());
 }
 
 ErrorOr<std::unique_ptr<File>>
 RealFileSystem::openFileForRead(const Twine &Name) {
   int FD;
-  SmallString<256> RealName;
-  if (std::error_code EC =
-          sys::fs::openFileForRead(Name, FD, sys::fs::OF_None, &RealName))
+  SmallString<256> RealName, Storage;
+  if (std::error_code EC = sys::fs::openFileForRead(
+          adjustPath(Name, Storage), FD, sys::fs::OF_None, &RealName))
     return EC;
   return std::unique_ptr<File>(new RealFile(FD, Name.str(), RealName.str()));
 }
 
 llvm::ErrorOr<std::string> RealFileSystem::getCurrentWorkingDirectory() const {
-  std::lock_guard<std::mutex> Lock(CWDMutex);
-  if (!CWDCache.empty())
-    return CWDCache;
-  SmallString<256> Dir;
+  if (WD)
+    return WD->Specified.str();
+
+  SmallString<128> Dir;
   if (std::error_code EC = llvm::sys::fs::current_path(Dir))
     return EC;
-  CWDCache = Dir.str();
-  return CWDCache;
+  return Dir.str();
 }
 
 std::error_code RealFileSystem::setCurrentWorkingDirectory(const Twine &Path) {
-  // FIXME: chdir is thread hostile; on the other hand, creating the same
-  // behavior as chdir is complex: chdir resolves the path once, thus
-  // guaranteeing that all subsequent relative path operations work
-  // on the same path the original chdir resulted in. This makes a
-  // difference for example on network filesystems, where symlinks might be
-  // switched during runtime of the tool. Fixing this depends on having a
-  // file system abstraction that allows openat() style interactions.
-  if (auto EC = llvm::sys::fs::set_current_path(Path))
-    return EC;
+  if (!WD)
+    return llvm::sys::fs::set_current_path(Path);
 
-  // Invalidate cache.
-  std::lock_guard<std::mutex> Lock(CWDMutex);
-  CWDCache.clear();
+  SmallString<128> Absolute, Resolved, Storage;
+  adjustPath(Path, Storage).toVector(Absolute);
+  bool IsDir;
+  if (auto Err = llvm::sys::fs::is_directory(Absolute, IsDir))
+    return Err;
+  if (!IsDir)
+    return std::make_error_code(std::errc::not_a_directory);
+  if (auto Err = llvm::sys::fs::real_path(Absolute, Resolved))
+    return Err;
+  WD = {Absolute, Resolved};
   return std::error_code();
 }
 
 std::error_code RealFileSystem::isLocal(const Twine &Path, bool &Result) {
-  return llvm::sys::fs::is_local(Path, Result);
+  SmallString<256> Storage;
+  return llvm::sys::fs::is_local(adjustPath(Path, Storage), Result);
 }
 
 std::error_code
 RealFileSystem::getRealPath(const Twine &Path,
                             SmallVectorImpl<char> &Output) const {
-  return llvm::sys::fs::real_path(Path, Output);
+  SmallString<256> Storage;
+  return llvm::sys::fs::real_path(adjustPath(Path, Storage), Output);
 }
 
 IntrusiveRefCntPtr<FileSystem> vfs::getRealFileSystem() {
-  static IntrusiveRefCntPtr<FileSystem> FS = new RealFileSystem();
+  static IntrusiveRefCntPtr<FileSystem> FS(new RealFileSystem(true));
   return FS;
 }
 
+std::unique_ptr<FileSystem> vfs::createPhysicalFileSystem() {
+  return llvm::make_unique<RealFileSystem>(false);
+}
+
 namespace {
 
 class RealFSDirIter : public llvm::vfs::detail::DirIterImpl {
   llvm::sys::fs::directory_iterator Iter;
 
 public:
   RealFSDirIter(const Twine &Path, std::error_code &EC) : Iter(Path, EC) {
     if (Iter != llvm::sys::fs::directory_iterator())
std::error_code increment() override {
     return EC;
   }
 };
 
 } // namespace
 
 directory_iterator RealFileSystem::dir_begin(const Twine &Dir,
                                              std::error_code &EC) {
-  return directory_iterator(std::make_shared<RealFSDirIter>(Dir, EC));
+  SmallString<128> Storage;
+  return directory_iterator(
+      std::make_shared<RealFSDirIter>(adjustPath(Dir, Storage), EC));
 }
 
 //===-----------------------------------------------------------------------===/
 // OverlayFileSystem implementation
 //===-----------------------------------------------------------------------===/
 
 OverlayFileSystem::OverlayFileSystem(IntrusiveRefCntPtr<FileSystem> BaseFS) {
   FSList.push_back(std::move(BaseFS));

unittests/Support/VirtualFileSystemTest.cpp

 
 namespace {
 struct ScopedDir {
   SmallString<128> Path;
   ScopedDir(const Twine &Name, bool Unique = false) {
     std::error_code EC;
     if (Unique) {
       EC = llvm::sys::fs::createUniqueDirectory(Name, Path);
+      if (!EC) {
+        // Resolve any symlinks in the new directory.
+        std::string UnresolvedPath = Path.str();
+        EC = llvm::sys::fs::real_path(UnresolvedPath, Path);
+      }
     } else {
       Path = Name.str();
       EC = llvm::sys::fs::create_directory(Twine(Path));
     }
     if (EC)
       Path = "";
-    EXPECT_FALSE(EC);
+    EXPECT_FALSE(EC) << EC.message();
   }
   ~ScopedDir() {
     if (Path != "") {
       EXPECT_FALSE(llvm::sys::fs::remove(Path.str()));
     }
   }
   operator StringRef() { return Path.str(); }
 };
struct ScopedLink {
   }
   ~ScopedLink() {
     if (Path != "") {
       EXPECT_FALSE(llvm::sys::fs::remove(Path.str()));
     }
   }
   operator StringRef() { return Path.str(); }
 };
+
+struct ScopedFile {
+  SmallString<128> Path;
+  ScopedFile(const Twine &Path, StringRef Contents) {
+    Path.toVector(this->Path);
+    std::error_code EC;
+    raw_fd_ostream OS(this->Path, EC);
+    EXPECT_FALSE(EC);
+    OS << Contents;
+    OS.flush();
+    EXPECT_FALSE(OS.error());
+    if (EC || OS.error())
+      this->Path = "";
+  }
+  ~ScopedFile() {
+    if (Path != "")
+      EXPECT_FALSE(llvm::sys::fs::remove(Path.str()));
+  }
+};
 } // end anonymous namespace
 
 TEST(VirtualFileSystemTest, BasicRealFSIteration) {
   ScopedDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
   IntrusiveRefCntPtr<vfs::FileSystem> FS = vfs::getRealFileSystem();
 
   std::error_code EC;
   vfs::directory_iterator I = FS->dir_begin(Twine(TestDirectory), EC);
TEST(VirtualFileSystemTest, BasicRealFSIteration) {
   ASSERT_FALSE(EC);
   ASSERT_NE(vfs::directory_iterator(), I);
   EXPECT_TRUE(I->path().endswith("a") || I->path().endswith("c"));
   I.increment(EC);
   EXPECT_EQ(vfs::directory_iterator(), I);
 }
 
 #ifdef LLVM_ON_UNIX
+TEST(VirtualFileSystemTest, MultipleWorkingDirs) {
+  // Our root contains a/aa, b/bb, c, where c is a link to a/.
+  // Run tests both in root/b/ and root/c/ (to test "normal" and symlink dirs).
+  // Interleave operations to show the working directories are independent.
+  ScopedDir Root("r", true), ADir(Root.Path + "/a"), BDir(Root.Path + "/b");
+  ScopedLink C(ADir.Path, Root.Path + "/c");
+  ScopedFile AA(ADir.Path + "/aa", "aaaa"), BB(BDir.Path + "/bb", "bbbb");
+  std::unique_ptr<vfs::FileSystem> BFS = vfs::createPhysicalFileSystem(),
+                                   CFS = vfs::createPhysicalFileSystem();
+
+  ASSERT_FALSE(BFS->setCurrentWorkingDirectory(BDir.Path));
+  ASSERT_FALSE(CFS->setCurrentWorkingDirectory(C.Path));
+  EXPECT_EQ(BDir.Path, *BFS->getCurrentWorkingDirectory());
+  EXPECT_EQ(C.Path, *CFS->getCurrentWorkingDirectory());
+
+  // openFileForRead(), indirectly.
+  auto BBuf = BFS->getBufferForFile("bb");
+  ASSERT_TRUE(BBuf);
+  EXPECT_EQ("bbbb", (*BBuf)->getBuffer());
+
+  auto ABuf = CFS->getBufferForFile("aa");
+  ASSERT_TRUE(ABuf);
+  EXPECT_EQ("aaaa", (*ABuf)->getBuffer());
+
+  // status()
+  auto BStat = BFS->status("bb");
+  ASSERT_TRUE(BStat);
+  EXPECT_EQ("bb", BStat->getName());
+
+  auto AStat = CFS->status("aa");
+  ASSERT_TRUE(AStat);
+  EXPECT_EQ("aa", AStat->getName()); // unresolved name
+
+  // getRealPath()
+  SmallString<128> BPath;
+  ASSERT_FALSE(BFS->getRealPath("bb", BPath));
+  EXPECT_EQ(BB.Path, BPath);
+
+  SmallString<128> APath;
+  ASSERT_FALSE(CFS->getRealPath("aa", APath));
+  EXPECT_EQ(AA.Path, APath); // Reports resolved name.
+
+  // dir_begin
+  std::error_code EC;
+  auto BIt = BFS->dir_begin(".", EC);
+  ASSERT_FALSE(EC);
+  ASSERT_NE(BIt, vfs::directory_iterator());
+  EXPECT_EQ((BDir.Path + "/./bb").str(), BIt->path());
+  BIt.increment(EC);
+  ASSERT_FALSE(EC);
+  ASSERT_EQ(BIt, vfs::directory_iterator());
+
+  auto CIt = CFS->dir_begin(".", EC);
+  ASSERT_FALSE(EC);
+  ASSERT_NE(CIt, vfs::directory_iterator());
+  EXPECT_EQ((ADir.Path + "/./aa").str(), CIt->path()); // Partly resolved name!
+  CIt.increment(EC); // Because likely to read through this path.
+  ASSERT_FALSE(EC);
+  ASSERT_EQ(CIt, vfs::directory_iterator());
+}
+
 TEST(VirtualFileSystemTest, BrokenSymlinkRealFSIteration) {
   ScopedDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
   IntrusiveRefCntPtr<vfs::FileSystem> FS = vfs::getRealFileSystem();
 
   ScopedLink _a("no_such_file", TestDirectory + "/a");
   ScopedDir _b(TestDirectory + "/b");
   ScopedLink _c("no_such_file", TestDirectory + "/c");