From 91fa1f9fdfa5c7d088d6489a03763e13cfefd8ac Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Wed, 3 Jul 2019 00:45:53 +0000 Subject: [PATCH 1/9] [VFS] Add reverse iterator to OverlayFileSystem Add a reverse iterator to the overlay file system. This makes it possible to take overlays from one OverlayFileSystem, and add them to another. Differential revision: https://reviews.llvm.org/D64113 llvm-svn: 364986 (cherry picked from commit efe21088d76c091a29bc7990ef9cf502e1f1bf62) --- llvm/include/llvm/Support/VirtualFileSystem.h | 13 ++++- .../Support/VirtualFileSystemTest.cpp | 51 +++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h index 0bb574e1b1211..31c9e851daed9 100644 --- a/llvm/include/llvm/Support/VirtualFileSystem.h +++ b/llvm/include/llvm/Support/VirtualFileSystem.h @@ -343,15 +343,24 @@ class OverlayFileSystem : public FileSystem { using iterator = FileSystemList::reverse_iterator; using const_iterator = FileSystemList::const_reverse_iterator; + using reverse_iterator = FileSystemList::iterator; + using const_reverse_iterator = FileSystemList::const_iterator; /// Get an iterator pointing to the most recently added file system. iterator overlays_begin() { return FSList.rbegin(); } const_iterator overlays_begin() const { return FSList.rbegin(); } - /// Get an iterator pointing one-past the least recently added file - /// system. + /// Get an iterator pointing one-past the least recently added file system. iterator overlays_end() { return FSList.rend(); } const_iterator overlays_end() const { return FSList.rend(); } + + /// Get an iterator pointing to the least recently added file system. + reverse_iterator overlays_rbegin() { return FSList.begin(); } + const_reverse_iterator overlays_rbegin() const { return FSList.begin(); } + + /// Get an iterator pointing one-past the most recently added file system. + reverse_iterator overlays_rend() { return FSList.end(); } + const_reverse_iterator overlays_rend() const { return FSList.end(); } }; /// By default, this delegates all calls to the underlying file system. This diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp index 55cb597cc4305..a867bc57e94b0 100644 --- a/llvm/unittests/Support/VirtualFileSystemTest.cpp +++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp @@ -342,6 +342,57 @@ TEST(VirtualFileSystemTest, MergedDirPermissions) { EXPECT_EQ(0200, Status->getPermissions()); } +TEST(VirtualFileSystemTest, OverlayIterator) { + IntrusiveRefCntPtr Lower(new DummyFileSystem()); + Lower->addRegularFile("/foo"); + IntrusiveRefCntPtr Upper(new DummyFileSystem()); + + IntrusiveRefCntPtr O( + new vfs::OverlayFileSystem(Lower)); + O->pushOverlay(Upper); + + ErrorOr Status((std::error_code())); + { + auto it = O->overlays_begin(); + auto end = O->overlays_end(); + + EXPECT_NE(it, end); + + Status = (*it)->status("/foo"); + ASSERT_TRUE(Status.getError()); + + it++; + EXPECT_NE(it, end); + + Status = (*it)->status("/foo"); + ASSERT_FALSE(Status.getError()); + EXPECT_TRUE(Status->exists()); + + it++; + EXPECT_EQ(it, end); + } + + { + auto it = O->overlays_rbegin(); + auto end = O->overlays_rend(); + + EXPECT_NE(it, end); + + Status = (*it)->status("/foo"); + ASSERT_FALSE(Status.getError()); + EXPECT_TRUE(Status->exists()); + + it++; + EXPECT_NE(it, end); + + Status = (*it)->status("/foo"); + ASSERT_TRUE(Status.getError()); + + it++; + EXPECT_EQ(it, end); + } +} + namespace { struct ScopedDir { SmallString<128> Path; From 78ddd86c74969887659236064415928cd807c95a Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Tue, 15 Oct 2019 17:14:24 +0000 Subject: [PATCH 2/9] [VirtualFileSystem] Support virtual working directory in the RedirectingFS Before this patch, changing the working directory of the RedirectingFS would just forward to its external file system. This prevented us from having a working directory that only existed in the VFS mapping. This patch adds support for a virtual working directory in the RedirectingFileSystem. It now keeps track of its own WD in addition to updating the WD of the external file system. This ensures that we can still fall through for relative paths. This change was originally motivated by the reproducer infrastructure in LLDB where we want to deal transparently with relative paths. Differential revision: https://reviews.llvm.org/D65677 llvm-svn: 374917 (cherry picked from commit 0b9981b180ef2f08d2a97cfda2fb6ca35ad5e93c) --- llvm/include/llvm/Support/VirtualFileSystem.h | 13 +- llvm/lib/Support/VirtualFileSystem.cpp | 45 ++++- .../Support/VirtualFileSystemTest.cpp | 175 +++++++++++++++++- 3 files changed, 217 insertions(+), 16 deletions(-) diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h index 31c9e851daed9..32656881210c1 100644 --- a/llvm/include/llvm/Support/VirtualFileSystem.h +++ b/llvm/include/llvm/Support/VirtualFileSystem.h @@ -647,9 +647,19 @@ class RedirectingFileSystem : public vfs::FileSystem { 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; @@ -689,8 +699,7 @@ class RedirectingFileSystem : public vfs::FileSystem { true; #endif - RedirectingFileSystem(IntrusiveRefCntPtr ExternalFS) - : ExternalFS(std::move(ExternalFS)) {} + RedirectingFileSystem(IntrusiveRefCntPtr 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/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp index 48ce31491a80c..67b7fb9f2205d 100644 --- a/llvm/lib/Support/VirtualFileSystem.cpp +++ b/llvm/lib/Support/VirtualFileSystem.cpp @@ -986,6 +986,16 @@ 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 @@ -1032,12 +1042,27 @@ class llvm::vfs::VFSFromYamlDirIterImpl llvm::ErrorOr RedirectingFileSystem::getCurrentWorkingDirectory() const { - return ExternalFS->getCurrentWorkingDirectory(); + return WorkingDirectory; } std::error_code RedirectingFileSystem::setCurrentWorkingDirectory(const Twine &Path) { - return ExternalFS->setCurrentWorkingDirectory(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 {}; } std::error_code RedirectingFileSystem::isLocal(const Twine &Path, @@ -1050,7 +1075,7 @@ directory_iterator RedirectingFileSystem::dir_begin(const Twine &Dir, ErrorOr E = lookupPath(Dir); if (!E) { EC = E.getError(); - if (IsFallthrough && EC == errc::no_such_file_or_directory) + if (shouldUseExternalFS() && EC == errc::no_such_file_or_directory) return ExternalFS->dir_begin(Dir, EC); return {}; } @@ -1068,7 +1093,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=*/IsFallthrough, *ExternalFS, EC)); + /*IterateExternalFS=*/shouldUseExternalFS(), *ExternalFS, EC)); } void RedirectingFileSystem::setExternalContentsPrefixDir(StringRef PrefixDir) { @@ -1570,7 +1595,7 @@ RedirectingFileSystem::create(std::unique_ptr Buffer, RedirectingFileSystemParser P(Stream); std::unique_ptr FS( - new RedirectingFileSystem(std::move(ExternalFS))); + new RedirectingFileSystem(ExternalFS)); if (!YAMLFilePath.empty()) { // Use the YAML path from -ivfsoverlay to compute the dir to be prefixed @@ -1699,7 +1724,7 @@ ErrorOr RedirectingFileSystem::status(const Twine &Path, ErrorOr RedirectingFileSystem::status(const Twine &Path) { ErrorOr Result = lookupPath(Path); if (!Result) { - if (IsFallthrough && + if (shouldUseExternalFS() && Result.getError() == llvm::errc::no_such_file_or_directory) { return ExternalFS->status(Path); } @@ -1737,7 +1762,7 @@ ErrorOr> RedirectingFileSystem::openFileForRead(const Twine &Path) { ErrorOr E = lookupPath(Path); if (!E) { - if (IsFallthrough && + if (shouldUseExternalFS() & E.getError() == llvm::errc::no_such_file_or_directory) { return ExternalFS->openFileForRead(Path); } @@ -1768,7 +1793,7 @@ RedirectingFileSystem::getRealPath(const Twine &Path, SmallVectorImpl &Output) const { ErrorOr Result = lookupPath(Path); if (!Result) { - if (IsFallthrough && + if (shouldUseExternalFS() && Result.getError() == llvm::errc::no_such_file_or_directory) { return ExternalFS->getRealPath(Path, Output); } @@ -1781,8 +1806,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 IsFallthrough ? ExternalFS->getRealPath(Path, Output) - : llvm::errc::invalid_argument; + return shouldUseExternalFS() ? ExternalFS->getRealPath(Path, Output) + : llvm::errc::invalid_argument; } IntrusiveRefCntPtr diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp index a867bc57e94b0..b81cb86f0b673 100644 --- a/llvm/unittests/Support/VirtualFileSystemTest.cpp +++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp @@ -41,7 +41,9 @@ 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; @@ -52,8 +54,7 @@ class DummyFileSystem : public vfs::FileSystem { DummyFileSystem() : FSID(getNextFSID()), FileID(0) {} ErrorOr status(const Twine &Path) override { - std::map::iterator I = - FilesAndDirs.find(Path.str()); + auto I = findEntry(Path); if (I == FilesAndDirs.end()) return make_error_code(llvm::errc::no_such_file_or_directory); return I->second; @@ -66,15 +67,16 @@ class DummyFileSystem : public vfs::FileSystem { return S.getError(); } llvm::ErrorOr getCurrentWorkingDirectory() const override { - return std::string(); + return WorkingDirectory; } 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 = FilesAndDirs.find(Path.str()); + auto I = findEntry(Path); if (I == FilesAndDirs.end()) return make_error_code(llvm::errc::no_such_file_or_directory); if (I->second.isSymlink()) { @@ -136,6 +138,14 @@ class DummyFileSystem : public vfs::FileSystem { 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, @@ -158,6 +168,12 @@ class DummyFileSystem : public vfs::FileSystem { } }; +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; @@ -1994,3 +2010,154 @@ 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/',\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 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/',\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 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/',\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 Status = FS->status("bar/a"); + ASSERT_FALSE(Status.getError()); + EXPECT_TRUE(Status->exists()); + + Status = FS->status("foo/a"); + ASSERT_TRUE(Status.getError()); +} From bcb6fcfc7d8024aa5a7bfabf4c040323cd23b0f7 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Tue, 15 Oct 2019 18:05:44 +0000 Subject: [PATCH 3/9] [test] Update YAML mapping in VirtualFileSystemTest The 'bar' directory should be part of the root rather than the file itself. llvm-svn: 374930 (cherry picked from commit 621ce3790ba254256222addad60d818cb90ac831) --- llvm/unittests/Support/VirtualFileSystemTest.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp index b81cb86f0b673..53b8630e47dee 100644 --- a/llvm/unittests/Support/VirtualFileSystemTest.cpp +++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp @@ -2022,10 +2022,10 @@ TEST_F(VFSFromYAMLTest, WorkingDirectory) { " 'roots': [\n" "{\n" " 'type': 'directory',\n" - " 'name': '//root/',\n" + " 'name': '//root/bar',\n" " 'contents': [ {\n" " 'type': 'file',\n" - " 'name': 'bar/a',\n" + " 'name': 'a',\n" " 'external-contents': '//root/foo/a'\n" " }\n" " ]\n" @@ -2081,10 +2081,10 @@ TEST_F(VFSFromYAMLTest, WorkingDirectoryFallthrough) { " 'roots': [\n" "{\n" " 'type': 'directory',\n" - " 'name': '//root/',\n" + " 'name': '//root/bar',\n" " 'contents': [ {\n" " 'type': 'file',\n" - " 'name': 'bar/a',\n" + " 'name': 'a',\n" " 'external-contents': '//root/foo/a'\n" " }\n" " ]\n" @@ -2138,10 +2138,10 @@ TEST_F(VFSFromYAMLTest, WorkingDirectoryFallthroughInvalid) { " 'roots': [\n" "{\n" " 'type': 'directory',\n" - " 'name': '//root/',\n" + " 'name': '//root/bar',\n" " 'contents': [ {\n" " 'type': 'file',\n" - " 'name': 'bar/a',\n" + " 'name': 'a',\n" " 'external-contents': '//root/foo/a'\n" " }\n" " ]\n" From 6871462072ceb5708dc8c293039a1c6e1ec09931 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Tue, 15 Oct 2019 18:37:00 +0000 Subject: [PATCH 4/9] 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" llvm-svn: 374935 (cherry picked from commit 409b4b5fb39efc775762f3391062d2258c073add) --- llvm/include/llvm/Support/VirtualFileSystem.h | 13 +- llvm/lib/Support/VirtualFileSystem.cpp | 45 +---- .../Support/VirtualFileSystemTest.cpp | 175 +----------------- 3 files changed, 16 insertions(+), 217 deletions(-) diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h index 32656881210c1..31c9e851daed9 100644 --- a/llvm/include/llvm/Support/VirtualFileSystem.h +++ b/llvm/include/llvm/Support/VirtualFileSystem.h @@ -647,19 +647,9 @@ class RedirectingFileSystem : public vfs::FileSystem { 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 @@ class RedirectingFileSystem : public vfs::FileSystem { 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/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp index 67b7fb9f2205d..48ce31491a80c 100644 --- a/llvm/lib/Support/VirtualFileSystem.cpp +++ b/llvm/lib/Support/VirtualFileSystem.cpp @@ -986,16 +986,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 @@ -1042,27 +1032,12 @@ class llvm::vfs::VFSFromYamlDirIterImpl 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, @@ -1075,7 +1050,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 {}; } @@ -1093,7 +1068,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) { @@ -1595,7 +1570,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 @@ -1724,7 +1699,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); } @@ -1762,7 +1737,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); } @@ -1793,7 +1768,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); } @@ -1806,8 +1781,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/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp index 53b8630e47dee..a867bc57e94b0 100644 --- a/llvm/unittests/Support/VirtualFileSystemTest.cpp +++ b/llvm/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 @@ class DummyFileSystem : public vfs::FileSystem { 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 @@ class DummyFileSystem : public vfs::FileSystem { 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 @@ class DummyFileSystem : public vfs::FileSystem { 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 @@ class DummyFileSystem : public vfs::FileSystem { } }; -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()); -} From 2e58d66e0fe36d834bbff51fb0da4985ac7f1452 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Tue, 15 Oct 2019 23:08:57 +0000 Subject: [PATCH 5/9] [Reland][VirtualFileSystem] Support virtual working directory in the RedirectingFS Before this patch, changing the working directory of the RedirectingFS would just forward to its external file system. This prevented us from having a working directory that only existed in the VFS mapping. This patch adds support for a virtual working directory in the RedirectingFileSystem. It now keeps track of its own WD in addition to updating the WD of the external file system. This ensures that we can still fall through for relative paths. This change was originally motivated by the reproducer infrastructure in LLDB where we want to deal transparently with relative paths. Differential revision: https://reviews.llvm.org/D65677 llvm-svn: 374955 (cherry picked from commit 21703543a77dfae0e7a4dc45d4c3c4eed0308953) --- llvm/include/llvm/Support/VirtualFileSystem.h | 13 +- llvm/lib/Support/VirtualFileSystem.cpp | 45 ++++- .../Support/VirtualFileSystemTest.cpp | 175 +++++++++++++++++- 3 files changed, 217 insertions(+), 16 deletions(-) diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h index 31c9e851daed9..32656881210c1 100644 --- a/llvm/include/llvm/Support/VirtualFileSystem.h +++ b/llvm/include/llvm/Support/VirtualFileSystem.h @@ -647,9 +647,19 @@ class RedirectingFileSystem : public vfs::FileSystem { 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; @@ -689,8 +699,7 @@ class RedirectingFileSystem : public vfs::FileSystem { true; #endif - RedirectingFileSystem(IntrusiveRefCntPtr ExternalFS) - : ExternalFS(std::move(ExternalFS)) {} + RedirectingFileSystem(IntrusiveRefCntPtr 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/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp index 48ce31491a80c..67b7fb9f2205d 100644 --- a/llvm/lib/Support/VirtualFileSystem.cpp +++ b/llvm/lib/Support/VirtualFileSystem.cpp @@ -986,6 +986,16 @@ 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 @@ -1032,12 +1042,27 @@ class llvm::vfs::VFSFromYamlDirIterImpl llvm::ErrorOr RedirectingFileSystem::getCurrentWorkingDirectory() const { - return ExternalFS->getCurrentWorkingDirectory(); + return WorkingDirectory; } std::error_code RedirectingFileSystem::setCurrentWorkingDirectory(const Twine &Path) { - return ExternalFS->setCurrentWorkingDirectory(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 {}; } std::error_code RedirectingFileSystem::isLocal(const Twine &Path, @@ -1050,7 +1075,7 @@ directory_iterator RedirectingFileSystem::dir_begin(const Twine &Dir, ErrorOr E = lookupPath(Dir); if (!E) { EC = E.getError(); - if (IsFallthrough && EC == errc::no_such_file_or_directory) + if (shouldUseExternalFS() && EC == errc::no_such_file_or_directory) return ExternalFS->dir_begin(Dir, EC); return {}; } @@ -1068,7 +1093,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=*/IsFallthrough, *ExternalFS, EC)); + /*IterateExternalFS=*/shouldUseExternalFS(), *ExternalFS, EC)); } void RedirectingFileSystem::setExternalContentsPrefixDir(StringRef PrefixDir) { @@ -1570,7 +1595,7 @@ RedirectingFileSystem::create(std::unique_ptr Buffer, RedirectingFileSystemParser P(Stream); std::unique_ptr FS( - new RedirectingFileSystem(std::move(ExternalFS))); + new RedirectingFileSystem(ExternalFS)); if (!YAMLFilePath.empty()) { // Use the YAML path from -ivfsoverlay to compute the dir to be prefixed @@ -1699,7 +1724,7 @@ ErrorOr RedirectingFileSystem::status(const Twine &Path, ErrorOr RedirectingFileSystem::status(const Twine &Path) { ErrorOr Result = lookupPath(Path); if (!Result) { - if (IsFallthrough && + if (shouldUseExternalFS() && Result.getError() == llvm::errc::no_such_file_or_directory) { return ExternalFS->status(Path); } @@ -1737,7 +1762,7 @@ ErrorOr> RedirectingFileSystem::openFileForRead(const Twine &Path) { ErrorOr E = lookupPath(Path); if (!E) { - if (IsFallthrough && + if (shouldUseExternalFS() & E.getError() == llvm::errc::no_such_file_or_directory) { return ExternalFS->openFileForRead(Path); } @@ -1768,7 +1793,7 @@ RedirectingFileSystem::getRealPath(const Twine &Path, SmallVectorImpl &Output) const { ErrorOr Result = lookupPath(Path); if (!Result) { - if (IsFallthrough && + if (shouldUseExternalFS() && Result.getError() == llvm::errc::no_such_file_or_directory) { return ExternalFS->getRealPath(Path, Output); } @@ -1781,8 +1806,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 IsFallthrough ? ExternalFS->getRealPath(Path, Output) - : llvm::errc::invalid_argument; + return shouldUseExternalFS() ? ExternalFS->getRealPath(Path, Output) + : llvm::errc::invalid_argument; } IntrusiveRefCntPtr diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp index a867bc57e94b0..504434c3be028 100644 --- a/llvm/unittests/Support/VirtualFileSystemTest.cpp +++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp @@ -41,7 +41,9 @@ 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; @@ -52,8 +54,7 @@ class DummyFileSystem : public vfs::FileSystem { DummyFileSystem() : FSID(getNextFSID()), FileID(0) {} ErrorOr status(const Twine &Path) override { - std::map::iterator I = - FilesAndDirs.find(Path.str()); + auto I = findEntry(Path); if (I == FilesAndDirs.end()) return make_error_code(llvm::errc::no_such_file_or_directory); return I->second; @@ -66,15 +67,16 @@ class DummyFileSystem : public vfs::FileSystem { return S.getError(); } llvm::ErrorOr getCurrentWorkingDirectory() const override { - return std::string(); + return WorkingDirectory; } 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 = FilesAndDirs.find(Path.str()); + auto I = findEntry(Path); if (I == FilesAndDirs.end()) return make_error_code(llvm::errc::no_such_file_or_directory); if (I->second.isSymlink()) { @@ -136,6 +138,14 @@ class DummyFileSystem : public vfs::FileSystem { 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, @@ -158,6 +168,12 @@ class DummyFileSystem : public vfs::FileSystem { } }; +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; @@ -1994,3 +2010,154 @@ 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()); +} From 886854a47197461686cd5854770c69464e690e5d Mon Sep 17 00:00:00 2001 From: "David L. Jones" Date: Wed, 16 Oct 2019 00:52:00 +0000 Subject: [PATCH 6/9] Fix an unused variable introduced in rL374955 / rG21703543. Even though this is a unit test, it still may be run under optimization. llvm-svn: 374961 (cherry picked from commit a3378063ff6c65a2335a5eca42858b2a968c1094) --- llvm/unittests/Support/VirtualFileSystemTest.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp index 504434c3be028..69e9ce80f79bb 100644 --- a/llvm/unittests/Support/VirtualFileSystemTest.cpp +++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp @@ -143,6 +143,7 @@ class DummyFileSystem : public vfs::FileSystem { Path.toVector(P); std::error_code EC = makeAbsolute(P); assert(!EC); + (void)EC; return FilesAndDirs.find(P.str()); } From 4b06991c0f7c47c8651c6c1b82751099e7007bf9 Mon Sep 17 00:00:00 2001 From: Simon Pilgrim Date: Wed, 16 Oct 2019 11:17:08 +0000 Subject: [PATCH 7/9] RedirectingFileSystem::openFileForRead - replace bitwise & with boolean && to fix warning Seems to be just a typo - now matches other instances which do something similar llvm-svn: 374995 (cherry picked from commit 0caee2762086f6f3bb5657c1d7798df6b4789337) --- llvm/lib/Support/VirtualFileSystem.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp index 67b7fb9f2205d..d517a8f0702f0 100644 --- a/llvm/lib/Support/VirtualFileSystem.cpp +++ b/llvm/lib/Support/VirtualFileSystem.cpp @@ -1762,7 +1762,7 @@ ErrorOr> RedirectingFileSystem::openFileForRead(const Twine &Path) { ErrorOr E = lookupPath(Path); if (!E) { - if (shouldUseExternalFS() & + if (shouldUseExternalFS() && E.getError() == llvm::errc::no_such_file_or_directory) { return ExternalFS->openFileForRead(Path); } From e6d86f30949e87102deaa81471d92c2afe21b25d Mon Sep 17 00:00:00 2001 From: Simon Pilgrim Date: Tue, 1 Oct 2019 11:25:29 +0000 Subject: [PATCH 8/9] VirtualFileSystem - replace dyn_cast<>+assert with cast<> calls. NFCI. Silences a number of clang static analyzer null dereference warnings. llvm-svn: 373325 (cherry picked from commit 7ce312307a25d15d70f7031ac571ab898c664e94) --- llvm/lib/Support/VirtualFileSystem.cpp | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp index d517a8f0702f0..bbb21f789b028 100644 --- a/llvm/lib/Support/VirtualFileSystem.cpp +++ b/llvm/lib/Support/VirtualFileSystem.cpp @@ -1243,7 +1243,7 @@ class llvm::vfs::RedirectingFileSystemParser { } auto *DE = - dyn_cast(ParentEntry); + cast(ParentEntry); DE->addContent(std::move(E)); return DE->getLastContent(); } @@ -1254,9 +1254,7 @@ class llvm::vfs::RedirectingFileSystemParser { StringRef Name = SrcE->getName(); switch (SrcE->getKind()) { case RedirectingFileSystem::EK_Directory: { - auto *DE = - dyn_cast(SrcE); - assert(DE && "Must be a directory"); + auto *DE = cast(SrcE); // Empty directories could be present in the YAML as a way to // describe a file for a current directory after some of its subdir // is parsed. This only leads to redundant walks, ignore it. @@ -1268,11 +1266,10 @@ class llvm::vfs::RedirectingFileSystemParser { break; } case RedirectingFileSystem::EK_File: { - auto *FE = dyn_cast(SrcE); - assert(FE && "Must be a file"); assert(NewParentE && "Parent entry must exist"); - auto *DE = dyn_cast( - NewParentE); + auto *FE = cast(SrcE); + auto *DE = + cast(NewParentE); DE->addContent( llvm::make_unique( Name, FE->getExternalContentsPath(), FE->getUseName())); From ff03dd24470fc7cbad1d7f0ad3b6dadcf1cb0e72 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Tue, 6 Aug 2019 15:46:45 +0000 Subject: [PATCH 9/9] [Path] Fix bug in make_absolute logic This fixes a bug for making path with a //net style root absolute. I discovered the bug while writing a test case for the VFS, which uses these paths because they're both legal absolute paths on Windows and Unix. Differential revision: https://reviews.llvm.org/D65675 llvm-svn: 368053 (cherry picked from commit cb6f2646fd44c5274312c8fa782c4f339dfadad7) --- llvm/lib/Support/Path.cpp | 6 +++--- llvm/unittests/Support/Path.cpp | 17 +++++++++++++---- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/llvm/lib/Support/Path.cpp b/llvm/lib/Support/Path.cpp index c49260125dba6..14def83802daf 100644 --- a/llvm/lib/Support/Path.cpp +++ b/llvm/lib/Support/Path.cpp @@ -855,11 +855,11 @@ void make_absolute(const Twine ¤t_directory, StringRef p(path.data(), path.size()); bool rootDirectory = path::has_root_directory(p); - bool rootName = - (real_style(Style::native) != Style::windows) || path::has_root_name(p); + bool rootName = path::has_root_name(p); // Already absolute. - if (rootName && rootDirectory) + if ((rootName || real_style(Style::native) != Style::windows) && + rootDirectory) return; // All of the following conditions will need the current directory. diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp index f0e11b4e3f62f..6e1ffbabe769c 100644 --- a/llvm/unittests/Support/Path.cpp +++ b/llvm/unittests/Support/Path.cpp @@ -185,10 +185,19 @@ TEST(Support, Path) { path::native(*i, temp_store); } - SmallString<32> Relative("foo.cpp"); - sys::fs::make_absolute("/root", Relative); - Relative[5] = '/'; // Fix up windows paths. - ASSERT_EQ("/root/foo.cpp", Relative); + { + SmallString<32> Relative("foo.cpp"); + sys::fs::make_absolute("/root", Relative); + Relative[5] = '/'; // Fix up windows paths. + ASSERT_EQ("/root/foo.cpp", Relative); + } + + { + SmallString<32> Relative("foo.cpp"); + sys::fs::make_absolute("//root", Relative); + Relative[6] = '/'; // Fix up windows paths. + ASSERT_EQ("//root/foo.cpp", Relative); + } } TEST(Support, FilenameParent) {