From f8f064133b8ed9cd8cead1ee416b7c44729184ae Mon Sep 17 00:00:00 2001 From: Haowei Wu Date: Fri, 4 Nov 2022 16:40:10 -0700 Subject: [PATCH] [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 --- .../VFS/Inputs/root-relative-overlay.yaml | 11 +++ clang/test/VFS/relative-path.c | 7 ++ llvm/include/llvm/Support/VirtualFileSystem.h | 56 ++++++++++-- llvm/lib/Support/VirtualFileSystem.cpp | 85 ++++++++++++++++--- .../Support/VirtualFileSystemTest.cpp | 75 ++++++++++++++-- 5 files changed, 207 insertions(+), 27 deletions(-) create mode 100644 clang/test/VFS/Inputs/root-relative-overlay.yaml 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 000000000000..931c1b4b73c5 --- /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 ab207d096848..ba5451136a0c 100644 --- a/clang/test/VFS/relative-path.c +++ b/clang/test/VFS/relative-path.c @@ -3,6 +3,13 @@ // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@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@OVERLAY_DIR@true@g" -e "s@EXTERNAL_DIR@.@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@OVERLAY_DIR@false@g" -e "s@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 6844a406f38c..671b610c93e3 100644 --- a/llvm/include/llvm/Support/VirtualFileSystem.h +++ b/llvm/include/llvm/Support/VirtualFileSystem.h @@ -651,17 +651,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': /// 'use-external-names': +/// 'root-relative': /// 'overlay-relative': /// 'fallthrough': /// 'redirecting-with': /// +/// 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 /// { @@ -745,6 +756,15 @@ public: 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; @@ -890,6 +910,21 @@ private: ErrorOr 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 => /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 &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 @@ -910,10 +945,11 @@ private: /// The file system to use for external references. IntrusiveRefCntPtr 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 /// @{ @@ -923,7 +959,7 @@ private: /// 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; @@ -934,6 +970,10 @@ private: /// 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 ExternalFS); @@ -984,9 +1024,9 @@ public: 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 97d63fff1069..656d565fb692 100644 --- a/llvm/lib/Support/VirtualFileSystem.cpp +++ b/llvm/lib/Support/VirtualFileSystem.cpp @@ -1350,32 +1350,51 @@ std::error_code RedirectingFileSystem::makeAbsolute(SmallVectorImpl &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 &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()); @@ -1482,12 +1501,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) { @@ -1621,6 +1640,20 @@ class llvm::vfs::RedirectingFileSystemParser { return None; } + Optional + parseRootRelativeKind(yaml::Node *N) { + SmallString<12> Storage; + StringRef Value; + if (!parseScalarString(N, Value, Storage)) + return None; + if (Value.equals_insensitive("cwd")) { + return RedirectingFileSystem::RootRelativeKind::CWD; + } else if (Value.equals_insensitive("overlay-dir")) { + return RedirectingFileSystem::RootRelativeKind::OverlayDir; + } + return None; + } + struct KeyStatus { bool Required; bool Seen = false; @@ -1828,7 +1861,7 @@ private: 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); @@ -1885,9 +1918,19 @@ private: 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( @@ -1899,6 +1942,12 @@ private: ? 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 @@ -1962,6 +2011,7 @@ public: 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), @@ -2051,6 +2101,13 @@ public: 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"); } @@ -2100,13 +2157,13 @@ RedirectingFileSystem::create(std::unique_ptr Buffer, // Example: // -ivfsoverlay dummy.cache/vfs/vfs.yaml // yields: - // FS->ExternalContentsPrefixDir => //dummy.cache/vfs + // FS->OverlayFileDir => //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 1e300eec711d..6cefd1d02ba3 100644 --- a/llvm/unittests/Support/VirtualFileSystemTest.cpp +++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp @@ -1452,18 +1452,20 @@ public: std::unique_ptr getFromYAMLRawString(StringRef Content, - IntrusiveRefCntPtr ExternalFS) { + IntrusiveRefCntPtr ExternalFS, + StringRef YAMLFilePath = "") { std::unique_ptr Buffer = MemoryBuffer::getMemBuffer(Content); - return getVFSFromYAML(std::move(Buffer), CountingDiagHandler, "", this, - ExternalFS); + return getVFSFromYAML(std::move(Buffer), CountingDiagHandler, YAMLFilePath, + this, ExternalFS); } std::unique_ptr getFromYAMLString( StringRef Content, - IntrusiveRefCntPtr ExternalFS = new DummyFileSystem()) { + IntrusiveRefCntPtr 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. @@ -1849,6 +1851,69 @@ TEST_F(VFSFromYAMLTest, ReturnsExternalPathVFSHit) { EXPECT_EQ(0, NumDiagnostics); } +TEST_F(VFSFromYAMLTest, RootRelativeTest) { + IntrusiveRefCntPtr Lower(new DummyFileSystem()); + Lower->addDirectory("//root/foo/bar"); + Lower->addRegularFile("//root/foo/bar/a"); + IntrusiveRefCntPtr 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 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 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 BaseFS( new vfs::InMemoryFileSystem); -- 2.38.1.1.g6d9df9d320