[llvm] r351091 - Revert "[VFS] Allow multiple RealFileSystem instances with independent CWDs."
Amara Emerson via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 14 10:32:09 PST 2019
Author: aemerson
Date: Mon Jan 14 10:32:09 2019
New Revision: 351091
URL: http://llvm.org/viewvc/llvm-project?rev=351091&view=rev
Log:
Revert "[VFS] Allow multiple RealFileSystem instances with independent CWDs."
This reverts commit r351079, r351069 and r351050 as it broken the greendragon bots on macOS.
Modified:
llvm/trunk/include/llvm/Support/VirtualFileSystem.h
llvm/trunk/lib/Support/VirtualFileSystem.cpp
llvm/trunk/unittests/Support/VirtualFileSystemTest.cpp
Modified: llvm/trunk/include/llvm/Support/VirtualFileSystem.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/VirtualFileSystem.h?rev=351091&r1=351090&r2=351091&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/VirtualFileSystem.h (original)
+++ llvm/trunk/include/llvm/Support/VirtualFileSystem.h Mon Jan 14 10:32:09 2019
@@ -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<FileSystem> 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<FileSystem> createPhysicalFileSystem();
-
/// A file system that allows overlaying one \p AbstractFileSystem on top
/// of another.
///
Modified: llvm/trunk/lib/Support/VirtualFileSystem.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/VirtualFileSystem.cpp?rev=351091&r1=351090&r2=351091&view=diff
==============================================================================
--- llvm/trunk/lib/Support/VirtualFileSystem.cpp (original)
+++ llvm/trunk/lib/Support/VirtualFileSystem.cpp Mon Jan 14 10:32:09 2019
@@ -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> status(const Twine &Path) override;
ErrorOr<std::unique_ptr<File>> openFileForRead(const Twine &Path) override;
directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override;
@@ -261,32 +242,15 @@ public:
SmallVectorImpl<char> &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<char> &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<WorkingDirectory> WD;
+ mutable std::mutex CWDMutex;
+ mutable std::string CWDCache;
};
} // namespace
ErrorOr<Status> 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<Status> RealFileSystem::status(c
ErrorOr<std::unique_ptr<File>>
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<File>(new RealFile(FD, Name.str(), RealName.str()));
}
llvm::ErrorOr<std::string> RealFileSystem::getCurrentWorkingDirectory() const {
- if (WD)
- return WD->Specified.str();
-
- SmallString<128> Dir;
+ std::lock_guard<std::mutex> 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);
+ // 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;
- 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};
+ // Invalidate cache.
+ std::lock_guard<std::mutex> 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<char> &Output) const {
- SmallString<256> Storage;
- return llvm::sys::fs::real_path(adjustPath(Path, Storage), Output);
+ return llvm::sys::fs::real_path(Path, Output);
}
IntrusiveRefCntPtr<FileSystem> vfs::getRealFileSystem() {
- static IntrusiveRefCntPtr<FileSystem> FS(new RealFileSystem(true));
+ static IntrusiveRefCntPtr<FileSystem> FS = new RealFileSystem();
return FS;
}
-std::unique_ptr<FileSystem> vfs::createPhysicalFileSystem() {
- return llvm::make_unique<RealFileSystem>(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<RealFSDirIter>(adjustPath(Dir, Storage), EC));
+ return directory_iterator(std::make_shared<RealFSDirIter>(Dir, EC));
}
//===-----------------------------------------------------------------------===/
Modified: llvm/trunk/unittests/Support/VirtualFileSystemTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/VirtualFileSystemTest.cpp?rev=351091&r1=351090&r2=351091&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/VirtualFileSystemTest.cpp (original)
+++ llvm/trunk/unittests/Support/VirtualFileSystemTest.cpp Mon Jan 14 10:32:09 2019
@@ -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, BasicRealFSI
}
#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<vfs::FileSystem> 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<vfs::FileSystem> FS = vfs::getRealFileSystem();
More information about the llvm-commits
mailing list