[PATCH] D113268: [clangd] Fix tests with forward slash as separator on windows

Martin Storsjö via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 5 09:07:08 PDT 2021


mstorsjo added a comment.

In D113268#3111798 <https://reviews.llvm.org/D113268#3111798>, @sammccall wrote:

> In D113268#3111709 <https://reviews.llvm.org/D113268#3111709>, @mstorsjo wrote:
>
>> FWIW, this change is not about mingw, it's about making the windows-with-forward-slashes configuration usable.
>
> OK - can I ask why the windows-with-forward-slashes configuration is valuable to support?
> I was assuming it was mostly useful to folks building with mingw, sounds like that's not all!

Well you're right that my own original need is for mingw setups, with various tools parsing and using the output of e.g. `clang -v` and `clang --print-resource-dir` etc, and using it in contexts where backslashes require extra care. I'm not familiar with all the other cases that others have had that have indicated a need for this though, but it's something that does come up semi-regularly, and as Windows itself mostly accepts this alternative form, it's a setup with possibly "less sharp corners".

>> The windows-with-forward-slashes configuration works just as fine in MSVC configurations, and that's where I've tested it. A MSVC build with `ninja check-all` that succeeded before, still succeed with `-DLLVM_ WINDOWS_PREFER_FORWARD_SLASH=ON` (with respect to clangd) after this change.
>>
>> As for testing strategy, it could be concievable to add a configuration to e.g. the buildkite premerge tests that build+test everything in a `-DLLVM_WINDOWS_PREFER_FORWARD_SLASH=ON` environment too. (I've seen interest from @rnk to push towards such a setup, possibly, so adding testing configurations for both modes is probably not inconcievable.)
>
> That's great to hear. It'd really be a prerequisite to keeping this configuration passing, as we don't have regular access to windows machines & rely on the buildbots to catch test problems.
>
> I think the biggest testing problem we have though is that we don't have a good way of defining integration tests with paths set up the right way (since we can't really use absolute paths in our tests, and most of our bugs involve comparing absolute paths).

Yup, I can totally understand that. And as I haven't tested clangd in this setup other than the unit tests, I'm not sure if it works entirely in practice though - with other unit tests in Clang I've seen lots of cases where e.g. paths aren't considered as in the same directory when paths are expressed with different separators.

For someone unfamiliar with the tool, is there a simple "smoke test procedure" I could try out with it to kick the tyres?

Anyway, I'll have a look at the seemingly odd/fragile change and get back to you on that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113268



More information about the cfe-commits mailing list