vsapsai created this revision.
vsapsai added reviewers: bruno, benlangmuir.
Herald added subscribers: dexonsmith, hiraditya.
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
https://reviews.llvm.org/D54245
Files:
clang/test/VFS/subframework-symlink.m
llvm/lib/Support/VirtualFileSystem.cpp
llvm/unittests/Support/VirtualFileSystemTest.cpp
Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===================================================================
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1775,3 +1775,49 @@
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);
+}
Index: llvm/lib/Support/VirtualFileSystem.cpp
===================================================================
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -1164,14 +1164,14 @@
/// 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.
@@ -1183,6 +1183,9 @@
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();
}
@@ -1726,7 +1729,7 @@
return FS.release();
}
-ErrorOr<Entry *> RedirectingFileSystem::lookupPath(const Twine &Path_) {
+ErrorOr<Entry *> RedirectingFileSystem::lookupPath(const Twine &Path_) const {
SmallString<256> Path;
Path_.toVector(Path);
@@ -1757,7 +1760,8 @@
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()) &&
@@ -1890,6 +1894,27 @@
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,
Index: clang/test/VFS/subframework-symlink.m
===================================================================
--- /dev/null
+++ clang/test/VFS/subframework-symlink.m
@@ -0,0 +1,23 @@
+// REQUIRES: shell
+
+// Test that when a subframework is a symlink to another framework, we don't
+// add it as a submodule to the enclosing framework. We also need to make clang
+// to infer module for the enclosing framework. For this we don't have
+// a module map for the framework itself but have it in a parent directory.
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo 'framework module * {}' > %t/module.modulemap
+// RUN: mkdir -p %t/WithSubframework.framework/Headers
+// RUN: echo '#include <Foo/Foo.h>' > %t/WithSubframework.framework/Headers/WithSubframework.h
+// RUN: cp -R %S/Inputs/Foo.framework %t
+// RUN: mkdir -p %t/WithSubframework.framework/Frameworks
+// RUN: ln -s %t/Foo.framework %t/WithSubframework.framework/Frameworks
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache1 -F %t -fsyntax-only %s
+
+// Adding VFS overlay shouldn't change this behavior.
+//
+// RUN: sed -e "s:INPUT_DIR:/InvalidPath:g" -e "s:OUT_DIR:/InvalidPath:g" %S/Inputs/vfsoverlay.yaml > %t/overlay.yaml
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache2 -F %t -fsyntax-only %s -ivfsoverlay %t/overlay.yaml
+
+#import <WithSubframework/WithSubframework.h>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
