[clang] a903ecb - [vfs] Allow root paths relative to the vfsoverlay YAML file
Haowei Wu via cfe-commits
cfe-commits at lists.llvm.org
Fri Dec 16 11:45:47 PST 2022
Author: Haowei Wu
Date: 2022-12-16T11:45:36-08:00
New Revision: a903ecb4a26d8cf9a2e9e8369105682fd98f3982
URL: https://github.com/llvm/llvm-project/commit/a903ecb4a26d8cf9a2e9e8369105682fd98f3982
DIFF: https://github.com/llvm/llvm-project/commit/a903ecb4a26d8cf9a2e9e8369105682fd98f3982.diff
LOG: [vfs] Allow root paths relative to the vfsoverlay YAML file
This change adds 'root-relative' option in vfsoverlay YAML file format
so the root patchs can be relative to the YAML file directory instead of
the current working directory.
Differential Revision: https://reviews.llvm.org/D137473
Added:
clang/test/VFS/Inputs/root-relative-overlay.yaml
Modified:
clang/test/VFS/relative-path.c
llvm/include/llvm/Support/VirtualFileSystem.h
llvm/lib/Support/VirtualFileSystem.cpp
llvm/unittests/Support/VirtualFileSystemTest.cpp
Removed:
################################################################################
diff --git a/clang/test/VFS/Inputs/root-relative-overlay.yaml b/clang/test/VFS/Inputs/root-relative-overlay.yaml
new file mode 100644
index 0000000000000..931c1b4b73c55
--- /dev/null
+++ b/clang/test/VFS/Inputs/root-relative-overlay.yaml
@@ -0,0 +1,11 @@
+{
+ 'version': 0,
+ 'case-sensitive': false,
+ 'overlay-relative': OVERLAY_DIR,
+ 'root-relative': 'overlay-dir',
+ 'roots': [
+ { 'name': 'not_real.h', 'type': 'file',
+ 'external-contents': 'EXTERNAL_DIR/actual_header.h'
+ },
+ ]
+}
diff --git a/clang/test/VFS/relative-path.c b/clang/test/VFS/relative-path.c
index ab207d096848b..ba5451136a0c1 100644
--- a/clang/test/VFS/relative-path.c
+++ b/clang/test/VFS/relative-path.c
@@ -3,6 +3,13 @@
// RUN: sed -e "s at INPUT_DIR@%{/S:regex_replacement}/Inputs at g" -e "s at OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/vfsoverlay.yaml > %t.yaml
// RUN: %clang_cc1 -Werror -I . -ivfsoverlay %t.yaml -fsyntax-only %s
+// RUN: cp %S/Inputs/actual_header.h %t/actual_header.h
+// RUN: sed -e "s at OVERLAY_DIR@true at g" -e "s at EXTERNAL_DIR@. at g" %S/Inputs/root-relative-overlay.yaml > %t/root-relative-overlay.yaml
+// RUN: %clang_cc1 -Werror -I %t -ivfsoverlay %t/root-relative-overlay.yaml -fsyntax-only %s
+
+// RUN: sed -e "s at OVERLAY_DIR@false at g" -e "s at EXTERNAL_DIR@%{/t:regex_replacement}@g" %S/Inputs/root-relative-overlay.yaml > %t/root-relative-overlay2.yaml
+// RUN: %clang_cc1 -Werror -I %t -ivfsoverlay %t/root-relative-overlay2.yaml -fsyntax-only %s
+
#include "not_real.h"
void foo(void) {
diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h
index 295ed63def9be..71dbc80edf9a5 100644
--- a/llvm/include/llvm/Support/VirtualFileSystem.h
+++ b/llvm/include/llvm/Support/VirtualFileSystem.h
@@ -653,17 +653,28 @@ class RedirectingFileSystemParser;
/// \endverbatim
///
/// The roots may be absolute or relative. If relative they will be made
-/// absolute against the current working directory.
+/// absolute against either current working directory or the directory where
+/// the Overlay YAML file is located, depending on the 'root-relative'
+/// configuration.
///
/// All configuration options are optional.
/// 'case-sensitive': <boolean, default=(true for Posix, false for Windows)>
/// 'use-external-names': <boolean, default=true>
+/// 'root-relative': <string, one of 'cwd' or 'overlay-dir', default='cwd'>
/// 'overlay-relative': <boolean, default=false>
/// 'fallthrough': <boolean, default=true, deprecated - use 'redirecting-with'
/// instead>
/// 'redirecting-with': <string, one of 'fallthrough', 'fallback', or
/// 'redirect-only', default='fallthrough'>
///
+/// To clarify, 'root-relative' option will prepend the current working
+/// directory, or the overlay directory to the 'roots->name' field only if
+/// 'roots->name' is a relative path. On the other hand, when 'overlay-relative'
+/// is set to 'true', external paths will always be prepended with the overlay
+/// directory, even if external paths are not relative paths. The
+/// 'root-relative' option has no interaction with the 'overlay-relative'
+/// option.
+///
/// Virtual directories that list their contents are represented as
/// \verbatim
/// {
@@ -747,6 +758,15 @@ class RedirectingFileSystem : public vfs::FileSystem {
RedirectOnly
};
+ /// The type of relative path used by Roots.
+ enum class RootRelativeKind {
+ /// The roots are relative to the current working directory.
+ CWD,
+ /// The roots are relative to the directory where the Overlay YAML file
+ // locates.
+ OverlayDir
+ };
+
/// A single file or directory in the VFS.
class Entry {
EntryKind Kind;
@@ -892,6 +912,21 @@ class RedirectingFileSystem : public vfs::FileSystem {
ErrorOr<Status> getExternalStatus(const Twine &CanonicalPath,
const Twine &OriginalPath) const;
+ /// Make \a Path an absolute path.
+ ///
+ /// Makes \a Path absolute using the \a WorkingDir if it is not already.
+ ///
+ /// /absolute/path => /absolute/path
+ /// relative/../path => <WorkingDir>/relative/../path
+ ///
+ /// \param WorkingDir A path that will be used as the base Dir if \a Path
+ /// is not already absolute.
+ /// \param Path A path that is modified to be an absolute path.
+ /// \returns success if \a path has been made absolute, otherwise a
+ /// platform-specific error_code.
+ std::error_code makeAbsolute(StringRef WorkingDir,
+ SmallVectorImpl<char> &Path) const;
+
// In a RedirectingFileSystem, keys can be specified in Posix or Windows
// style (or even a mixture of both), so this comparison helper allows
// slashes (representing a root) to match backslashes (and vice versa). Note
@@ -912,10 +947,11 @@ class RedirectingFileSystem : public vfs::FileSystem {
/// The file system to use for external references.
IntrusiveRefCntPtr<FileSystem> ExternalFS;
- /// If IsRelativeOverlay is set, this represents the directory
- /// path that should be prefixed to each 'external-contents' entry
- /// when reading from YAML files.
- std::string ExternalContentsPrefixDir;
+ /// This represents the directory path that the YAML file is located.
+ /// This will be prefixed to each 'external-contents' if IsRelativeOverlay
+ /// is set. This will also be prefixed to each 'roots->name' if RootRelative
+ /// is set to RootRelativeKind::OverlayDir and the path is relative.
+ std::string OverlayFileDir;
/// @name Configuration
/// @{
@@ -925,7 +961,7 @@ class RedirectingFileSystem : public vfs::FileSystem {
/// Currently, case-insensitive matching only works correctly with ASCII.
bool CaseSensitive = is_style_posix(sys::path::Style::native);
- /// IsRelativeOverlay marks whether a ExternalContentsPrefixDir path must
+ /// IsRelativeOverlay marks whether a OverlayFileDir path must
/// be prefixed in every 'external-contents' when reading from YAML files.
bool IsRelativeOverlay = false;
@@ -936,6 +972,10 @@ class RedirectingFileSystem : public vfs::FileSystem {
/// Determines the lookups to perform, as well as their order. See
/// \c RedirectKind for details.
RedirectKind Redirection = RedirectKind::Fallthrough;
+
+ /// Determine the prefix directory if the roots are relative paths. See
+ /// \c RootRelativeKind for details.
+ RootRelativeKind RootRelative = RootRelativeKind::CWD;
/// @}
RedirectingFileSystem(IntrusiveRefCntPtr<FileSystem> ExternalFS);
@@ -986,9 +1026,9 @@ class RedirectingFileSystem : public vfs::FileSystem {
directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override;
- void setExternalContentsPrefixDir(StringRef PrefixDir);
+ void setOverlayFileDir(StringRef PrefixDir);
- StringRef getExternalContentsPrefixDir() const;
+ StringRef getOverlayFileDir() const;
/// Sets the redirection kind to \c Fallthrough if true or \c RedirectOnly
/// otherwise. Will removed in the future, use \c setRedirection instead.
diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp
index eac6fc9a9e359..1c0d2a044448f 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -1347,32 +1347,51 @@ std::error_code RedirectingFileSystem::makeAbsolute(SmallVectorImpl<char> &Path)
if (llvm::sys::path::is_absolute(Path, llvm::sys::path::Style::posix) ||
llvm::sys::path::is_absolute(Path,
llvm::sys::path::Style::windows_backslash))
+ // This covers windows absolute path with forward slash as well, as the
+ // forward slashes are treated as path seperation in llvm::path
+ // regardless of what path::Style is used.
return {};
auto WorkingDir = getCurrentWorkingDirectory();
if (!WorkingDir)
return WorkingDir.getError();
+ return makeAbsolute(WorkingDir.get(), Path);
+}
+
+std::error_code
+RedirectingFileSystem::makeAbsolute(StringRef WorkingDir,
+ SmallVectorImpl<char> &Path) const {
// We can't use sys::fs::make_absolute because that assumes the path style
// is native and there is no way to override that. Since we know WorkingDir
// is absolute, we can use it to determine which style we actually have and
// append Path ourselves.
+ if (!WorkingDir.empty() &&
+ !sys::path::is_absolute(WorkingDir, sys::path::Style::posix) &&
+ !sys::path::is_absolute(WorkingDir,
+ sys::path::Style::windows_backslash)) {
+ return std::error_code();
+ }
sys::path::Style style = sys::path::Style::windows_backslash;
- if (sys::path::is_absolute(WorkingDir.get(), sys::path::Style::posix)) {
+ if (sys::path::is_absolute(WorkingDir, sys::path::Style::posix)) {
style = sys::path::Style::posix;
} else {
// Distinguish between windows_backslash and windows_slash; getExistingStyle
// returns posix for a path with windows_slash.
- if (getExistingStyle(WorkingDir.get()) !=
- sys::path::Style::windows_backslash)
+ if (getExistingStyle(WorkingDir) != sys::path::Style::windows_backslash)
style = sys::path::Style::windows_slash;
}
- std::string Result = WorkingDir.get();
+ std::string Result = std::string(WorkingDir);
StringRef Dir(Result);
if (!Dir.endswith(sys::path::get_separator(style))) {
Result += sys::path::get_separator(style);
}
+ // backslashes '\' are legit path charactors under POSIX. Windows APIs
+ // like CreateFile accepts forward slashes '/' as path
+ // separator (even when mixed with backslashes). Therefore,
+ // `Path` should be directly appended to `WorkingDir` without converting
+ // path separator.
Result.append(Path.data(), Path.size());
Path.assign(Result.begin(), Result.end());
@@ -1479,12 +1498,12 @@ directory_iterator RedirectingFileSystem::dir_begin(const Twine &Dir,
return Combined;
}
-void RedirectingFileSystem::setExternalContentsPrefixDir(StringRef PrefixDir) {
- ExternalContentsPrefixDir = PrefixDir.str();
+void RedirectingFileSystem::setOverlayFileDir(StringRef Dir) {
+ OverlayFileDir = Dir.str();
}
-StringRef RedirectingFileSystem::getExternalContentsPrefixDir() const {
- return ExternalContentsPrefixDir;
+StringRef RedirectingFileSystem::getOverlayFileDir() const {
+ return OverlayFileDir;
}
void RedirectingFileSystem::setFallthrough(bool Fallthrough) {
@@ -1619,6 +1638,20 @@ class llvm::vfs::RedirectingFileSystemParser {
return std::nullopt;
}
+ Optional<RedirectingFileSystem::RootRelativeKind>
+ parseRootRelativeKind(yaml::Node *N) {
+ SmallString<12> Storage;
+ StringRef Value;
+ if (!parseScalarString(N, Value, Storage))
+ return std::nullopt;
+ if (Value.equals_insensitive("cwd")) {
+ return RedirectingFileSystem::RootRelativeKind::CWD;
+ } else if (Value.equals_insensitive("overlay-dir")) {
+ return RedirectingFileSystem::RootRelativeKind::OverlayDir;
+ }
+ return std::nullopt;
+ }
+
struct KeyStatus {
bool Required;
bool Seen = false;
@@ -1826,7 +1859,7 @@ class llvm::vfs::RedirectingFileSystemParser {
SmallString<256> FullPath;
if (FS->IsRelativeOverlay) {
- FullPath = FS->getExternalContentsPrefixDir();
+ FullPath = FS->getOverlayFileDir();
assert(!FullPath.empty() &&
"External contents prefix directory must exist");
llvm::sys::path::append(FullPath, Value);
@@ -1883,9 +1916,19 @@ class llvm::vfs::RedirectingFileSystemParser {
sys::path::Style::windows_backslash)) {
path_style = sys::path::Style::windows_backslash;
} else {
- // Relative VFS root entries are made absolute to the current working
- // directory, then we can determine the path style from that.
- auto EC = sys::fs::make_absolute(Name);
+ // Relative VFS root entries are made absolute to either the overlay
+ // directory, or the current working directory, then we can determine
+ // the path style from that.
+ std::error_code EC;
+ if (FS->RootRelative ==
+ RedirectingFileSystem::RootRelativeKind::OverlayDir) {
+ StringRef FullPath = FS->getOverlayFileDir();
+ assert(!FullPath.empty() && "Overlay file directory must exist");
+ EC = FS->makeAbsolute(FullPath, Name);
+ Name = canonicalize(Name);
+ } else {
+ EC = sys::fs::make_absolute(Name);
+ }
if (EC) {
assert(NameValueNode && "Name presence should be checked earlier");
error(
@@ -1897,6 +1940,12 @@ class llvm::vfs::RedirectingFileSystemParser {
? sys::path::Style::posix
: sys::path::Style::windows_backslash;
}
+ // is::path::is_absolute(Name, sys::path::Style::windows_backslash) will
+ // return true even if `Name` is using forward slashes. Distinguish
+ // between windows_backslash and windows_slash.
+ if (path_style == sys::path::Style::windows_backslash &&
+ getExistingStyle(Name) != sys::path::Style::windows_backslash)
+ path_style = sys::path::Style::windows_slash;
}
// Remove trailing slash(es), being careful not to remove the root path
@@ -1960,6 +2009,7 @@ class llvm::vfs::RedirectingFileSystemParser {
KeyStatusPair("version", true),
KeyStatusPair("case-sensitive", false),
KeyStatusPair("use-external-names", false),
+ KeyStatusPair("root-relative", false),
KeyStatusPair("overlay-relative", false),
KeyStatusPair("fallthrough", false),
KeyStatusPair("redirecting-with", false),
@@ -2049,6 +2099,13 @@ class llvm::vfs::RedirectingFileSystemParser {
error(I.getValue(), "expected valid redirect kind");
return false;
}
+ } else if (Key == "root-relative") {
+ if (auto Kind = parseRootRelativeKind(I.getValue())) {
+ FS->RootRelative = *Kind;
+ } else {
+ error(I.getValue(), "expected valid root-relative kind");
+ return false;
+ }
} else {
llvm_unreachable("key missing from Keys");
}
@@ -2098,13 +2155,13 @@ RedirectingFileSystem::create(std::unique_ptr<MemoryBuffer> Buffer,
// Example:
// -ivfsoverlay dummy.cache/vfs/vfs.yaml
// yields:
- // FS->ExternalContentsPrefixDir => /<absolute_path_to>/dummy.cache/vfs
+ // FS->OverlayFileDir => /<absolute_path_to>/dummy.cache/vfs
//
SmallString<256> OverlayAbsDir = sys::path::parent_path(YAMLFilePath);
std::error_code EC = llvm::sys::fs::make_absolute(OverlayAbsDir);
assert(!EC && "Overlay dir final path must be absolute");
(void)EC;
- FS->setExternalContentsPrefixDir(OverlayAbsDir);
+ FS->setOverlayFileDir(OverlayAbsDir);
}
if (!P.parse(Root, FS.get()))
diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp
index e4f93cad3fbdf..242bb76865b2c 100644
--- a/llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1456,18 +1456,20 @@ class VFSFromYAMLTest : public ::testing::Test {
std::unique_ptr<vfs::FileSystem>
getFromYAMLRawString(StringRef Content,
- IntrusiveRefCntPtr<vfs::FileSystem> ExternalFS) {
+ IntrusiveRefCntPtr<vfs::FileSystem> ExternalFS,
+ StringRef YAMLFilePath = "") {
std::unique_ptr<MemoryBuffer> Buffer = MemoryBuffer::getMemBuffer(Content);
- return getVFSFromYAML(std::move(Buffer), CountingDiagHandler, "", this,
- ExternalFS);
+ return getVFSFromYAML(std::move(Buffer), CountingDiagHandler, YAMLFilePath,
+ this, ExternalFS);
}
std::unique_ptr<vfs::FileSystem> getFromYAMLString(
StringRef Content,
- IntrusiveRefCntPtr<vfs::FileSystem> ExternalFS = new DummyFileSystem()) {
+ IntrusiveRefCntPtr<vfs::FileSystem> ExternalFS = new DummyFileSystem(),
+ StringRef YAMLFilePath = "") {
std::string VersionPlusContent("{\n 'version':0,\n");
VersionPlusContent += Content.slice(Content.find('{') + 1, StringRef::npos);
- return getFromYAMLRawString(VersionPlusContent, ExternalFS);
+ return getFromYAMLRawString(VersionPlusContent, ExternalFS, YAMLFilePath);
}
// This is intended as a "XFAIL" for windows hosts.
@@ -1853,6 +1855,69 @@ TEST_F(VFSFromYAMLTest, ReturnsExternalPathVFSHit) {
EXPECT_EQ(0, NumDiagnostics);
}
+TEST_F(VFSFromYAMLTest, RootRelativeTest) {
+ IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());
+ Lower->addDirectory("//root/foo/bar");
+ Lower->addRegularFile("//root/foo/bar/a");
+ IntrusiveRefCntPtr<vfs::FileSystem> FS =
+ getFromYAMLString("{\n"
+ " 'case-sensitive': false,\n"
+ " 'root-relative': 'overlay-dir',\n"
+ " 'roots': [\n"
+ " { 'name': 'b', 'type': 'file',\n"
+ " 'external-contents': '//root/foo/bar/a'\n"
+ " }\n"
+ " ]\n"
+ "}",
+ Lower, "//root/foo/bar/overlay");
+
+ ASSERT_NE(FS.get(), nullptr);
+ ErrorOr<vfs::Status> S = FS->status("//root/foo/bar/b");
+ ASSERT_FALSE(S.getError());
+ EXPECT_EQ("//root/foo/bar/a", S->getName());
+
+ // On Windows, with overlay-relative set to true, the relative
+ // path in external-contents field will be prepend by OverlayDir
+ // with native path separator, regardless of the actual path separator
+ // used in YAMLFilePath field.
+#ifndef _WIN32
+ FS = getFromYAMLString("{\n"
+ " 'case-sensitive': false,\n"
+ " 'overlay-relative': true,\n"
+ " 'root-relative': 'overlay-dir',\n"
+ " 'roots': [\n"
+ " { 'name': 'b', 'type': 'file',\n"
+ " 'external-contents': 'a'\n"
+ " }\n"
+ " ]\n"
+ "}",
+ Lower, "//root/foo/bar/overlay");
+ ASSERT_NE(FS.get(), nullptr);
+ S = FS->status("//root/foo/bar/b");
+ ASSERT_FALSE(S.getError());
+ EXPECT_EQ("//root/foo/bar/a", S->getName());
+#else
+ IntrusiveRefCntPtr<DummyFileSystem> LowerWindows(new DummyFileSystem());
+ LowerWindows->addDirectory("\\\\root\\foo\\bar");
+ LowerWindows->addRegularFile("\\\\root\\foo\\bar\\a");
+ FS = getFromYAMLString("{\n"
+ " 'case-sensitive': false,\n"
+ " 'overlay-relative': true,\n"
+ " 'root-relative': 'overlay-dir',\n"
+ " 'roots': [\n"
+ " { 'name': 'b', 'type': 'file',\n"
+ " 'external-contents': 'a'\n"
+ " }\n"
+ " ]\n"
+ "}",
+ LowerWindows, "\\\\root\\foo\\bar\\overlay");
+ ASSERT_NE(FS.get(), nullptr);
+ S = FS->status("\\\\root\\foo\\bar\\b");
+ ASSERT_FALSE(S.getError());
+ EXPECT_EQ("\\\\root\\foo\\bar\\a", S->getName());
+#endif
+}
+
TEST_F(VFSFromYAMLTest, ReturnsInternalPathVFSHit) {
IntrusiveRefCntPtr<vfs::InMemoryFileSystem> BaseFS(
new vfs::InMemoryFileSystem);
More information about the cfe-commits
mailing list