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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
