From 738b5c9639b4323f75b03e21494bef13d9adfa86 Mon Sep 17 00:00:00 2001 From: Adrian McCarthy Date: Mon, 25 Nov 2019 15:57:21 -0800 Subject: [PATCH] Fix more VFS tests on Windows Since VFS paths can be in either Posix or Windows style, we have to use a more flexible definition of "absolute" path. The key here is that FileSystem::makeAbsolute is now virtual, and the RedirectingFileSystem override checks for either concept of absolute before trying to make the path absolute by combining it with the current directory. Differential Revision: https://reviews.llvm.org/D70701 --- clang/test/VFS/vfsroot-include.c | 3 -- clang/test/VFS/vfsroot-module.m | 3 -- clang/test/VFS/vfsroot-with-overlay.c | 3 -- llvm/include/llvm/Support/VirtualFileSystem.h | 4 ++- llvm/lib/Support/VirtualFileSystem.cpp | 43 ++++++++++++++++++++------- 5 files changed, 36 insertions(+), 20 deletions(-) diff --git a/clang/test/VFS/vfsroot-include.c b/clang/test/VFS/vfsroot-include.c index 2564004..eb641c5 100644 --- a/clang/test/VFS/vfsroot-include.c +++ b/clang/test/VFS/vfsroot-include.c @@ -1,6 +1,3 @@ -// FIXME: PR43272 -// XFAIL: system-windows - // RUN: rm -rf %t // RUN: mkdir -p %t // RUN: sed -e "s@TEST_DIR@%{/S:regex_replacement}@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/vfsroot.yaml > %t.yaml diff --git a/clang/test/VFS/vfsroot-module.m b/clang/test/VFS/vfsroot-module.m index 3ad3e19..787e2f5 100644 --- a/clang/test/VFS/vfsroot-module.m +++ b/clang/test/VFS/vfsroot-module.m @@ -1,6 +1,3 @@ -// FIXME: PR43272 -// XFAIL: system-windows - // RUN: rm -rf %t // RUN: mkdir -p %t // RUN: sed -e "s@TEST_DIR@%{/S:regex_replacement}@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/vfsroot.yaml > %t.yaml diff --git a/clang/test/VFS/vfsroot-with-overlay.c b/clang/test/VFS/vfsroot-with-overlay.c index 4a2c64c..d181f4d 100644 --- a/clang/test/VFS/vfsroot-with-overlay.c +++ b/clang/test/VFS/vfsroot-with-overlay.c @@ -1,6 +1,3 @@ -// FIXME: PR43272 -// XFAIL: system-windows - // RUN: rm -rf %t // RUN: mkdir -p %t // RUN: sed -e "s@TEST_DIR@%{/S:regex_replacement}@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/vfsroot.yaml > %t.yaml diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h index 3b4c4af..e45e6e7 100644 --- a/llvm/include/llvm/Support/VirtualFileSystem.h +++ b/llvm/include/llvm/Support/VirtualFileSystem.h @@ -293,7 +293,7 @@ public: /// \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(SmallVectorImpl &Path) const; + virtual std::error_code makeAbsolute(SmallVectorImpl &Path) const; }; /// Gets an \p vfs::FileSystem for the 'real' file system, as seen by @@ -749,6 +749,8 @@ public: std::error_code isLocal(const Twine &Path, bool &Result) override; + std::error_code makeAbsolute(SmallVectorImpl &Path) const override; + directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override; void setExternalContentsPrefixDir(StringRef PrefixDir); diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp index 2d5c04b..edd4234 100644 --- a/llvm/lib/Support/VirtualFileSystem.cpp +++ b/llvm/lib/Support/VirtualFileSystem.cpp @@ -1073,6 +1073,19 @@ std::error_code RedirectingFileSystem::isLocal(const Twine &Path, return ExternalFS->isLocal(Path, Result); } +std::error_code RedirectingFileSystem::makeAbsolute(SmallVectorImpl &Path) const { + if (llvm::sys::path::is_absolute(Path, llvm::sys::path::Style::posix) || + llvm::sys::path::is_absolute(Path, llvm::sys::path::Style::windows)) + return {}; + + auto WorkingDir = getCurrentWorkingDirectory(); + if (!WorkingDir) + return WorkingDir.getError(); + + llvm::sys::fs::make_absolute(WorkingDir.get(), Path); + return {}; +} + directory_iterator RedirectingFileSystem::dir_begin(const Twine &Dir, std::error_code &EC) { ErrorOr E = lookupPath(Dir); @@ -1428,21 +1441,31 @@ class llvm::vfs::RedirectingFileSystemParser { return nullptr; } - if (IsRootEntry && !sys::path::is_absolute(Name)) { - assert(NameValueNode && "Name presence should be checked earlier"); - error(NameValueNode, - "entry with relative path at the root level is not discoverable"); - return nullptr; + sys::path::Style path_style = sys::path::Style::native; + if (IsRootEntry) { + // VFS root entries may be in either Posix or Windows style. Figure out + // which style we have, and use it consistently. + if (sys::path::is_absolute(Name, sys::path::Style::posix)) { + path_style = sys::path::Style::posix; + } else if (sys::path::is_absolute(Name, sys::path::Style::windows)) { + path_style = sys::path::Style::windows; + } else { + assert(NameValueNode && "Name presence should be checked earlier"); + error(NameValueNode, + "entry with relative path at the root level is not discoverable"); + return nullptr; + } } // Remove trailing slash(es), being careful not to remove the root path StringRef Trimmed(Name); - size_t RootPathLen = sys::path::root_path(Trimmed).size(); + size_t RootPathLen = sys::path::root_path(Trimmed, path_style).size(); while (Trimmed.size() > RootPathLen && - sys::path::is_separator(Trimmed.back())) + sys::path::is_separator(Trimmed.back(), path_style)) Trimmed = Trimmed.slice(0, Trimmed.size() - 1); + // Get the last component - StringRef LastComponent = sys::path::filename(Trimmed); + StringRef LastComponent = sys::path::filename(Trimmed, path_style); std::unique_ptr Result; switch (Kind) { @@ -1460,12 +1483,12 @@ class llvm::vfs::RedirectingFileSystemParser { break; } - StringRef Parent = sys::path::parent_path(Trimmed); + StringRef Parent = sys::path::parent_path(Trimmed, path_style); if (Parent.empty()) return Result; // if 'name' contains multiple components, create implicit directory entries - for (sys::path::reverse_iterator I = sys::path::rbegin(Parent), + for (sys::path::reverse_iterator I = sys::path::rbegin(Parent, path_style), E = sys::path::rend(Parent); I != E; ++I) { std::vector> Entries; -- 2.7.4