<div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Aug 28, 2019, 10:04 PM Jonas Devlieghere via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">JDevlieghere added a comment.<br>
<br>
In D65677#1649329 <<a href="https://reviews.llvm.org/D65677#1649329" rel="noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D65677#1649329</a>>, @sammccall wrote:<br>
<br>
> In D65677#1649291 <<a href="https://reviews.llvm.org/D65677#1649291" rel="noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D65677#1649291</a>>, @JDevlieghere wrote:<br>
><br>
> > 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. :-)<br>
><br>
><br>
> Right! I think the key distinction is that there wasn't any functional change to the APIs, because the abstraction didn't change.<br>
>  (There was a second factory function, which basically lets callers choose between the two implementations that have different sets of bugs)<br>
><br>
> >>>   What do you think? Is there another approach that's worth considering?<br>
> >> <br>
> >> 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)?<br>
> > <br>
> > 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.<br>
><br>
> I mean why can't you just call `setCurrentWorkingDirectory` before starting? something like this:<br>
><br>
>   struct Reproducer { string FSYaml; string CWD; ... };<br>
>   void run(Reproducer &R) {<br>
>     auto* FS = RedirectingFileSystem::create(R.FSYaml, ...);<br>
>     FS->setCurrentWorkingDirectory(R.CWD);<br>
>     runLLDBWith(FS, ...);<br>
>   }<br>
>  <br>
><br>
<br>
<br>
That doesn't work for the reproducer because the working directory likely doesn't exist during replay. The</blockquote></div></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> redirecting file system just forwards the `setCurrentWorkingDirectory` call to it's underlying (real) FS, so this becomes a no-op.</blockquote></div></div><div dir="auto">Can we fix that instead? The RedirectingVFS already knows about virtual directory entries, being able to cd to them makes sense.</div><div dir="auto"><br></div><div dir="auto">This seems mostly like fixing a bug/limitation that wouldn't involve API changes.<br></div><div dir="auto"><br></div><div dir="auto">Is there a part of the problem that needs the new search path concept/multiple paths?</div></div>