[PATCH] D70701: Fix more VFS tests on Windows

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

dexonsmith added a comment.

Thanks for the quick and detailed response!

Your explanation of hybrid behaviour on Windows was especially helpful, as I didn't fully understand how that worked before.

One thing I see, but wasn't obvious from your description, is that the mixed/hybrid separator behaviour only happens for `defined(_WIN_32)`. E.g., from Path.cpp:

  bool is_separator(char value, Style style) {
    if (value == '/')
      return true;
    if (real_style(style) == Style::windows)
      return value == '\\';
    return false;
  }



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

>> - The path style is detected when reading the YAML file (as now).
>
> Which path's style?  The virtual one that's going to be redirected or the
> actual one it's redirected at?

Both, but you've mostly convinced me not to go down this route.

> - Paths in the YAML file are canonicalized to native at parse time.
>
> If we canonicalize the virtual path, VFS would no longer be valuable for
> creating platform-agnostic tests.

This is a good point I hadn't considered.

> In LLDB, we have the additional wrinkle of remote debugging, where the
> debugger may be running on a Windows machine while the program being
> debugged is running on a Linux box.  You always have to know whether a path
> will be used on the debugger host or the debuggee host.  And there are
> similar concerns for post-mortem debugging from a crash collected on a
> different type of host.

Ah, interesting.

> Honestly, I'm not sure we have a good definition of what makeAbsolute
> should do.

Perhaps the name isn't ideal -- `prependRelativePathsWithCurrentWorkingDirectory()` would be more precise -- but otherwise I'm not sure I fully agree. Regardless, I acknowledge your point that the two worlds are awkwardly different.

I'm going to think about other options; thanks again for your feedback. I am still leaning toward `FileSystem::makeAbsolute()` not being virtual / overridden, but I have a better idea of how to approach that. One idea is for the `RedirectingFileSystem` to keep track of where different styles were used when parsing.... I'm not sure if that'll pan out though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70701