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

[Keith Smiley via Phabricator via llvm-commits]

keith added inline comments.


================
Comment at: llvm/unittests/Support/VirtualFileSystemTest.cpp:1738
+
+TEST_F(VFSFromYAMLTest, RelativePathHitWithoutCWD) {
+  IntrusiveRefCntPtr<vfs::InMemoryFileSystem> BaseFS(
----------------
So this test case is actually failing. The difference between it and the others is I don't call `FS->setCurrentWorkingDirectory("//root/foo")`. This results in us (with my most recent change here) performing this logic:

1. Fetch the absolute //root/foo/vfsname
2. This results in `realname` being returned
3. We attempt to canonicalize `realname`, but we have no `pwd`, so this doesn't result in a valid path
4. everything fails past this

It seems to me, without having a ton of context here, that the value returned from the VFS lookup should actually be `//root/foo/realname`, since otherwise we could likely hit one of the same issues as those discussed above where if you actually had this situation:

- `mkdir /root/foo /root/bar`
- `touch /root/foo/realname /root/bar/realname`
- `cd /root/bar`
- lookup `/root/foo/vfsname`, get back `realname`
- canonicalize `realname` to the wrong path `/root/bar/realname`

You'd end up with the wrong file, but I don't think this is actually new behavior with my change?

@dexonsmith  wdyt?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109128