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

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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