[PATCH] D121423: [VFS] OverlayFileSystem should consistently use the last-added FS first

Tue Mar 15 13:06:40 PDT 2022

dexonsmith added a comment.

In D121423#3383159 <https://reviews.llvm.org/D121423#3383159>, @bnbarham wrote:

> In D121423#3380920 <https://reviews.llvm.org/D121423#3380920>, @bnbarham wrote:
>
>> If we could resolve the path in `OverlayFileSystem` then we could just do a `status` + check that it's a directory on each FS and if so, set CWD in `OverlayFileSystem` and not any underlying FS.
>>
>> Maybe we should just do that though with one exception - if the returned path is not the one we gave then *don't* replace the path...? External names is a bit of a hack anyway, so hack for a hack 😆?
>
> I went ahead and implemented this. The main problem I can see is that the `File` returned by `RealFileSystem::openFileForRead` looks like it's supposed to give the *real path* for `File::getName` (though this seems to be the case for only `RealFile`).

Do you really mean real path here (system call to `realpath`, that sees through symlinks), or just absolute path?

Is it only when `RealFileSystem` has a CWD? If `RealFileSystem` has a CWD, then in practice it needs to use absolute paths for calls to `sys::fs` since the process CWD cannot be trusted. I suspect it's just failing to model the working directory for the paths it returns (which is sometimes hard, in the presence of symlinks). Maybe that's fine for now.

Is this behaviour (overlayfilesystem's behaviour when wrapping a RealFileSystem) a regression/change or is it the same as before?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121423