[PATCH] D109128: [VFS] Use original path when falling back to external FS

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 14 14:18:35 PDT 2021


dexonsmith added a comment.

In D109128#3000374 <https://reviews.llvm.org/D109128#3000374>, @vsapsai wrote:

> In D109128#2999846 <https://reviews.llvm.org/D109128#2999846>, @dexonsmith wrote:
>
>> In D109128#2997588 <https://reviews.llvm.org/D109128#2997588>, @JDevlieghere wrote:
>>
>>> Keith and I discussed this offline. My suggestion was to do the following:
>>>
>>> 1. Check the overlay for the canonicalized path
>>> 2. Check the fall-through for the canonicalized path
>>> 3. Check the fall-through for the original path
>>
>> I'm not sure it's correct to do (3) at all...
>
> I might have missed that but what use case are we addressing by avoiding the usage of the original path for the fall-through file system?

See rG0be9ca7c0f9a733f846bb6bc4e8e36d46b518162 <https://reviews.llvm.org/rG0be9ca7c0f9a733f846bb6bc4e8e36d46b518162>.

- RedirectingFileSystem has a virtual working directory.
- Prior to that commit, the external FS's working directory and RedirectingFileSystem's could get out of sync.
- Starting with that commit, RedirectingFileSystem consistently applies the virtual working directory.

> Because we have not only a problem with names reported back but also canonicalization can break symlinks with relative paths.

Maybe this should be using `makeAbsolute` instead of `makeCanonical` (the latter calls the former)? (Note BTW that the absolute path logic is all pretty iffy on Windows, since each drive should have its own shadow virtual current directory, but that's got to be outside the scope here...)

In terms of canonicalization and symlinks, I think you're right that `remove_dot_dot=true` isn't really safe to use.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109128



More information about the llvm-commits mailing list