From: Jonas Devlieghere Date: Tue, 15 Oct 2019 18:37:00 +0000 (+0000) Subject: Revert "[VirtualFileSystem] Support virtual working directory in the RedirectingFS" X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=62e7137cb94bf39cdb2760c586fee1b1be7437c1;p=llvm Revert "[VirtualFileSystem] Support virtual working directory in the RedirectingFS" This reverts the original commit and the follow up: Revert "[VirtualFileSystem] Support virtual working directory in the RedirectingFS" Revert "[test] Update YAML mapping in VirtualFileSystemTest" git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@374935 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/Support/VirtualFileSystem.h b/include/llvm/Support/VirtualFileSystem.h index c844d9d194f..627831ea7cb 100644 --- a/include/llvm/Support/VirtualFileSystem.h +++ b/include/llvm/Support/VirtualFileSystem.h @@ -647,19 +647,9 @@ private: friend class VFSFromYamlDirIterImpl; friend class RedirectingFileSystemParser; - bool shouldUseExternalFS() const { - return ExternalFSValidWD && IsFallthrough; - } - /// The root(s) of the virtual file system. std::vector> 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 ExternalFS; @@ -699,7 +689,8 @@ private: true; #endif - RedirectingFileSystem(IntrusiveRefCntPtr ExternalFS); + RedirectingFileSystem(IntrusiveRefCntPtr ExternalFS) + : ExternalFS(std::move(ExternalFS)) {} /// Looks up the path [Start, End) in \p From, possibly /// recursing into the contents of \p From if it is a directory. diff --git a/lib/Support/VirtualFileSystem.cpp b/lib/Support/VirtualFileSystem.cpp index 7c3d04817a3..5aa420cfc72 100644 --- a/lib/Support/VirtualFileSystem.cpp +++ b/lib/Support/VirtualFileSystem.cpp @@ -989,16 +989,6 @@ std::error_code InMemoryFileSystem::isLocal(const Twine &Path, bool &Result) { // RedirectingFileSystem implementation //===-----------------------------------------------------------------------===/ -RedirectingFileSystem::RedirectingFileSystem(IntrusiveRefCntPtr FS) - : ExternalFS(std::move(FS)) { - 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 @@ -1045,27 +1035,12 @@ public: llvm::ErrorOr RedirectingFileSystem::getCurrentWorkingDirectory() const { - return WorkingDirectory; + return ExternalFS->getCurrentWorkingDirectory(); } std::error_code RedirectingFileSystem::setCurrentWorkingDirectory(const Twine &Path) { - // Don't change the working directory if the path doesn't exist. - if (!exists(Path)) - return errc::no_such_file_or_directory; - - // Always change the external FS but ignore its result. - if (ExternalFS) { - auto EC = ExternalFS->setCurrentWorkingDirectory(Path); - ExternalFSValidWD = !static_cast(EC); - } - - SmallString<128> AbsolutePath; - Path.toVector(AbsolutePath); - if (std::error_code EC = makeAbsolute(AbsolutePath)) - return EC; - WorkingDirectory = AbsolutePath.str(); - return {}; + return ExternalFS->setCurrentWorkingDirectory(Path); } std::error_code RedirectingFileSystem::isLocal(const Twine &Path, @@ -1078,7 +1053,7 @@ directory_iterator RedirectingFileSystem::dir_begin(const Twine &Dir, ErrorOr E = lookupPath(Dir); if (!E) { EC = E.getError(); - if (shouldUseExternalFS() && EC == errc::no_such_file_or_directory) + if (IsFallthrough && EC == errc::no_such_file_or_directory) return ExternalFS->dir_begin(Dir, EC); return {}; } @@ -1096,7 +1071,7 @@ directory_iterator RedirectingFileSystem::dir_begin(const Twine &Dir, auto *D = cast(*E); return directory_iterator(std::make_shared( Dir, D->contents_begin(), D->contents_end(), - /*IterateExternalFS=*/shouldUseExternalFS(), *ExternalFS, EC)); + /*IterateExternalFS=*/IsFallthrough, *ExternalFS, EC)); } void RedirectingFileSystem::setExternalContentsPrefixDir(StringRef PrefixDir) { @@ -1597,7 +1572,7 @@ RedirectingFileSystem::create(std::unique_ptr Buffer, RedirectingFileSystemParser P(Stream); std::unique_ptr FS( - new RedirectingFileSystem(ExternalFS)); + new RedirectingFileSystem(std::move(ExternalFS))); if (!YAMLFilePath.empty()) { // Use the YAML path from -ivfsoverlay to compute the dir to be prefixed @@ -1726,7 +1701,7 @@ ErrorOr RedirectingFileSystem::status(const Twine &Path, ErrorOr RedirectingFileSystem::status(const Twine &Path) { ErrorOr Result = lookupPath(Path); if (!Result) { - if (shouldUseExternalFS() && + if (IsFallthrough && Result.getError() == llvm::errc::no_such_file_or_directory) { return ExternalFS->status(Path); } @@ -1764,7 +1739,7 @@ ErrorOr> RedirectingFileSystem::openFileForRead(const Twine &Path) { ErrorOr E = lookupPath(Path); if (!E) { - if (shouldUseExternalFS() & + if (IsFallthrough && E.getError() == llvm::errc::no_such_file_or_directory) { return ExternalFS->openFileForRead(Path); } @@ -1795,7 +1770,7 @@ RedirectingFileSystem::getRealPath(const Twine &Path, SmallVectorImpl &Output) const { ErrorOr Result = lookupPath(Path); if (!Result) { - if (shouldUseExternalFS() && + if (IsFallthrough && Result.getError() == llvm::errc::no_such_file_or_directory) { return ExternalFS->getRealPath(Path, Output); } @@ -1808,8 +1783,8 @@ RedirectingFileSystem::getRealPath(const Twine &Path, } // Even if there is a directory entry, fall back to ExternalFS if allowed, // because directories don't have a single external contents path. - return shouldUseExternalFS() ? ExternalFS->getRealPath(Path, Output) - : llvm::errc::invalid_argument; + return IsFallthrough ? ExternalFS->getRealPath(Path, Output) + : llvm::errc::invalid_argument; } IntrusiveRefCntPtr diff --git a/unittests/Support/VirtualFileSystemTest.cpp b/unittests/Support/VirtualFileSystemTest.cpp index 53b8630e47d..a867bc57e94 100644 --- a/unittests/Support/VirtualFileSystemTest.cpp +++ b/unittests/Support/VirtualFileSystemTest.cpp @@ -41,9 +41,7 @@ struct DummyFile : public vfs::File { class DummyFileSystem : public vfs::FileSystem { int FSID; // used to produce UniqueIDs int FileID; // used to produce UniqueIDs - std::string WorkingDirectory; std::map FilesAndDirs; - typedef std::map::const_iterator const_iterator; static int getNextFSID() { static int Count = 0; @@ -54,7 +52,8 @@ public: DummyFileSystem() : FSID(getNextFSID()), FileID(0) {} ErrorOr status(const Twine &Path) override { - auto I = findEntry(Path); + std::map::iterator I = + FilesAndDirs.find(Path.str()); if (I == FilesAndDirs.end()) return make_error_code(llvm::errc::no_such_file_or_directory); return I->second; @@ -67,16 +66,15 @@ public: return S.getError(); } llvm::ErrorOr getCurrentWorkingDirectory() const override { - return WorkingDirectory; + return std::string(); } std::error_code setCurrentWorkingDirectory(const Twine &Path) override { - WorkingDirectory = Path.str(); return std::error_code(); } // Map any symlink to "/symlink". std::error_code getRealPath(const Twine &Path, SmallVectorImpl &Output) const override { - auto I = findEntry(Path); + auto I = FilesAndDirs.find(Path.str()); if (I == FilesAndDirs.end()) return make_error_code(llvm::errc::no_such_file_or_directory); if (I->second.isSymlink()) { @@ -138,14 +136,6 @@ public: FilesAndDirs[Path] = Status; } - const_iterator findEntry(const Twine &Path) const { - SmallString<128> P; - Path.toVector(P); - std::error_code EC = makeAbsolute(P); - assert(!EC); - return FilesAndDirs.find(P.str()); - } - void addRegularFile(StringRef Path, sys::fs::perms Perms = sys::fs::all_all) { vfs::Status S(Path, UniqueID(FSID, FileID++), std::chrono::system_clock::now(), 0, 0, 1024, @@ -168,12 +158,6 @@ public: } }; -class ErrorDummyFileSystem : public DummyFileSystem { - std::error_code setCurrentWorkingDirectory(const Twine &Path) override { - return llvm::errc::no_such_file_or_directory; - } -}; - /// Replace back-slashes by front-slashes. std::string getPosixPath(std::string S) { SmallString<128> Result; @@ -2010,154 +1994,3 @@ TEST_F(VFSFromYAMLTest, GetRealPath) { EXPECT_EQ(FS->getRealPath("/non_existing", RealPath), errc::no_such_file_or_directory); } - -TEST_F(VFSFromYAMLTest, WorkingDirectory) { - IntrusiveRefCntPtr Lower(new DummyFileSystem()); - Lower->addDirectory("//root/"); - Lower->addDirectory("//root/foo"); - Lower->addRegularFile("//root/foo/a"); - Lower->addRegularFile("//root/foo/b"); - IntrusiveRefCntPtr FS = getFromYAMLString( - "{ 'use-external-names': false,\n" - " 'roots': [\n" - "{\n" - " 'type': 'directory',\n" - " 'name': '//root/bar',\n" - " 'contents': [ {\n" - " 'type': 'file',\n" - " 'name': '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 WorkingDir = FS->getCurrentWorkingDirectory(); - ASSERT_TRUE(WorkingDir); - EXPECT_EQ(*WorkingDir, "//root/bar/"); - - llvm::ErrorOr 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 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 FS = getFromYAMLString( - "{ 'use-external-names': false,\n" - " 'roots': [\n" - "{\n" - " 'type': 'directory',\n" - " 'name': '//root/bar',\n" - " 'contents': [ {\n" - " 'type': 'file',\n" - " 'name': '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 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()); - - Status = FS->status("./c"); - ASSERT_TRUE(Status.getError()); - - EC = FS->setCurrentWorkingDirectory("//root/"); - ASSERT_FALSE(EC); - - Status = FS->status("c"); - ASSERT_FALSE(Status.getError()); - EXPECT_TRUE(Status->exists()); -} - -TEST_F(VFSFromYAMLTest, WorkingDirectoryFallthroughInvalid) { - IntrusiveRefCntPtr Lower(new ErrorDummyFileSystem()); - Lower->addDirectory("//root/"); - Lower->addDirectory("//root/foo"); - Lower->addRegularFile("//root/foo/a"); - Lower->addRegularFile("//root/foo/b"); - Lower->addRegularFile("//root/c"); - IntrusiveRefCntPtr FS = getFromYAMLString( - "{ 'use-external-names': false,\n" - " 'roots': [\n" - "{\n" - " 'type': 'directory',\n" - " 'name': '//root/bar',\n" - " 'contents': [ {\n" - " 'type': 'file',\n" - " 'name': '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 Status = FS->status("bar/a"); - ASSERT_FALSE(Status.getError()); - EXPECT_TRUE(Status->exists()); - - Status = FS->status("foo/a"); - ASSERT_TRUE(Status.getError()); -}