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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
