[clang] support relative roots to vfs overlays

[rmaz]

Summary

This diff adds support for relative roots to VFS overlays. The directory root
will be made absolute from the current working directory and will be used to
determine the path style to use. This supports the use of VFS overlays with
remote build systems that might use a different working directory for each
compilation.

[rmaz]

lint

[rmaz]

clang-format changes

[keith]

This LGTM but I'm interested to hear from folks with more knowledge about the VFS' desired behavior to comment on if this will have any unexpected side effects

[dexonsmith]

In D116174#3206895, @keith wrote:

This LGTM but I'm interested to hear from folks with more knowledge about the VFS' desired behavior to comment on if this will have any unexpected side effects

I haven't reviewed the patch, but looking at the description I don't have concerns with this.

@benlangmuir or @vsapsai, any thoughts from you?

[benlangmuir]

Each VFS may have its own working directory, so it could be surprising if we use the OS working directory instead of that. This is complicated by the fact the VFS working directory may not be set yet during parsing the yaml (I haven't checked). I'm not really sure what to recommend here. If we do change this, we should document the new behaviour in the doc comment for RedirectingFileSystem though.

[rmaz]

In D116174#3242087, @benlangmuir wrote:

Each VFS may have its own working directory, so it could be surprising if we use the OS working directory instead of that. This is complicated by the fact the VFS working directory may not be set yet during parsing the yaml (I haven't checked). I'm not really sure what to recommend here. If we do change this, we should document the new behaviour in the doc comment for RedirectingFileSystem though.

I don't believe the working directory will be set on the VFS at this point, no. If you're happy with the change then i'll update the header docs to describe this behaviour, unless there is another way to achieve using relative paths for root entries in the overlay yaml?

[rmaz]

Add comment to describe relative root path behaviour

[benlangmuir]

Make sure the test failure gets fixed, but otherwise LGTM.

x64 debian > LLVM-Unit.Support/_/SupportTests::VFSFromYAMLTest.RelativePaths

[rmaz]

Update VirtualFileSystemTest to validate relative root paths.

[rmaz]

Windows test fixes

[rmaz]

Rebase on stable commit

clang/test/VFS/Inputs/vfsoverlay-root-relative.yaml

+{
+  'version': 0,
+  'fallthrough': true,
+  'overlay-relative': true,
+  'roots': [
+    { 'name': 'virtual',
+      'type': 'directory',
+      'contents': [
+        {
+          'external-contents': 'actual_header.h',
+          'type': 'file',
+          'name': 'virtual_header.h',
+        }
+      ]
+    }
+  ]
+}

clang/test/VFS/vfsoverlay-relative-root.c

+// RUN: %clang_cc1 -Werror -ivfsoverlay %S/Inputs/vfsoverlay-root-relative.yaml -I virtual -fsyntax-only %s
+
+#include "virtual_header.h"

llvm/include/llvm/Support/VirtualFileSystem.h

 ///   'version': <version number>,
 ///   <optional configuration>
 ///   'roots': [
 ///              <directory entries>
 ///            ]
 /// }
 /// \endverbatim
 ///
+/// The roots may be absolute or relative. If relative they will be made
+/// absolute against the current working directory.
+///
 /// All configuration options are optional.
 ///   'case-sensitive': <boolean, default=(true for Posix, false for Windows)>
 ///   'use-external-names': <boolean, default=true>
 ///   'overlay-relative': <boolean, default=false>
 ///   'fallthrough': <boolean, default=true>
 ///
 /// Virtual directories that list their contents are represented as
 /// \verbatim

llvm/lib/Support/VirtualFileSystem.cpp

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_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);
+        if (EC) {
           assert(NameValueNode && "Name presence should be checked earlier");
-        error(NameValueNode,
+          error(
+              NameValueNode,
               "entry with relative path at the root level is not discoverable");
           return nullptr;
         }
+        path_style = sys::path::is_absolute(Name, sys::path::Style::posix)
+                         ? sys::path::Style::posix
+                         : sys::path::Style::windows_backslash;
+      }
     }
 
     // Remove trailing slash(es), being careful not to remove the root path
     StringRef Trimmed = Name;
     size_t RootPathLen = sys::path::root_path(Trimmed, path_style).size();
     while (Trimmed.size() > RootPathLen &&
            sys::path::is_separator(Trimmed.back(), path_style))
       Trimmed = Trimmed.slice(0, Trimmed.size() - 1);

llvm/unittests/Support/VirtualFileSystemTest.cpp

for (int l = 0; I != E; I.increment(EC), ++l) {
     ASSERT_FALSE(EC);
     EXPECT_EQ(I.level(), l);
   }
   EXPECT_EQ(I, E);
 }
 
 TEST_F(VFSFromYAMLTest, RelativePaths) {
   IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());
+  std::error_code EC;
+  SmallString<128> CWD;
+  EC = llvm::sys::fs::current_path(CWD);
+  ASSERT_FALSE(EC);
+
   // Filename at root level without a parent directory.
   IntrusiveRefCntPtr<vfs::FileSystem> FS = getFromYAMLString(
       "{ 'roots': [\n"
       "  { 'type': 'file', 'name': 'file-not-in-directory.h',\n"
       "    'external-contents': '//root/external/file'\n"
       "  }\n"
       "] }",
       Lower);
-  EXPECT_EQ(nullptr, FS.get());
+  ASSERT_TRUE(FS.get() != nullptr);
+  SmallString<128> ExpectedPathNotInDir("file-not-in-directory.h");
+  llvm::sys::fs::make_absolute(ExpectedPathNotInDir);
+  checkContents(FS->dir_begin(CWD, EC), {ExpectedPathNotInDir});
 
   // Relative file path.
   FS = getFromYAMLString("{ 'roots': [\n"
-                         "  { 'type': 'file', 'name': 'relative/file/path.h',\n"
+                         "  { 'type': 'file', 'name': 'relative/path.h',\n"
                          "    'external-contents': '//root/external/file'\n"
                          "  }\n"
                          "] }",
                          Lower);
-  EXPECT_EQ(nullptr, FS.get());
+  ASSERT_TRUE(FS.get() != nullptr);
+  SmallString<128> Parent("relative");
+  llvm::sys::fs::make_absolute(Parent);
+  auto I = FS->dir_begin(Parent, EC);
+  ASSERT_FALSE(EC);
+  // Convert to POSIX path for comparison of windows paths
+  ASSERT_EQ("relative/path.h",
+            getPosixPath(std::string(I->path().substr(CWD.size() + 1))));
 
   // Relative directory path.
   FS = getFromYAMLString(
       "{ 'roots': [\n"
       "  { 'type': 'directory', 'name': 'relative/directory/path.h',\n"
       "    'contents': []\n"
       "  }\n"
       "] }",
       Lower);
-  EXPECT_EQ(nullptr, FS.get());
+  ASSERT_TRUE(FS.get() != nullptr);
+  SmallString<128> Root("relative/directory");
+  llvm::sys::fs::make_absolute(Root);
+  I = FS->dir_begin(Root, EC);
+  ASSERT_FALSE(EC);
+  ASSERT_EQ("path.h", std::string(I->path().substr(Root.size() + 1)));
 
-  EXPECT_EQ(3, NumDiagnostics);
+  EXPECT_EQ(0, NumDiagnostics);
 }
 
 TEST_F(VFSFromYAMLTest, NonFallthroughDirectoryIteration) {
   IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());
   Lower->addDirectory("//root/");
   Lower->addRegularFile("//root/a");
   Lower->addRegularFile("//root/b");
   IntrusiveRefCntPtr<vfs::FileSystem> FS = getFromYAMLString(