dexonsmith added a comment. In D121425#3384479 <https://reviews.llvm.org/D121425#3384479>, @bnbarham wrote:
> In D121425#3384188 <https://reviews.llvm.org/D121425#3384188>, @bnbarham > wrote: > >> There's two failing tests with this change: >> >> - VFSFromYAMLTest.ReturnsExternalPathVFSHit >> - VFSFromYAMLTest.ReturnsInternalPathVFSHit >> >> Apparently we allow relative paths in external-contents *without* specifying >> `overlay-relative: true`. In this case the relative paths are resolved based >> on the CWD at the time of the operation. Do we really want that? I believe >> this wouldn't have worked prior to https://reviews.llvm.org/D109128 as there >> was no canonicalising before then as well. @keith was that change >> intentional? If we want it then it will *require* setting CWD on each FS in >> OverlayFS, which I was really hoping to avoid. > > I spoke to Keith offline. This has always worked - it previously worked by > `RedirectingFileSystem` setting CWD on `ExternalFS` when `setCWD` was called > on it. It's also important to keep supporting as it's used in Bazel > (https://github.com/bazelbuild/rules_swift/blob/c1d7d1df6969c2675c7826ecf1202d78016b1753/swift/internal/vfsoverlay.bzl#L41-L55). > > I'm hoping I can fix this by resolving the paths when the overlay is created. > I'll see if that works (it'll depend on when -working-directory is actually > used by the frontend). (Would this mean resolving everything in the `.yaml` file eagerly at launch? That sounds a bit scary...) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121425/new/ https://reviews.llvm.org/D121425 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
