From f1309ffebf718d16aec4fab83380556c660e2825 Mon Sep 17 00:00:00 2001 From: Amara Emerson Date: Mon, 14 Jan 2019 18:32:09 +0000 Subject: [PATCH] Revert "[VFS] Allow multiple RealFileSystem instances with independent CWDs." This reverts commit r351079, r351069 and r351050 as it broken the greendragon bots on macOS. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@351091 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Support/VirtualFileSystem.h | 8 -- lib/Support/VirtualFileSystem.cpp | 103 ++++++-------------- unittests/Support/VirtualFileSystemTest.cpp | 80 --------------- 3 files changed, 30 insertions(+), 161 deletions(-) diff --git a/include/llvm/Support/VirtualFileSystem.h b/include/llvm/Support/VirtualFileSystem.h index b8871a512fe..606a81e494e 100644 --- a/include/llvm/Support/VirtualFileSystem.h +++ b/include/llvm/Support/VirtualFileSystem.h @@ -298,16 +298,8 @@ public: /// 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 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 createPhysicalFileSystem(); - /// A file system that allows overlaying one \p AbstractFileSystem on top /// of another. /// diff --git a/lib/Support/VirtualFileSystem.cpp b/lib/Support/VirtualFileSystem.cpp index f400a7c8312..360f8d0061f 100644 --- a/lib/Support/VirtualFileSystem.cpp +++ b/lib/Support/VirtualFileSystem.cpp @@ -228,28 +228,9 @@ std::error_code RealFile::close() { namespace { -/// 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. +/// The file system according to your operating system. 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(const Twine &Path) override; ErrorOr> openFileForRead(const Twine &Path) override; directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override; @@ -261,32 +242,15 @@ public: SmallVectorImpl &Output) const override; private: - // 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 &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 WD; + mutable std::mutex CWDMutex; + mutable std::string CWDCache; }; } // namespace ErrorOr RealFileSystem::status(const Twine &Path) { - SmallString<256> Storage; sys::fs::file_status RealStatus; - if (std::error_code EC = - sys::fs::status(adjustPath(Path, Storage), RealStatus)) + if (std::error_code EC = sys::fs::status(Path, RealStatus)) return EC; return Status::copyWithNewName(RealStatus, Path.str()); } @@ -294,61 +258,56 @@ ErrorOr RealFileSystem::status(const Twine &Path) { ErrorOr> RealFileSystem::openFileForRead(const Twine &Name) { int FD; - SmallString<256> RealName, Storage; - if (std::error_code EC = sys::fs::openFileForRead( - adjustPath(Name, Storage), FD, sys::fs::OF_None, &RealName)) + SmallString<256> RealName; + if (std::error_code EC = + sys::fs::openFileForRead(Name, FD, sys::fs::OF_None, &RealName)) return EC; return std::unique_ptr(new RealFile(FD, Name.str(), RealName.str())); } llvm::ErrorOr RealFileSystem::getCurrentWorkingDirectory() const { - if (WD) - return WD->Specified.str(); - - SmallString<128> Dir; + std::lock_guard Lock(CWDMutex); + if (!CWDCache.empty()) + return CWDCache; + SmallString<256> Dir; if (std::error_code EC = llvm::sys::fs::current_path(Dir)) return EC; - return Dir.str(); + CWDCache = Dir.str(); + return CWDCache; } std::error_code RealFileSystem::setCurrentWorkingDirectory(const Twine &Path) { - if (!WD) - return llvm::sys::fs::set_current_path(Path); - - 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}; + // 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; + + // Invalidate cache. + std::lock_guard Lock(CWDMutex); + CWDCache.clear(); return std::error_code(); } std::error_code RealFileSystem::isLocal(const Twine &Path, bool &Result) { - SmallString<256> Storage; - return llvm::sys::fs::is_local(adjustPath(Path, Storage), Result); + return llvm::sys::fs::is_local(Path, Result); } std::error_code RealFileSystem::getRealPath(const Twine &Path, SmallVectorImpl &Output) const { - SmallString<256> Storage; - return llvm::sys::fs::real_path(adjustPath(Path, Storage), Output); + return llvm::sys::fs::real_path(Path, Output); } IntrusiveRefCntPtr vfs::getRealFileSystem() { - static IntrusiveRefCntPtr FS(new RealFileSystem(true)); + static IntrusiveRefCntPtr FS = new RealFileSystem(); return FS; } -std::unique_ptr vfs::createPhysicalFileSystem() { - return llvm::make_unique(false); -} - namespace { class RealFSDirIter : public llvm::vfs::detail::DirIterImpl { @@ -374,9 +333,7 @@ public: directory_iterator RealFileSystem::dir_begin(const Twine &Dir, std::error_code &EC) { - SmallString<128> Storage; - return directory_iterator( - std::make_shared(adjustPath(Dir, Storage), EC)); + return directory_iterator(std::make_shared(Dir, EC)); } //===-----------------------------------------------------------------------===/ diff --git a/unittests/Support/VirtualFileSystemTest.cpp b/unittests/Support/VirtualFileSystemTest.cpp index b072712c638..7b42943f0fd 100644 --- a/unittests/Support/VirtualFileSystemTest.cpp +++ b/unittests/Support/VirtualFileSystemTest.cpp @@ -382,25 +382,6 @@ struct ScopedLink { } 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) { @@ -431,67 +412,6 @@ TEST(VirtualFileSystemTest, BasicRealFSIteration) { } #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 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 FS = vfs::getRealFileSystem(); -- 2.50.1