[llvm] 738b5c9 - Fix more VFS tests on Windows

Adrian McCarthy via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 18 11:38:41 PST 2019


Author: Adrian McCarthy
Date: 2019-12-18T11:38:04-08:00
New Revision: 738b5c9639b4323f75b03e21494bef13d9adfa86

URL: https://github.com/llvm/llvm-project/commit/738b5c9639b4323f75b03e21494bef13d9adfa86
DIFF: https://github.com/llvm/llvm-project/commit/738b5c9639b4323f75b03e21494bef13d9adfa86.diff

LOG: 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

Added: 
    

Modified: 
    clang/test/VFS/vfsroot-include.c
    clang/test/VFS/vfsroot-module.m
    clang/test/VFS/vfsroot-with-overlay.c
    llvm/include/llvm/Support/VirtualFileSystem.h
    llvm/lib/Support/VirtualFileSystem.cpp

Removed: 
    


################################################################################
diff  --git a/clang/test/VFS/vfsroot-include.c b/clang/test/VFS/vfsroot-include.c
index 2564004ea4b1..eb641c58a650 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 at TEST_DIR@%{/S:regex_replacement}@g" -e "s at 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 3ad3e19d4b37..787e2f513c55 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 at TEST_DIR@%{/S:regex_replacement}@g" -e "s at 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 4a2c64cb8734..d181f4d8382c 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 at TEST_DIR@%{/S:regex_replacement}@g" -e "s at 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 3b4c4aff9bdc..e45e6e756786 100644
--- a/llvm/include/llvm/Support/VirtualFileSystem.h
+++ b/llvm/include/llvm/Support/VirtualFileSystem.h
@@ -293,7 +293,7 @@ class FileSystem : public llvm::ThreadSafeRefCountedBase<FileSystem> {
   /// \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<char> &Path) const;
+  virtual std::error_code makeAbsolute(SmallVectorImpl<char> &Path) const;
 };
 
 /// Gets an \p vfs::FileSystem for the 'real' file system, as seen by
@@ -749,6 +749,8 @@ class RedirectingFileSystem : public vfs::FileSystem {
 
   std::error_code isLocal(const Twine &Path, bool &Result) override;
 
+  std::error_code makeAbsolute(SmallVectorImpl<char> &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 2d5c04baa57e..edd4234fe501 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<char> &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<RedirectingFileSystem::Entry *> 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<RedirectingFileSystem::Entry> 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<std::unique_ptr<RedirectingFileSystem::Entry>> Entries;


        


More information about the llvm-commits mailing list