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