[PATCH] D54277: Extend VFS with function to get external path.

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere planned changes to this revision.
JDevlieghere added a comment.

Thanks for bearing with me on this, Sam. I really appreciate you taking the time to find the best solution that works for all of us.

In https://reviews.llvm.org/D54277#1298127, @sammccall wrote:

> So if I understand correctly, you want a way to devirtualize the VFS: to take a VFS file and get a concrete `FILE*` out of it. (In this case, via the filename and `fopen()`).
>
> The fact that VFS doesn't have any way to do this is an important property, it's what makes the file system virtual.
>  VFS allows code to work in the same way on OS-native, in-memory, or distributed filesystems, and `getExternalPath` would break that - any code that called it can only work when the files are 1:1 with native OS files. Code that wants to couple to the FS implementation in this way shouldn't work transparently with VFS, because it breaks the design goal that VFSes are substitutable.
>
> For what you're trying to do, I'd suggest one of the following:
>
> - use a concrete VFS implementation that maps files and exposes the functions you need (e.g. expose RedirectingFileSystem from VFS.h with an appropriate public interface)


I guess this could work if we ignore the concern Volodymyr raised. If we assume that the `ExternalFS` is the `RealFileSystem` this could work. The problem is that `RedirectingFileSystem` takes any `FileSystem` as its external file system so if that doesn't have the right API we can't "dig deeper".

> - use the existing `getRealPath` if you know which concrete VFSes you're using and it happens to do what you want.

This is not a viable option because it defeats the purpose of the VFS.

> - track the correspondence between virtual and real paths externally (where you set up the FS)

This is not a viable option because it defeats the purpose of the VFS.

> - decouple LLDB's IO from the concrete filesystem (use VFS file instead of FILE*)

This is also not a viable option because, like I said earlier, this would be too big of a change for LLDB. In addition to the risk such a change brings, I don't have the bandwidth for it and even if I did I'm not sure it be feasible. Pavel said he looked into this and found that some of LLDB's dependencies formed a problem.

I'll update the patch to reflect what (1) would look like.


https://reviews.llvm.org/D54277