[Lldb-commits] [PATCH] D65677: [VirtualFileSystem] Make the RedirectingFileSystem hold on to its own working directory.

[Sam McCall via Phabricator via lldb-commits]

sammccall added a subscriber: benlangmuir.
sammccall added inline comments.


================
Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1051
 std::error_code
 RedirectingFileSystem::setCurrentWorkingDirectory(const Twine &Path) {
+  // Don't change the working directory if the path doesn't exist.
----------------
This is a change in behavior, in that the WD of the underlying filesystem is no longer set, so e.g. in overlay mode RedirectingFileSystem::status() will look for non-overlaid relative paths in the wrong directory.

e.g. if we're using an overlay VFS with no files mapped and external WD is /foo, `cd /bar; stat baz` reads /bar/baz before this patch and reads /foo/baz after it. The old behavior seems more logical.

I think the right thing to do is to have setCurrentWorkingDirectory() call ExternalFS->setCurrentWorkingDirectory() if overlay mode is on. It shouldn't propagate failure, but rather record it `bool ExternalFSValidWD`, and only allow fallthrough operations when this flag is true.
This means cd into a dir which is invalid in the underlying FS would put it into a "broken" state where it's no longer consulted, but an (absolute) cd to a valid directory would fix it. (Variations are possible, depending on how you want to handle relative cd when the CWD is invalid in the underlying FS).

If you would rather just change the behavior here, you probably need to talk to someone familiar with the existing clients. It seems mostly used by clang's `-ivfsoverlay`, which was added by @benlangmuir way back when. I don't really know what it's used for.


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

https://reviews.llvm.org/D65677