[PATCH] Some infrastructure work for virtual file system

[Ben Langmuir]

On Feb 11, 2014, at 4:17 AM, Dmitri Gribenko <gribozavr at gmail.com> wrote:

> On Mon, Feb 10, 2014 at 10:32 PM, Ben Langmuir <blangmuir at apple.com> wrote:
>> Hi Doug et al.
>> 
>> This is the first patch in the implementation of the virtual file system discussed here: http://lists.cs.uiuc.edu/pipermail/cfe-dev/2014-February/035188.html.  It is intended to include enough of the AbstractFileSystem interface to replace the use of llvm::sys::fs in FileManager. Incidentally, I’ve included a number of the convenience methods that are trivially implemented on top of the status object.  When created by the CompilerInstance, FileManager will use a virtual file system created by the CI, but to avoid modifying every other user of the FileManager, in the absence of an explicit AbstractFileSystem to use, it will default to the real file system during construction.
>> 
>> Additionally, I’ve included a stub implementation of the OverlayFileSystem, which the CompilerInstance uses.  It is able to overlay files from a second file system, but will not merge directories correctly yet.  There are no users that actually push a second file system yet (other than the included unit test).
>> 
>> This patch is notably missing: the interfaces for recursive directory traversal, file system modification (create dir, rename, etc.), and any notion of thread safety.
> 
> Hi Ben,
> 
> This is looking great!
> 
> +/// \brief A file system that allows overlaying one \p
> AbstractFileSystem on top
> +/// of another.
> +///
> +/// Consists of a stack of >=1 \p AbstractFileSytem objects, which are treated
> +/// as being one merged file system. When there is a directory that exists
> +/// in more than one file system, the \p OverlayFileSystem contains a directory
> +/// containing the union of their contents.  When there is a file
> that exists in
> +/// more that one file system, the file in the top-most (most recently added)
> +/// file system overrides the other(s).
> 
> What about permissions -- the first overlay wins, right?  Please
> document and add tests.

For directories, yes.  This should be handled when merged directory support is added, but I can change the comment now if you like.

> 
> +  llvm::error_code status(const llvm::Twine &Path, Status &Result);
> 
> Please use LLVM_OVERRIDE (here and in other overrides).

Will do.

> 
> +  /// Create the virtual file system and replace any existing one with it.
> +  /// The top level of the virtual file system is an OverlayFileSystem with the
> +  /// RealFileSystem as its base and initially no overlays.
> +  void createVirtualFileSystem();
> 
> Is the correct default?  If some clients need an overlay, they should
> set it up from the start, rather than deciding late in the process
> "oh, and I think I need to add an overlay”.

That makes sense.  I guess the answer is to default to RealFileSystem, and make the client do something special if it wants something different.

> 
> --- /dev/null
> +++ b/lib/Basic/VirtualFileSystem.cpp
> @@ -0,0 +1,284 @@
> +//===- VirtualFileSystem.h - Virtual File System Layer ----------*-
> C++ -*-===//
> 
> File name does not match comment.

Oops. Will fix.

> 
> +  /// \brief Get an iterator pointing to the most recently added file system.
> +  iterator begin() { return FSList.rbegin(); }
> +
> +  /// \brief Get an iterator pointing one-past the least recently added file
> +  /// system.
> +  iterator end() { return FSList.rend(); }
> 
> begin/end is too generic, maybe overlays_begin() / overlays_end()?

And here I thought I would fit my for loops on one line ;)  I can use overlays_*, that seems like the best name (even though technically the base file system is not an overlay).

> 
> +  EXPECT_FALSE(AbstractFileSystem::equivalent(Status1, Status3));
> +}
> +
> +} // end anonymous namespace
> 
> There is no need to put tests in the unnamed namespace, please close
> it after the helper class.

Will do.

> 
> I just thought that we might need to resolve symlinks ourselves,
> here's why.  Suppose that we have a real FS:
> 
> /foo
> /foo/bar/bar.h
> /foo/baz/baz.h -> symlink to ../bar/bar.h
> 
> Then we push an overlay over /foo/bar.  If we rely on OS-level open(),
> then opening baz/baz.h will open bar.h from the real FS, but it is
> hidden by the overlay, thus we have an inconsistency.

Good catch. It looks like the overlay file system will need to stat every path prefix and then resolve links itself when it finds them.  That’s unpleasant, but doable.  Looks like we’ll need to add a read link function to sys::fs… which might be a pain on Windows.  I’m reading the documentation on Windows file system reparse points, and it’s not nearly as easy as calling ::readlink.

Ben

> 
> Dmitri
> 
> -- 
> main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
> (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/