[Lldb-commits] [PATCH] D121425: [VFS] Add a new getVFSFromYAMLs API

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

dexonsmith 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'm not sure how external-contents factors into this (shouldn't that just affect the observed file path, not whether the lookup succeeds?).

Regardless, here's the testcase again:

  OverlayFileSystem
    RedirectingFileSystem
      /a/foo -> bar
  
    RealFileSystem
      /a/bar
      /b/bar

I think that should behave like:

  InMemoryFileSystem
    /a/bar
    /a/foo -> bar (link, with "magic" to sometimes leak path)
    /b/bar
  
  cd /a # succeeds
  cat foo # succeeds (get content of /a/bar; report path of /a/bar or /a/foo)
  
  cd /b # succeeds
  cat foo # fails

Is the model correct?

If so, I think these are the things we need to get right (not just for that test case):

1. Return error on operations that should fail.
2. Get the right content what `cat` succeeds.
3. Reporting the "right" paths for files that are opened (depends on external contents, etc.).
4. Keeping the "view" / state consistent when the underlying filesystems changes state.

Does that breakdown make sense?

If so, can you clarify which part is failing? (Not sure if it's (3) or (4)... or maybe some (5).)

Or another way in: I don't fully understand why this fails:

>   cd /a
>   cat foo # fails since OverlayFS::setCWD doesn't set CWD on the underlying FS

Can you be more detailed about the overall state at the time of `cat`, which causes it to fail?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121425