... and LG from my side
On Wed, Feb 12, 2014 at 9:08 AM, Manuel Klimek <[email protected]> wrote: > On Tue, Feb 11, 2014 at 10:13 PM, Ben Langmuir <[email protected]>wrote: > >> >> On Feb 11, 2014, at 12:23 PM, Argyrios Kyrtzidis <[email protected]> >> wrote: >> >> > On Feb 10, 2014, at 2: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. >> >> >> >> Ben >> >> >> >> <vfs-part1.patch> >> >> >> > >> > >> > Looks good! >> > >> > To add on the nitpicking.. :-) >> > >> > + /// In addition to providing the usual stat information, this class >> provides >> > + /// \p Name and \p RealName fields, which can be used by clients >> that need >> > + /// to differentiate between the name that this object was looked up >> by, >> > + /// and the real name of this object on disk (if there is one). >> > >> > Could you clarify what should happen if there is no real path (it's a >> virtual file), is RealName == Name or is it empty ? >> >> RealName == “” >> >> I’ll update the comment. >> >> > >> > >> >>> >> >>> 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. >> > >> > I'm concerned about the performance implications of stat'ing everything >> and trying to resolve symlinks, and I'm not sure how bad the issue will be >> in practice. >> > Could we defer investigating into this for after we have the simpler >> case working ? >> >> With caching I don’t think the performance will be a big problem, but we >> will have to see. I’m fine with deferring this. >> >> > >> > On a related note, what about using a filename with different casing >> from what it was declared in the vfs ('myfoo.h' vs 'MyFoo.h'). >> > >> >> >> Since the overlay defers to other file systems, I don’t think it needs to >> care. When we add a file system for vfs files, we probably need an option >> to do case-insensitive matching. I’m not sure if that is a property of the >> host or just a property of the filesystem though… >> > > I would intuitively say it's a property of the file system; you can mount > case insensitive file systems on unix'es, or case sensitive ones on windows. > Also, one feature I'm looking for (mid to long term) where this is a great > step in the right direction (so we really appreciate your work here ;) is > to be able to replay Windows-based compilations on Unix machines; think bug > reports where you get a Windows user to file a bug with the full set of > "virtual files" and we can just replay it anywhere. > > Cheers, > /Manuel > > >> >> Ben >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
