[PATCH] D70701: Fix more VFS tests on Windows

dexonsmith added a comment. In 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.

Re: [PATCH] D70701: Fix more VFS tests on Windows

BTW, I hope I didn't come across as overly negative in my previous response. I'd love to see the situation improved. I just don't know what that would look like. > One thing I see, but wasn't obvious from your description, is that the mixed/hybrid separator behaviour only happens for `defined(_W

[PATCH] D70701: Fix more VFS tests on Windows

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 beh

Re: [PATCH] D70701: Fix more VFS tests on Windows

I didn't design VFS. I just fixed a bunch of portability bugs that prevented us from running most of the VFS tests on Windows. If I were designing it, I (hope) I wouldn't have done it this way. But the horse is already out of the barn. Here's my background with VFS (and, specifically, the redi

[PATCH] D70701: Fix more VFS tests on Windows

dexonsmith added a comment. @rnk / @amccarth, I've been looking at the history of `makeAbsolute` being virtual, since it's a bit awkward that `RedirectingFileSystem` behaves differently from the others. I'm hoping you can give me a bit more context. I'm wondering about an alternative implementa

[PATCH] D70701: Fix more VFS tests on Windows

This revision was automatically updated to reflect the committed changes. amccarth marked an inline comment as done. Closed by commit rG738b5c9639b4: Fix more VFS tests on Windows (authored by amccarth). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.org/D70701?vs=

[PATCH] D70701: Fix more VFS tests on Windows

rnk accepted this revision. rnk added a subscriber: JDevlieghere. rnk added a comment. This revision is now accepted and ready to land. +@JDevlieghere, due to recent VFS test fixup. I think this looks good, and in the absence of input from other VFS stakeholders, let's land this soon. Thanks!

[PATCH] D70701: Fix more VFS tests on Windows

amccarth marked 2 inline comments as done. amccarth added inline comments. Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1431 -if (IsRootEntry && !sys::path::is_absolute(Name)) { - assert(NameValueNode && "Name presence should be checked earlier"); ---

[PATCH] D70701: Fix more VFS tests on Windows

rnk added inline comments. Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1077-1078 +std::error_code RedirectingFileSystem::makeAbsolute(SmallVectorImpl &Path) const { + if (llvm::sys::path::is_absolute(Path, llvm::sys::path::Style::posix) || + llvm::sys::path::is_abso

[PATCH] D70701: Fix more VFS tests on Windows

amccarth marked 2 inline comments as done. amccarth added a comment. A (hopefully) more cogent response than my last round. I'm still hoping to hear from VFS owners. Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1077-1078 +std::error_code RedirectingFileSystem::makeAbsol

[PATCH] D70701: Fix more VFS tests on Windows

amccarth marked 3 inline comments as done. amccarth added inline comments. Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1077-1078 +std::error_code RedirectingFileSystem::makeAbsolute(SmallVectorImpl &Path) const { + if (llvm::sys::path::is_absolute(Path, llvm::sys::path::

[PATCH] D70701: Fix more VFS tests on Windows

rnk added inline comments. Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1077-1078 +std::error_code RedirectingFileSystem::makeAbsolute(SmallVectorImpl &Path) const { + if (llvm::sys::path::is_absolute(Path, llvm::sys::path::Style::posix) || + llvm::sys::path::is_abso

[PATCH] D70701: Fix more VFS tests on Windows

amccarth created this revision. amccarth added reviewers: rnk, vsapsai, arphaman, Bigcheese. Herald added subscribers: dexonsmith, hiraditya. Herald added a project: LLVM. Since VFS paths can be in either Posix or Windows style, we have to use a more flexible definition of "absolute" path. The k