[PATCH] D65677: [VirtualFileSystem] Support encoding working directories in a VFS mapping.

Sam McCall via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 28 13:49:07 PDT 2019


On Wed, Aug 28, 2019, 10:04 PM Jonas Devlieghere via Phabricator <
reviews at reviews.llvm.org> wrote:

> JDevlieghere added a comment.
>
> In D65677#1649329 <https://reviews.llvm.org/D65677#1649329>, @sammccall
> wrote:
>
> > In D65677#1649291 <https://reviews.llvm.org/D65677#1649291>,
> @JDevlieghere wrote:
> >
> > > It's funny you say that, because the code to resolve relative paths is
> almost identical to the thing you added for supporting different working
> directories in different threads. :-)
> >
> >
> > Right! I think the key distinction is that there wasn't any functional
> change to the APIs, because the abstraction didn't change.
> >  (There was a second factory function, which basically lets callers
> choose between the two implementations that have different sets of bugs)
> >
> > >>>   What do you think? Is there another approach that's worth
> considering?
> > >>
> > >> Per my previous comment, what goes wrong if you try to make the
> working directory a sibling of VFS (within the reproducer container) rather
> than a child of it (within shared infrastructure)?
> > >
> > > Oops, seems like I didn't see your question either :-) Please clarify
> what you mean by sibling and child. Do you mean that the working
> directories are part of the mapping or that the redirecting file system
> knows about it? I don't care where we store the entries, I'm happy to have
> a separate YAML mapping that only the LLDB reproducers know about. However,
> I do need the underlying logic to be part of the (redirecting) VFS. Just
> like clang, LLDB is agnostic to the VFS, so this whole thing should be
> transparent. The only reason I didn't keep them separate is because then
> you need a way to tell the redirecting file system about the working
> directories, which means you need to get the concrete VFS, i.e. casting the
> VFS to a RedirectingVFS, which I try to avoid.
> >
> > I mean why can't you just call `setCurrentWorkingDirectory` before
> starting? something like this:
> >
> >   struct Reproducer { string FSYaml; string CWD; ... };
> >   void run(Reproducer &R) {
> >     auto* FS = RedirectingFileSystem::create(R.FSYaml, ...);
> >     FS->setCurrentWorkingDirectory(R.CWD);
> >     runLLDBWith(FS, ...);
> >   }
> >
> >
>
>
> That doesn't work for the reproducer because the working directory likely
> doesn't exist during replay. The

redirecting file system just forwards the `setCurrentWorkingDirectory` call
> to it's underlying (real) FS, so this becomes a no-op.

Can we fix that instead? The RedirectingVFS already knows about virtual
directory entries, being able to cd to them makes sense.

This seems mostly like fixing a bug/limitation that wouldn't involve API
changes.

Is there a part of the problem that needs the new search path
concept/multiple paths?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190828/67e3069c/attachment.html>


More information about the llvm-commits mailing list