Skip to content

Commit

Permalink
[VFS] Implement RedirectingFileSystem::getRealPath.
Browse files Browse the repository at this point in the history
It fixes the case when Objective-C framework is added as a subframework
through a symlink. When parent framework infers a module map and fails
to detect a symlink, it would add a subframework as a submodule. And
when we parse module map for the subframework, we would encounter an
error like

> error: umbrella for module 'WithSubframework.Foo' already covers this directory

By implementing `getRealPath` "an egregious but useful hack" in
`ModuleMap::inferFrameworkModule` works as expected.

rdar://problem/45821279

Reviewers: bruno, benlangmuir, erik.pilkington

Reviewed By: bruno

Subscribers: hiraditya, dexonsmith, JDevlieghere, cfe-commits, llvm-commits

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

llvm-svn: 347009
  • Loading branch information
vsapsai committed Nov 16, 2018
1 parent cac749a commit 7610033
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 4 deletions.
33 changes: 29 additions & 4 deletions llvm/lib/Support/VirtualFileSystem.cpp
Expand Up @@ -1164,14 +1164,14 @@ class RedirectingFileSystem : public vfs::FileSystem {
/// Looks up the path <tt>[Start, End)</tt> in \p From, possibly
/// recursing into the contents of \p From if it is a directory.
ErrorOr<Entry *> lookupPath(sys::path::const_iterator Start,
sys::path::const_iterator End, Entry *From);
sys::path::const_iterator End, Entry *From) const;

/// Get the status of a given an \c Entry.
ErrorOr<Status> status(const Twine &Path, Entry *E);

public:
/// Looks up \p Path in \c Roots.
ErrorOr<Entry *> lookupPath(const Twine &Path);
ErrorOr<Entry *> lookupPath(const Twine &Path) const;

/// Parses \p Buffer, which is expected to be in YAML format and
/// returns a virtual file system representing its contents.
Expand All @@ -1183,6 +1183,9 @@ class RedirectingFileSystem : public vfs::FileSystem {
ErrorOr<Status> status(const Twine &Path) override;
ErrorOr<std::unique_ptr<File>> openFileForRead(const Twine &Path) override;

std::error_code getRealPath(const Twine &Path,
SmallVectorImpl<char> &Output) const override;

llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override {
return ExternalFS->getCurrentWorkingDirectory();
}
Expand Down Expand Up @@ -1726,7 +1729,7 @@ RedirectingFileSystem::create(std::unique_ptr<MemoryBuffer> Buffer,
return FS.release();
}

ErrorOr<Entry *> RedirectingFileSystem::lookupPath(const Twine &Path_) {
ErrorOr<Entry *> RedirectingFileSystem::lookupPath(const Twine &Path_) const {
SmallString<256> Path;
Path_.toVector(Path);

Expand Down Expand Up @@ -1757,7 +1760,8 @@ ErrorOr<Entry *> RedirectingFileSystem::lookupPath(const Twine &Path_) {

ErrorOr<Entry *>
RedirectingFileSystem::lookupPath(sys::path::const_iterator Start,
sys::path::const_iterator End, Entry *From) {
sys::path::const_iterator End,
Entry *From) const {
#ifndef _WIN32
assert(!isTraversalComponent(*Start) &&
!isTraversalComponent(From->getName()) &&
Expand Down Expand Up @@ -1890,6 +1894,27 @@ RedirectingFileSystem::openFileForRead(const Twine &Path) {
llvm::make_unique<FileWithFixedStatus>(std::move(*Result), S));
}

std::error_code
RedirectingFileSystem::getRealPath(const Twine &Path,
SmallVectorImpl<char> &Output) const {
ErrorOr<Entry *> Result = lookupPath(Path);
if (!Result) {
if (IsFallthrough &&
Result.getError() == llvm::errc::no_such_file_or_directory) {
return ExternalFS->getRealPath(Path, Output);
}
return Result.getError();
}

if (auto *F = dyn_cast<RedirectingFileEntry>(*Result)) {
return ExternalFS->getRealPath(F->getExternalContentsPath(), Output);
}
// 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;
}

IntrusiveRefCntPtr<FileSystem>
vfs::getVFSFromYAML(std::unique_ptr<MemoryBuffer> Buffer,
SourceMgr::DiagHandlerTy DiagHandler,
Expand Down
46 changes: 46 additions & 0 deletions llvm/unittests/Support/VirtualFileSystemTest.cpp
Expand Up @@ -1775,3 +1775,49 @@ TEST_F(VFSFromYAMLTest, DirectoryIterationErrorInVFSLayer) {
checkContents(FS->dir_begin("//root/foo", EC),
{"//root/foo/a", "//root/foo/b"});
}

TEST_F(VFSFromYAMLTest, GetRealPath) {
IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());
Lower->addDirectory("/dir/");
Lower->addRegularFile("/foo");
Lower->addSymlink("/link");
IntrusiveRefCntPtr<vfs::FileSystem> FS = getFromYAMLString(
"{ 'use-external-names': false,\n"
" 'roots': [\n"
"{\n"
" 'type': 'directory',\n"
" 'name': '/root/',\n"
" 'contents': [ {\n"
" 'type': 'file',\n"
" 'name': 'bar',\n"
" 'external-contents': '/link'\n"
" }\n"
" ]\n"
"},\n"
"{\n"
" 'type': 'directory',\n"
" 'name': '/dir/',\n"
" 'contents': []\n"
"}\n"
"]\n"
"}",
Lower);
ASSERT_TRUE(FS.get() != nullptr);

// Regular file present in underlying file system.
SmallString<16> RealPath;
EXPECT_FALSE(FS->getRealPath("/foo", RealPath));
EXPECT_EQ(RealPath.str(), "/foo");

// File present in YAML pointing to symlink in underlying file system.
EXPECT_FALSE(FS->getRealPath("/root/bar", RealPath));
EXPECT_EQ(RealPath.str(), "/symlink");

// Directories should fall back to the underlying file system is possible.
EXPECT_FALSE(FS->getRealPath("/dir/", RealPath));
EXPECT_EQ(RealPath.str(), "/dir/");

// Try a non-existing file.
EXPECT_EQ(FS->getRealPath("/non_existing", RealPath),
errc::no_such_file_or_directory);
}

0 comments on commit 7610033

Please sign in to comment.