Skip to content

Commit

Permalink
Reapply [VFS] Allow multiple RealFileSystem instances with independen…
Browse files Browse the repository at this point in the history
…t CWDs.

This reverts commit r351091.
The original mac breakages are addressed by ensuring the root directory
we're working from is fully symlink-resolved before starting.

Differential Revision: https://reviews.llvm.org/D58169

llvm-svn: 354026
  • Loading branch information
sam-mccall committed Feb 14, 2019
1 parent 14c7ecf commit 15e475e
Show file tree
Hide file tree
Showing 3 changed files with 167 additions and 31 deletions.
8 changes: 8 additions & 0 deletions llvm/include/llvm/Support/VirtualFileSystem.h
Expand Up @@ -298,8 +298,16 @@ class FileSystem : public llvm::ThreadSafeRefCountedBase<FileSystem> {

/// 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.
///
Expand Down
103 changes: 73 additions & 30 deletions llvm/lib/Support/VirtualFileSystem.cpp
Expand Up @@ -228,9 +228,28 @@ std::error_code RealFile::close() {

namespace {

/// The file system according to your operating system.
/// 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.
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;
Expand All @@ -242,72 +261,94 @@ class RealFileSystem : public FileSystem {
SmallVectorImpl<char> &Output) const override;

private:
mutable std::mutex CWDMutex;
mutable std::string CWDCache;
// 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;
};

} // namespace

ErrorOr<Status> RealFileSystem::status(const Twine &Path) {
SmallString<256> Storage;
sys::fs::file_status RealStatus;
if (std::error_code EC = sys::fs::status(Path, RealStatus))
if (std::error_code EC =
sys::fs::status(adjustPath(Path, Storage), RealStatus))
return EC;
return Status::copyWithNewName(RealStatus, Path.str());
}

ErrorOr<std::unique_ptr<File>>
RealFileSystem::openFileForRead(const Twine &Name) {
int FD;
SmallString<256> RealName;
if (std::error_code EC =
sys::fs::openFileForRead(Name, FD, sys::fs::OF_None, &RealName))
SmallString<256> RealName, Storage;
if (std::error_code EC = sys::fs::openFileForRead(
adjustPath(Name, Storage), 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 {
std::lock_guard<std::mutex> Lock(CWDMutex);
if (!CWDCache.empty())
return CWDCache;
SmallString<256> Dir;
if (WD)
return WD->Specified.str();

SmallString<128> Dir;
if (std::error_code EC = llvm::sys::fs::current_path(Dir))
return EC;
CWDCache = Dir.str();
return CWDCache;
return Dir.str();
}

std::error_code RealFileSystem::setCurrentWorkingDirectory(const Twine &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;

// Invalidate cache.
std::lock_guard<std::mutex> Lock(CWDMutex);
CWDCache.clear();
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};
return std::error_code();
}

std::error_code RealFileSystem::isLocal(const Twine &Path, bool &Result) {
return llvm::sys::fs::is_local(Path, Result);
SmallString<256> Storage;
return llvm::sys::fs::is_local(adjustPath(Path, Storage), Result);
}

std::error_code
RealFileSystem::getRealPath(const Twine &Path,
SmallVectorImpl<char> &Output) const {
return llvm::sys::fs::real_path(Path, Output);
SmallString<256> Storage;
return llvm::sys::fs::real_path(adjustPath(Path, Storage), Output);
}

IntrusiveRefCntPtr<FileSystem> vfs::getRealFileSystem() {
static IntrusiveRefCntPtr<FileSystem> FS = new RealFileSystem();
static IntrusiveRefCntPtr<FileSystem> FS(new RealFileSystem(true));
return FS;
}

std::unique_ptr<FileSystem> vfs::createPhysicalFileSystem() {
return llvm::make_unique<RealFileSystem>(false);
}

namespace {

class RealFSDirIter : public llvm::vfs::detail::DirIterImpl {
Expand All @@ -333,7 +374,9 @@ class RealFSDirIter : public llvm::vfs::detail::DirIterImpl {

directory_iterator RealFileSystem::dir_begin(const Twine &Dir,
std::error_code &EC) {
return directory_iterator(std::make_shared<RealFSDirIter>(Dir, EC));
SmallString<128> Storage;
return directory_iterator(
std::make_shared<RealFSDirIter>(adjustPath(Dir, Storage), EC));
}

//===-----------------------------------------------------------------------===/
Expand Down
87 changes: 86 additions & 1 deletion llvm/unittests/Support/VirtualFileSystemTest.cpp
Expand Up @@ -349,13 +349,18 @@ struct ScopedDir {
std::error_code EC;
if (Unique) {
EC = llvm::sys::fs::createUniqueDirectory(Name, Path);
if (!EC) {
// Resolve any symlinks in the new directory.
std::string UnresolvedPath = Path.str();
EC = llvm::sys::fs::real_path(UnresolvedPath, Path);
}
} else {
Path = Name.str();
EC = llvm::sys::fs::create_directory(Twine(Path));
}
if (EC)
Path = "";
EXPECT_FALSE(EC);
EXPECT_FALSE(EC) << EC.message();
}
~ScopedDir() {
if (Path != "") {
Expand All @@ -381,6 +386,25 @@ 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) {
Expand Down Expand Up @@ -411,6 +435,67 @@ 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<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();
Expand Down

0 comments on commit 15e475e

Please sign in to comment.