bnbarham added a comment.
In D121425#3384492 <https://reviews.llvm.org/D121425#3384492>, @dexonsmith
wrote:
> Can you be more detailed about the overall state at the time of `cat`, which
> causes it to fail?
Sure. I think there's also some confusion with names here but I'm generally
trying to use the names as they are used in the overlay YAML or the name of the
members in RedirectingFS.
"external-contents" is just the path that we're mapping to, so in my example
above (/a/foo -> bar), "bar" is what I was referring to by "external-contents".
So let's use that again -
OverlayFileSystem
RedirectingFileSystem
/a/foo -> bar
RealFileSystem
/a/bar
/b/bar
After D121423 <https://reviews.llvm.org/D121423>, `cd /a` will result in the
following -
- OverlayFS: CWD set to `/a`
- RedirectingFS and RealFS: CWD unchanged, so let's assume they were `/` at the
time of creation and that's what they are now
`cat foo` -
- OverlayFS canonicalises to `/a/foo` and passes that to `openFileForRead` in
RedirectingFS
- RedirectingFS has a `/a/foo` -> `bar` mapping, so it now runs
`openFileForRead` on its ExternalFS (which is the RealFS)
- RealFS is at `/` and there is no `/bar` and thus fails
- RedirectingFS also fails
- Back in OverlayFS now, and because we failed we now try RealFS with `/a/foo`
- RealFS has no `/a/foo`, it fails
- Back in OverlayFS, all FS have now failed, return a failure
How this used to work really depends on when we're talking about 😆. Prior to
D121423 <https://reviews.llvm.org/D121423> the FS would have looked like:
RedirectingFileSystem
/a/foo -> bar
ExternalFS
RealFileSystem
/a/bar
/b/bar
Prior to Jonas' change, `setCWD` on RedirectingFS would run `setCWD` on
`ExternalFS`. So `cd /a` would set CWD to `/a` for both RedirectingFS and
RealFileSystem. Then `cat /a/foo` would give `bar` and `openFileForRead` on
`ExternalFS` (RealFS) (which has `/a` CWD) would open `/a/bar`.
After Jonas' *and* Keith's change, `setCWD` no longer runs on `ExternalFS` but
instead the path was canonicalised before sending it along. So `/a/foo` maps to
`bar` as before, but then it would be canonicalised using `RedirectingFS` CWD
and thus do an open for `/a/bar` on `ExternalFS`.
In D121425#3384499 <https://reviews.llvm.org/D121425#3384499>, @dexonsmith
wrote:
> (Would this mean resolving everything in the `.yaml` file eagerly at launch?
> That sounds a bit scary...)
Yes, depending on what you mean by "resolving". All I mean is "prefix relative
paths with CWD". The main issue with doing this is that it prohibits
canonicalising paths to a virtual CWD, since you wouldn't be able to set CWD to
a virtual path before making the overlay. Perhaps that's what you mean by "a
bit scary" though.
We currently prefix relative paths with `ExternalContentsPrefixDir` if
`overlay-relative: true` is set. That path is a combination of CWD + the
directory to the overlay. This is to handle overlays that have absolute paths
that we want to remap (specifically for the crash dump use case I believe). But
if `overlay-relative: false` is set then we just canonicalise the path on
open/etc.
I just ran a few tests with the frontend though:
- By default, paths come in relative and will become absolute from the
process-wide CWD
- If `-working-directory` *is* set then paths come in absolute from
`FileManager` - `setCWD` is never run
I'm not sure if there's a good reason it's done this way or it's just that CWD
was only added to FileSystem relatively recently, but it does mean that that
we're actually canonicalising the paths with the process-wide CWD regardless at
the moment (at least from the frontend). This isn't the case if something was
to use the API directly, but in that case I'm not sure that using the current
CWD makes any more sense than using the CWD when the overlay is created. The
semantics would be less confusing if that was the case (IMO), but again would
prohibit canonicalising to a virtual CWD.
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
