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

[Sam McCall via Phabricator via lldb-commits]

sammccall added a comment.

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

> In D65677#1648506 <https://reviews.llvm.org/D65677#1648506>, @sammccall wrote:
>
> > In D65677#1627576 <https://reviews.llvm.org/D65677#1627576>, @JDevlieghere wrote:
> >
> > > After some brainstorming I've identified a few other approaches that should better reflect the transience of the current working directory:
> > >
> > > - We can modify the VFS to have a notion of //search paths//. The `adjustPath` function could iterate over the search paths until it finds an absolute path that exists. If none exist we'd return the same path we return today. This would be the most general approach.
> > > - We could create a new virtual file system that we put on top of the RedirectingFileSystem which does something similar to what I've described above. This would require us to override every function that calls `adjustPath`, so it would be pretty heavyweight and rather inflexible.
> > >
> > >   I'd like to get your feedback before I start implementing these. What do you think? Is there another approach that's worth considering?
> >
> >
> > I'm really sorry for missing this comment, I was out sick and missed the email.
> >
> > > I'd like to get your feedback before I start implementing these.
> >
> > Honestly, this still seems way too complicated and this doesn't seem like a feature that needs to be part of VFS.
>
>
> 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, ...);
  }


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

https://reviews.llvm.org/D65677