[PATCH] D76670: [VirtualFileSystem] Support directory entries in the YAMLVFSWriter

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: bkramer, arphaman, vsapsai, sammccall.
JDevlieghere added a project: LLVM.
Herald added subscribers: dexonsmith, hiraditya.

The current implementation of the JSONWriter does not support writing out directory entries. Earlier today I added a unit test to illustrate the problem. When an entry is added to the YAMLVFSWriter and the path is a directory, it will incorrectly emit the directory as a file, and any files inside that directory will not be found by the VFS.

Usually it's possible to work around the issue by only adding leaf nodes (files) to the YAMLVFSWriter. However, this doesn't work for representing empty directories. This is a real issue for clients of the VFS that want to iterate over a directory. The directory not being there is not the same as the directory being empty.

This is not just a hypothetical problem but a real issue. The FileCollector does not differentiate between file and directory paths. I temporarily worked around the issue for LLDB by ignoring directories, but I suspect this will prove problematic sooner rather than later.

This patch fixes the issue by extending the JSONWriter to support writing out directory entries. We have to special-case the first entry, but otherwise it's just a matter of emitting a directory entry instead of a file entry.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76670

Files:
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp


Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===================================================================
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -2200,12 +2200,6 @@
   ScopedDir _g(TestDirectory + "/g");
   ScopedFile _h(TestDirectory + "/h", "");
 
-  // This test exposes a bug/shortcoming in the YAMLVFSWriter. Below we call
-  // addFileMapping for _a and _e, which causes _ab and _ef not to exists in
-  // the deserialized file system, because _a and _e got emitted as regular
-  // files. The counter example is _c, if we only call addFileMapping for _cd,
-  // things work as expected.
-
   vfs::YAMLVFSWriter VFSWriter;
   VFSWriter.addFileMapping(_a.Path, "//root/a");
   VFSWriter.addFileMapping(_ab.Path, "//root/a/b");
@@ -2236,11 +2230,11 @@
   ASSERT_TRUE(FS.get() != nullptr);
 
   EXPECT_TRUE(FS->exists(_a.Path));
-  EXPECT_FALSE(FS->exists(_ab.Path)); // FIXME: See explanation above.
+  EXPECT_TRUE(FS->exists(_ab.Path));
   EXPECT_TRUE(FS->exists(_c.Path));
   EXPECT_TRUE(FS->exists(_cd.Path));
   EXPECT_TRUE(FS->exists(_e.Path));
-  EXPECT_FALSE(FS->exists(_ef.Path)); // FIXME: See explanation above.
+  EXPECT_TRUE(FS->exists(_ef.Path));
   EXPECT_TRUE(FS->exists(_g.Path));
   EXPECT_TRUE(FS->exists(_h.Path));
 }
Index: llvm/lib/Support/VirtualFileSystem.cpp
===================================================================
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -2017,7 +2017,10 @@
 
   if (!Entries.empty()) {
     const YAMLVFSEntry &Entry = Entries.front();
-    startDirectory(path::parent_path(Entry.VPath));
+    bool first_entry_is_directory = fs::is_directory(Entry.VPath);
+    StringRef Dir =
+        first_entry_is_directory ? Entry.VPath : path::parent_path(Entry.VPath);
+    startDirectory(Dir);
 
     StringRef RPath = Entry.RPath;
     if (UseOverlayRelative) {
@@ -2027,13 +2030,19 @@
       RPath = RPath.slice(OverlayDirLen, RPath.size());
     }
 
-    writeEntry(path::filename(Entry.VPath), RPath);
+    if (!first_entry_is_directory)
+      writeEntry(path::filename(Entry.VPath), RPath);
 
     for (const auto &Entry : Entries.slice(1)) {
-      StringRef Dir = path::parent_path(Entry.VPath);
-      if (Dir == DirStack.back())
-        OS << ",\n";
-      else {
+      StringRef Dir = fs::is_directory(Entry.VPath)
+                          ? Entry.VPath
+                          : path::parent_path(Entry.VPath);
+      if (Dir == DirStack.back()) {
+        if (!first_entry_is_directory) {
+          OS << ",\n";
+          first_entry_is_directory = false;
+        }
+      } else {
         while (!DirStack.empty() && !containedIn(DirStack.back(), Dir)) {
           OS << "\n";
           endDirectory();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76670.252228.patch
Type: text/x-patch
Size: 2817 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200324/d493ee41/attachment.bin>