On Mon, Feb 10, 2014 at 10:32 PM, Ben Langmuir <[email protected]> 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.
+ llvm::error_code status(const llvm::Twine &Path, Status &Result);
Please use LLVM_OVERRIDE (here and in other overrides).
+ /// 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".
--- /dev/null
+++ b/lib/Basic/VirtualFileSystem.cpp
@@ -0,0 +1,284 @@
+//===- VirtualFileSystem.h - Virtual File System Layer ----------*-
C++ -*-===//
File name does not match comment.
+ /// \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()?
+ 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.
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.
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 <[email protected]>*/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
