bnbarham added a comment. There's two failing tests with this change:
- VFSFromYAMLTest.ReturnsExternalPathVFSHit - VFSFromYAMLTest.ReturnsInternalPathVFSHit Apparently we allow relative paths in external-contents *without* specifying `overlay-relative: true`. In this case the relative paths are resolved based on the CWD at the time of the operation. Do we really want that? I believe this wouldn't have worked prior to https://reviews.llvm.org/D109128 as there was no canonicalising before then as well. @keith was that change intentional? If we want it then it will *require* setting CWD on each FS in OverlayFS, which I was really hoping to avoid. To give the concrete case: OverlayFileSystem RedirectingFileSystem /a/foo -> bar RealFileSystem /a/bar /b/bar cd /a cat foo # /a/bar cd /b # would fail previously since it doesn't exist in RedirectingFS but would still setCWD for RealFS cat foo # and thus return /a/bar since RedirectingFS is still in /a This now fails completely because OverlayFS doesn't setCWD on the underlying filesystems. So RedirectingFS has the CWD of whatever ExternalFS was at creation. In D121424 <https://reviews.llvm.org/D121424> it will fail because there's no longer any canonicalise after mapping the path in RedirectingFS (I assumed this was unintentional and missed the test case). If we want to allow this then I think we'll need to go with the previous plan for OverlayFS, ie. keep track of filesystems that setCWD failed on and don't call them at all until they succeed again. That then doesn't allow certain cases that the current method allows. 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
