[PATCH] D70701: Fix more VFS tests on Windows

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D70701#2515934 <https://reviews.llvm.org/D70701#2515934>, @amccarth wrote:

> BTW, I hope I didn't come across as overly negative in my previous
> response.

No, not at all!

> [...] On Windows, a process can have multiple
> current directories:  one for each drive.  And a process has one current
> drive.  So the current working directory is the current directory for the
> current drive.  A Windows path like "D:foo.txt" is a relative path.

Also news to me; thanks for the extra info. Looks like this isn't just a problem with `makeAbsolute`; even `sys::fs::make_absolute` is incorrect for Windows.

Seems to me like we could:

- Fix the one argument version of `sys::fs::make_absolute` on Windows by calling GetFullPathNameW (documented at https://en.cppreference.com/w/cpp/filesystem/absolute)
- "Fix" the two argument version of `sys::fs::make_absolute` by returning an error if the path and CWD both have drive names and they don't match. (Or leave it alone.)

For the VFS, I imagine we could:

- Keep an enum on `FileSystem` that indicates its path style (immutable, set on construction).
- Maybe change APIs to support the `windows` idea of a different CWD per drive (`getCurrentRootDirectoryFor(...)`), in order to have a correct implementation of the one-argument `makeAbsolute` (I assume there's way to get the full initial set to support `getPhysicalFileSystem()`?).
- Split `RedirectingFileSystem` implementation in two, between the "redirecting" and the "overlay" parts (overlaying being optional, depending on `fallthrough:`).
- Add a `WindowsFileSystemAsPOSIX` adaptor/proxy that takes an underlying filesystem in `windows` mode and presents it as-if `posix`, given a drive mapping in the constructor. This could be implemented using the same guts as the "redirecting" part of `RedirectingFileSystem`. (Note also https://reviews.llvm.org/D94844, which allows a directory to be remapped/redirected wholesale; this could be used for mapping drives to POSIX paths.)
- Add a `POSIXFileSystemAsWindows` adaptor/proxy that does the inverse.
- Disallow overlaying `FileSystem`s that have mismatched path styles. E.g., `OverlayFileSystem::pushOverlay` would error if a mismatched style is passed into it, requiring the caller to first wrap it in an adaptor.
- Maybe add `make{Native,Windows,POSIX}FileSystem` adaptors which return the given filesystem directly or add the appropriate adaptor (I'm not sure how drive mapping would be handled... maybe there are sane default mappings we could use...).
- Maybe make `getVFSFromYAML` add an adaptor to native by default.

I suppose I'm imagining windows mode would remain "hybrid", but I wonder if this would model the two worlds, even in the face of a VFS using different path styles than `native`. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70701