JDevlieghere updated this revision to Diff 219398.
JDevlieghere added a comment.

Implement Sam's suggestion for changing the external file system's working 
directory.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65677/new/

https://reviews.llvm.org/D65677

Files:
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===================================================================
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1994,3 +1994,120 @@
   EXPECT_EQ(FS->getRealPath("/non_existing", RealPath),
             errc::no_such_file_or_directory);
 }
+
+TEST_F(VFSFromYAMLTest, WorkingDirectory) {
+  IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());
+  Lower->addDirectory("//root/");
+  Lower->addDirectory("//root/foo");
+  Lower->addRegularFile("//root/foo/a");
+  Lower->addRegularFile("//root/foo/b");
+  IntrusiveRefCntPtr<vfs::FileSystem> FS = getFromYAMLString(
+      "{ 'use-external-names': false,\n"
+      "  'roots': [\n"
+      "{\n"
+      "  'type': 'directory',\n"
+      "  'name': '//root/',\n"
+      "  'contents': [ {\n"
+      "                  'type': 'file',\n"
+      "                  'name': 'bar/a',\n"
+      "                  'external-contents': '//root/foo/a'\n"
+      "                }\n"
+      "              ]\n"
+      "}\n"
+      "]\n"
+      "}",
+      Lower);
+  ASSERT_TRUE(FS.get() != nullptr);
+  std::error_code EC = FS->setCurrentWorkingDirectory("//root/bar/");
+  ASSERT_FALSE(EC);
+
+  llvm::ErrorOr<std::string> WorkingDir = FS->getCurrentWorkingDirectory();
+  ASSERT_TRUE(WorkingDir);
+  EXPECT_EQ(*WorkingDir, "//root/bar/");
+
+  llvm::ErrorOr<vfs::Status> Status = FS->status("./a");
+  ASSERT_FALSE(Status.getError());
+  EXPECT_TRUE(Status->isStatusKnown());
+  EXPECT_FALSE(Status->isDirectory());
+  EXPECT_TRUE(Status->isRegularFile());
+  EXPECT_FALSE(Status->isSymlink());
+  EXPECT_FALSE(Status->isOther());
+  EXPECT_TRUE(Status->exists());
+
+  EC = FS->setCurrentWorkingDirectory("bogus");
+  ASSERT_TRUE(EC);
+  WorkingDir = FS->getCurrentWorkingDirectory();
+  ASSERT_TRUE(WorkingDir);
+  EXPECT_EQ(*WorkingDir, "//root/bar/");
+
+  EC = FS->setCurrentWorkingDirectory("//root/");
+  ASSERT_FALSE(EC);
+  WorkingDir = FS->getCurrentWorkingDirectory();
+  ASSERT_TRUE(WorkingDir);
+  EXPECT_EQ(*WorkingDir, "//root/");
+
+  EC = FS->setCurrentWorkingDirectory("bar/");
+  ASSERT_FALSE(EC);
+  WorkingDir = FS->getCurrentWorkingDirectory();
+  ASSERT_TRUE(WorkingDir);
+  EXPECT_EQ(*WorkingDir, "//root/bar/");
+}
+
+TEST_F(VFSFromYAMLTest, WorkingDirectoryFallthrough) {
+  IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());
+  Lower->addDirectory("//root/");
+  Lower->addDirectory("//root/foo");
+  Lower->addRegularFile("//root/foo/a");
+  Lower->addRegularFile("//root/foo/b");
+  Lower->addRegularFile("//root/c");
+  IntrusiveRefCntPtr<vfs::FileSystem> FS = getFromYAMLString(
+      "{ 'use-external-names': false,\n"
+      "  'roots': [\n"
+      "{\n"
+      "  'type': 'directory',\n"
+      "  'name': '//root/',\n"
+      "  'contents': [ {\n"
+      "                  'type': 'file',\n"
+      "                  'name': 'bar/a',\n"
+      "                  'external-contents': '//root/foo/a'\n"
+      "                }\n"
+      "              ]\n"
+      "}\n"
+      "]\n"
+      "}",
+      Lower);
+  ASSERT_TRUE(FS.get() != nullptr);
+  std::error_code EC = FS->setCurrentWorkingDirectory("//root/");
+  ASSERT_FALSE(EC);
+  ASSERT_TRUE(FS.get() != nullptr);
+
+  llvm::ErrorOr<vfs::Status> Status = FS->status("./bar/a");
+  ASSERT_FALSE(Status.getError());
+  EXPECT_TRUE(Status->exists());
+
+  Status = FS->status("./foo/a");
+  ASSERT_FALSE(Status.getError());
+  EXPECT_TRUE(Status->exists());
+
+  EC = FS->setCurrentWorkingDirectory("//root/bar/");
+  ASSERT_FALSE(EC);
+
+  Status = FS->status("./a");
+  ASSERT_FALSE(Status.getError());
+  EXPECT_TRUE(Status->exists());
+
+  Status = FS->status("./b");
+  ASSERT_TRUE(Status.getError());
+  EXPECT_FALSE(Status->exists());
+
+  Status = FS->status("./c");
+  ASSERT_TRUE(Status.getError());
+  EXPECT_FALSE(Status->exists());
+
+  EC = FS->setCurrentWorkingDirectory("//root/");
+  ASSERT_FALSE(EC);
+
+  Status = FS->status("./c");
+  ASSERT_FALSE(Status.getError());
+  EXPECT_TRUE(Status->exists());
+}
Index: llvm/lib/Support/VirtualFileSystem.cpp
===================================================================
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -989,6 +989,17 @@
 // RedirectingFileSystem implementation
 //===-----------------------------------------------------------------------===/
 
