diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h index 6547d2cdb629f3..c6ddbf60efdf96 100644 --- a/llvm/include/llvm/Support/VirtualFileSystem.h +++ b/llvm/include/llvm/Support/VirtualFileSystem.h @@ -651,9 +651,12 @@ class RedirectingFileSystem : public vfs::FileSystem { friend class VFSFromYamlDirIterImpl; friend class RedirectingFileSystemParser; - bool shouldUseExternalFS() const { - return ExternalFSValidWD && IsFallthrough; - } + bool shouldUseExternalFS() const { return IsFallthrough; } + + /// Canonicalize path by removing ".", "..", "./", components. This is + /// a VFS request, do not bother about symlinks in the path components + /// but canonicalize in order to perform the correct entry search. + std::error_code makeCanonical(SmallVectorImpl &Path) const; // In a RedirectingFileSystem, keys can be specified in Posix or Windows // style (or even a mixture of both), so this comparison helper allows @@ -672,9 +675,6 @@ class RedirectingFileSystem : public vfs::FileSystem { /// 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; @@ -722,7 +722,7 @@ class RedirectingFileSystem : public vfs::FileSystem { public: /// Looks up \p Path in \c Roots. - ErrorOr lookupPath(const Twine &Path) const; + ErrorOr lookupPath(StringRef Path) const; /// Parses \p Buffer, which is expected to be in YAML format and /// returns a virtual file system representing its contents. diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp index c2ce63964f08f8..05332b00bd1f11 100644 --- a/llvm/lib/Support/VirtualFileSystem.cpp +++ b/llvm/lib/Support/VirtualFileSystem.cpp @@ -1016,7 +1016,6 @@ RedirectingFileSystem::RedirectingFileSystem(IntrusiveRefCntPtr FS) if (auto ExternalWorkingDirectory = ExternalFS->getCurrentWorkingDirectory()) { WorkingDirectory = *ExternalWorkingDirectory; - ExternalFSValidWD = true; } } @@ -1075,12 +1074,6 @@ RedirectingFileSystem::setCurrentWorkingDirectory(const Twine &Path) { 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)) @@ -1089,8 +1082,14 @@ RedirectingFileSystem::setCurrentWorkingDirectory(const Twine &Path) { return {}; } -std::error_code RedirectingFileSystem::isLocal(const Twine &Path, +std::error_code RedirectingFileSystem::isLocal(const Twine &Path_, bool &Result) { + SmallString<256> Path; + Path_.toVector(Path); + + if (std::error_code EC = makeCanonical(Path)) + return {}; + return ExternalFS->isLocal(Path, Result); } @@ -1125,14 +1124,21 @@ std::error_code RedirectingFileSystem::makeAbsolute(SmallVectorImpl &Path) directory_iterator RedirectingFileSystem::dir_begin(const Twine &Dir, std::error_code &EC) { - ErrorOr E = lookupPath(Dir); + SmallString<256> Path; + Dir.toVector(Path); + + EC = makeCanonical(Path); + if (EC) + return {}; + + ErrorOr E = lookupPath(Path); if (!E) { EC = E.getError(); if (shouldUseExternalFS() && EC == errc::no_such_file_or_directory) - return ExternalFS->dir_begin(Dir, EC); + return ExternalFS->dir_begin(Path, EC); return {}; } - ErrorOr S = status(Dir, *E); + ErrorOr S = status(Path, *E); if (!S) { EC = S.getError(); return {}; @@ -1145,7 +1151,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(), + Path, D->contents_begin(), D->contents_end(), /*IterateExternalFS=*/shouldUseExternalFS(), *ExternalFS, EC)); } @@ -1739,22 +1745,22 @@ std::unique_ptr RedirectingFileSystem::create( return FS; } -ErrorOr -RedirectingFileSystem::lookupPath(const Twine &Path_) const { - SmallString<256> Path; - Path_.toVector(Path); - - // Handle relative paths +std::error_code +RedirectingFileSystem::makeCanonical(SmallVectorImpl &Path) const { if (std::error_code EC = makeAbsolute(Path)) return EC; - // Canonicalize path by removing ".", "..", "./", components. This is - // a VFS request, do not bother about symlinks in the path components - // but canonicalize in order to perform the correct entry search. - Path = canonicalize(Path); - if (Path.empty()) + llvm::SmallString<256> CanonicalPath = + canonicalize(StringRef(Path.data(), Path.size())); + if (CanonicalPath.empty()) return make_error_code(llvm::errc::invalid_argument); + Path.assign(CanonicalPath.begin(), CanonicalPath.end()); + return {}; +} + +ErrorOr +RedirectingFileSystem::lookupPath(StringRef Path) const { sys::path::const_iterator Start = sys::path::begin(Path); sys::path::const_iterator End = sys::path::end(Path); for (const auto &Root : Roots) { @@ -1829,7 +1835,13 @@ ErrorOr RedirectingFileSystem::status(const Twine &Path, } } -ErrorOr RedirectingFileSystem::status(const Twine &Path) { +ErrorOr RedirectingFileSystem::status(const Twine &Path_) { + SmallString<256> Path; + Path_.toVector(Path); + + if (std::error_code EC = makeCanonical(Path)) + return EC; + ErrorOr Result = lookupPath(Path); if (!Result) { if (shouldUseExternalFS() && @@ -1867,7 +1879,13 @@ class FileWithFixedStatus : public File { } // namespace ErrorOr> -RedirectingFileSystem::openFileForRead(const Twine &Path) { +RedirectingFileSystem::openFileForRead(const Twine &Path_) { + SmallString<256> Path; + Path_.toVector(Path); + + if (std::error_code EC = makeCanonical(Path)) + return EC; + ErrorOr E = lookupPath(Path); if (!E) { if (shouldUseExternalFS() && @@ -1897,8 +1915,14 @@ RedirectingFileSystem::openFileForRead(const Twine &Path) { } std::error_code -RedirectingFileSystem::getRealPath(const Twine &Path, +RedirectingFileSystem::getRealPath(const Twine &Path_, SmallVectorImpl &Output) const { + SmallString<256> Path; + Path_.toVector(Path); + + if (std::error_code EC = makeCanonical(Path)) + return EC; + ErrorOr Result = lookupPath(Path); if (!Result) { if (shouldUseExternalFS() && diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp index 6aff06b9a72f3e..86b747466b5b5a 100644 --- a/llvm/unittests/Support/VirtualFileSystemTest.cpp +++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp @@ -2135,7 +2135,41 @@ TEST_F(VFSFromYAMLTest, WorkingDirectoryFallthroughInvalid) { EXPECT_TRUE(Status->exists()); Status = FS->status("foo/a"); - ASSERT_TRUE(Status.getError()); + ASSERT_FALSE(Status.getError()); + EXPECT_TRUE(Status->exists()); +} + +TEST_F(VFSFromYAMLTest, VirtualWorkingDirectory) { + 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/bar"); + ASSERT_FALSE(EC); + ASSERT_TRUE(FS.get() != nullptr); + + llvm::ErrorOr Status = FS->status("a"); + ASSERT_FALSE(Status.getError()); + EXPECT_TRUE(Status->exists()); } TEST_F(VFSFromYAMLTest, YAMLVFSWriterTest) {