[PATCH] D121425: [VFS] Add a new getVFSFromYAMLs API

Tue Mar 15 17:42:03 PDT 2022

bnbarham added a comment.

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).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121425