+RedirectingFileSystem::RedirectingFileSystem(
+    IntrusiveRefCntPtr<FileSystem> ExternalFS)
+    : ExternalFS(std::move(ExternalFS)) {
+  if (ExternalFS)
+    if (auto ExternalWorkingDirectory =
+            ExternalFS->getCurrentWorkingDirectory()) {
+      WorkingDirectory = *ExternalWorkingDirectory;
+      ExternalFSValidWD = true;
+    }
+}
+
 // FIXME: reuse implementation common with OverlayFSDirIterImpl as these
 // iterators are conceptually similar.
 class llvm::vfs::VFSFromYamlDirIterImpl
@@ -1035,12 +1046,33 @@
 
 llvm::ErrorOr<std::string>
 RedirectingFileSystem::getCurrentWorkingDirectory() const {
-  return ExternalFS->getCurrentWorkingDirectory();
+  return WorkingDirectory;
 }
 
 std::error_code
 RedirectingFileSystem::setCurrentWorkingDirectory(const Twine &Path) {
-  return ExternalFS->setCurrentWorkingDirectory(Path);
+  // Always change the external FS but ignore its result.
+  if (ExternalFS) {
+    auto EC = ExternalFS->setCurrentWorkingDirectory(Path);
+    ExternalFSValidWD = static_cast<bool>(EC);
+  }
+
+  // Don't change the working directory if the path doesn't exist.
+  if (!exists(Path))
+    return errc::no_such_file_or_directory;
+
+  // Non-absolute paths are relative to the current working directory.
+  if (!sys::path::is_absolute(Path)) {
+    SmallString<128> AbsolutePath;
+    Path.toVector(AbsolutePath);
+    if (std::error_code EC = makeAbsolute(AbsolutePath))
+      return EC;
+    WorkingDirectory = AbsolutePath.str();
+    return {};
+  }
+
+  WorkingDirectory = Path.str();
+  return {};
 }
 
 std::error_code RedirectingFileSystem::isLocal(const Twine &Path,
@@ -1053,7 +1085,7 @@
   ErrorOr<RedirectingFileSystem::Entry *> E = lookupPath(Dir);
   if (!E) {
     EC = E.getError();
-    if (IsFallthrough && EC == errc::no_such_file_or_directory)
+    if (Fallthrough() && EC == errc::no_such_file_or_directory)
       return ExternalFS->dir_begin(Dir, EC);
     return {};
   }
@@ -1071,7 +1103,7 @@
   auto *D = cast<RedirectingFileSystem::RedirectingDirectoryEntry>(*E);
   return directory_iterator(std::make_shared<VFSFromYamlDirIterImpl>(
       Dir, D->contents_begin(), D->contents_end(),
-      /*IterateExternalFS=*/IsFallthrough, *ExternalFS, EC));
+      /*IterateExternalFS=*/Fallthrough(), *ExternalFS, EC));
 }
 
 void RedirectingFileSystem::setExternalContentsPrefixDir(StringRef PrefixDir) {
@@ -1702,7 +1734,7 @@
 ErrorOr<Status> RedirectingFileSystem::status(const Twine &Path) {
   ErrorOr<RedirectingFileSystem::Entry *> Result = lookupPath(Path);
   if (!Result) {
-    if (IsFallthrough &&
+    if (Fallthrough() &&
         Result.getError() == llvm::errc::no_such_file_or_directory) {
       return ExternalFS->status(Path);
     }
@@ -1740,8 +1772,7 @@
 RedirectingFileSystem::openFileForRead(const Twine &Path) {
   ErrorOr<RedirectingFileSystem::Entry *> E = lookupPath(Path);
   if (!E) {
-    if (IsFallthrough &&
-        E.getError() == llvm::errc::no_such_file_or_directory) {
+    if (Fallthrough() & E.getError() == llvm::errc::no_such_file_or_directory) {
       return ExternalFS->openFileForRead(Path);
     }
     return E.getError();
@@ -1771,7 +1802,7 @@
                                    SmallVectorImpl<char> &Output) const {
   ErrorOr<RedirectingFileSystem::Entry *> Result = lookupPath(Path);
   if (!Result) {
-    if (IsFallthrough &&
+    if (Fallthrough() &&
         Result.getError() == llvm::errc::no_such_file_or_directory) {
       return ExternalFS->getRealPath(Path, Output);
     }
@@ -1784,7 +1815,7 @@
   }
   // Even if there is a directory entry, fall back to ExternalFS if allowed,
   // because directories don't have a single external contents path.
-  return IsFallthrough ? ExternalFS->getRealPath(Path, Output)
+  return Fallthrough() ? ExternalFS->getRealPath(Path, Output)
                        : llvm::errc::invalid_argument;
 }
 
Index: llvm/include/llvm/Support/VirtualFileSystem.h
===================================================================
--- llvm/include/llvm/Support/VirtualFileSystem.h
+++ llvm/include/llvm/Support/VirtualFileSystem.h
@@ -647,9 +647,17 @@
   friend class VFSFromYamlDirIterImpl;
   friend class RedirectingFileSystemParser;
 
+  bool Fallthrough() const { return ExternalFSValidWD && IsFallthrough; }
+
   /// The root(s) of the virtual file system.
   std::vector<std::unique_ptr<Entry>> Roots;
 
+  /// The current working directory of the file system.
+  std::string WorkingDirectory;
+
+  /// Whether the current working directory is valid for the external FS.
+  bool ExternalFSValidWD = false;
+
   /// The file system to use for external references.
   IntrusiveRefCntPtr<FileSystem> ExternalFS;
 
@@ -689,8 +697,7 @@
       true;
 #endif
 
-  RedirectingFileSystem(IntrusiveRefCntPtr<FileSystem> ExternalFS)
-      : ExternalFS(std::move(ExternalFS)) {}
+  RedirectingFileSystem(IntrusiveRefCntPtr<FileSystem> ExternalFS);
 
   /// Looks up the path <tt>[Start, End)</tt> in \p From, possibly
   /// recursing into the contents of \p From if it is a directory.
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to