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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to