<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Nov 15, 2018, at 3:34 AM, Whisperity <<a href="mailto:whisperity@gmail.com" class="">whisperity@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><div dir="ltr" class=""><div dir="ltr" class=""><div class="">I am really not sure if adding real file system functionality strictly into the VFS is a good approach. This "ExternalFileSystem" thing sounds weird to me.</div></div></div></div></div></blockquote><div><br class=""></div><div>The `ExternalFileSystem` was an attempt to provide a more limited interface while exposing the "external" path in a way that made sense for the RedirectingFileSystem. Like Sam said in the review it's not great because it only does half of the work. </div><div><br class=""></div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div dir="ltr" class=""><div dir="ltr" class=""><div class="">Does LLDB need to *write* the files through the VFS? I'm not sure perhaps a "WritableVFS" could be implemented, or the necessary casting/conversion options.</div></div></div></div></div></blockquote><div><br class=""></div><div>Most likely yes because of LLDB's design that abstracts over flies without prior knowledge about whether they'd only get read or written. However wouldn't it suffer form the exact same problems? </div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div dir="ltr" class=""><div dir="ltr" class=""><div class="">In case:</div><div class=""> - there is a real path behind the file --- you could spawn an llvm::RealFileSystem (the fqdn might not be this after the migration patch!) and use that to obtain the file's buffer.</div></div></div></div></div></blockquote><div><br class=""></div><div>I'm not sure I follow what you have in mind here. Can you give a little more detail?</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div dir="ltr" class=""><div dir="ltr" class=""><div class="">How can you be sure the file actually exists on the FS? That's what the VFS should be all about, hiding this abstraction... if you *are* sure it exists, or want to make sure, you need to pull the appropriate realFS from the VFS Overlay (most tools have an overlay of a memoryFS above the realFS).</div></div></div></div></div></blockquote><div><br class=""></div><div>That makes sense, for LLDB's use case we would be happy having just a real or redirecting filesystem (with fall through). </div><br class=""><blockquote type="cite" class=""><div dir="ltr" class=""><div dir="ltr" class=""><div dir="ltr" class=""><div class="">What I am not sure about is extending the general interface in a way that it caters to a particular (or half of a particular) use case.</div></div></div></div></blockquote><div><br class=""></div><div>I totally understand this sentiment but I don't think that's totally fair. Finding files in different locations is an important feature of the VFS, when it was introduced in 2014 this was the only use case. The "devirtualization" aspect is unfortunate because native IO. </div><br class=""><blockquote type="cite" class=""><div dir="ltr" class=""><div dir="ltr" class=""><div dir="ltr" class=""><div class="">For example, in CodeCompass, we used a custom VFS implementation that "hijacked" the overlay and included itself between the realFS and the memoryFS. It obtains files from the database!</div><div class=""><br class=""></div><div class="">See:<br class=""></div><div class=""><a href="https://github.com/Ericsson/CodeCompass/blob/a1a7b10e3a9e2e4f493135ea68566cee54adc081/plugins/cpp_reparse/service/src/databasefilesystem.cpp#L191-L224" class="">https://github.com/Ericsson/CodeCompass/blob/a1a7b10e3a9e2e4f493135ea68566cee54adc081/plugins/cpp_reparse/service/src/databasefilesystem.cpp#L191-L224</a><br class=""></div><div class=""><br class=""></div><div class="">These files *do not necessarily* (in 99% of the cases, not at all) exist on the hard drive at the moment of the code wanting to pull the file, hence why we implemented this to give the file source buffer from DB. The ClangTool that needs this still gets the memoryFS for its own purposes, and for the clang libraries, the realFS is still under there.</div><div class=""><br class=""></div><div class="">Perhaps the "Status" type could be extended to carry extra information? <a href="https://github.com/Ericsson/CodeCompass/blob/a1a7b10e3a9e2e4f493135ea68566cee54adc081/plugins/cpp_reparse/service/src/databasefilesystem.cpp#L85-L87" class="">https://github.com/Ericsson/CodeCompass/blob/a1a7b10e3a9e2e4f493135ea68566cee54adc081/plugins/cpp_reparse/service/src/databasefilesystem.cpp#L85-L87</a><br class=""></div></div></div></div></blockquote><div><br class=""></div><div>This sounds like an interesting idea. We already have the option to expose the external name here, would it be reasonable to also expose the external path here? (of course being an optional)</div><br class=""><blockquote type="cite" class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">Sam McCall via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" class="">cfe-dev@lists.llvm.org</a>> ezt írta (időpont: 2018. nov. 15., Cs, 12:02):<br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class=""><div dir="ltr" class=""><div class="">I'd like to get some more perspectives on the role of the VirtualFileSystem abstraction in llvm/Support.</div><div class="">(The VFS layer has recently moved from Clang to LLVM, so crossposting to both lists)</div><div class=""><br class=""></div><div class=""><a href="https://reviews.llvm.org/D54277" target="_blank" class="">https://reviews.llvm.org/D54277</a> proposed adding a function to VirtualFileSystem to get the underlying "real file" path from a VFS path. LLDB is starting to use VFS for some filesystem interactions, but wants/needs to keep using native IO (FILE*, file descriptors) for others. There's some more context/discussion in the review.</div><div class=""><br class=""></div><div class="">My perspective is coloured by work on clang tooling, clangd etc. There we rely on VFS to ensure code (typically clang library code) works in a variety of environments, e.g:</div><div class="">in an IDE the edited file is consistently used rather than the one on disk</div><div class="">clang-tidy checks work on a local codebase, but our code review tool also runs them as a service</div><div class="">This works because all IO goes through the VFS, so VFSes are substitutable. We tend to rely on the static type system to ensure this (most people write lit tests that use the real FS).</div><div class=""><br class=""></div><div class="">Adding facilities to use native IO together with VFS works against this, e.g. a likely interface is</div><div class=""> // Returns the OS-native path to the specified virtual file.</div><div class=""> // Returns None if Path doesn't describe a native file, or its path is unknown.</div><div class=""> Optional<string> FileSystem::getNativePath(string Path)</div><div class="">Most potential uses of such a function are going to produce code that doesn't work well with arbitrary VFSes.</div><div class="">Anecdotally, filesystems are confusing, and most features exposed by VFS end up getting misused if possible.</div><div class=""><br class=""></div><div class="">So those are my reasons for pushing back on this change, but I'm not sure how strong they are.</div><div class="">I think broadly the alternatives for LLDB are:</div><div class="">make a change like this to the VFS APIs</div><div class="">migrate to actually doing IO using VFS (likely a lot of work)</div><div class="">know which concrete VFSes they construct, and track the needed info externally</div><div class="">stop using VFS, and build separate abstractions for tracking remapping of native files etc</div><div class="">abandon the new features that depend on this file remapping</div><div class=""><br class=""></div><div class="">As a purist, 2 and 4 seem like the cleanest options, but that's easy to say when it's someone else's work.</div><div class="">What path should we take here?</div><div class=""><br class=""></div><div class="">Cheers, Sam</div></div></div>
_______________________________________________<br class="">
cfe-dev mailing list<br class="">
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank" class="">cfe-dev@lists.llvm.org</a><br class="">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br class="">
</blockquote></div>
</blockquote></div><br class=""></body></html>