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

[Sam McCall via Phabricator via llvm-commits]

sammccall added a comment.

(I replied by email, it seems phabricator doesn't pick it up.)

In D65677#1649731 <https://reviews.llvm.org/D65677#1649731>, @JDevlieghere wrote:

> 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?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65677/new/

https://reviews.llvm.org/D65677