ArcsinX added a comment. In D89529#2334407 <https://reviews.llvm.org/D89529#2334407>, @kbobyrev wrote:
> Hi, thanks for flagging the issue! Could you please give more information > where exactly the tests fail (e.g. logs with assertions)? This way I could > understand where exactly the failure happens. With current implementation I could not pass Windows path (e.g. C:\a\b\c) to `Marshaller` constructor because of `assert(RemoteIndexRoot == llvm::sys::path::convert_to_slash(RemoteIndexRoot));`, because on Windows `llvm::sys::path::convert_to_slash(RemoteIndexRoot)` converts C:\a\b\c to C:/a/b/c Another problem is that Windows URI paths are not supported without this patch (e.g. URI=file:///X:/path => Body=/X:/path, and /X:/path does not start with X:\path) The third problem, that without conversion local windows paths to native slashes, mixes-slash paths appears at `path::append()` calls and we again face the problem when some paths does not start with given prefix (but it should) > I can understand what you're trying to do but the patch introduces the > behaviour which is unintended as of right now. The convention is to use POSIX > paths everywhere in the Marshalling code since the end result will be a URI > anyway. The patch changes it to Windows slashes which is orthogonal to this > direction. Idea is to use native slashes (Windows slashes on Windows) *for local paths only* and UNIX slashes inside the wire. Just to clarify why I changed `llvm::sys::path::convert_to_slash(Result, llvm::sys::path::Style::posix)` to `llvm::sys::path::convert_to_slash(Result, llvm::sys::path::Style::windows)`: - `llvm::sys::path::convert_to_slash(Result, llvm::sys::path::Style::posix)` no-op on any platform and does nothing with '\' in Windows paths. - `llvm::sys::path::convert_to_slash(Result, llvm::sys::path::Style::windows)` replaces '\' with '/' on any platform. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89529/new/ https://reviews.llvm.org/D89529 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
