Replaced clang-cl for MSVC Compiler on Windows - #2460
Conversation
|
Were you able to resolve the compile issue in the CI ? (i can't see the log anymore, so maybe you have to rebase and re-trigger). It would be interesting to see if we can also make |
|
I don't remember where it failed, it was either on tests or other platforms. I don't have a Windows box anymore, and the main issue (tree-operations.h) is still beyond my comprehension. I am putting AI to the task, it seems to have a reasonable fix for it. |
f7a3968 to
a5b4b19
Compare
|
Uh, I wonder what is up with the protobuf include in the context of rust that is breaking... |
That's the reason MSVC was deprecated (protocolbuffers/protobuf#20085). A newer bazel version is required to compile without error. I think we have a path forward with MSVC, the code change is minimal. However, it requires updating bazel in order to compile protobuf. The update to protobuf itself is optional, we can keep the current version if we add the right compile flag. Updating bazel should be a separate task, once it's done we can merge MSVC support. |
|
So to move to bazel 8, we need to solve the flex-bison issue #2435 I have two solutions, somewhat sitting as drafts, that should be re-activated.
So to resolve this, we might start with #2520 which allows to move to bazel8. So here might be a sequence:
|
|
Added you as reviewer of #2520 so that we can get bazel 8+ unlocked. |
I will modify the PR to only include code changes. Meanwhile, have you looked at the changes in |
|
ah, nice. I agree, the no-toolchain solution looks better. |
|
Alright, I split this PR and the other section are merged. All left is update to .bazelrc and ci to actually use msvc! |
MSVC is the default compiler on Windows and switching verible to use it would alleviate some pain points of using bazel on Windows.
For context,
clang-clwas originally used because designated identifiers are used in the codebase, which are only supported by C++20 in MSVC, which was not available at the time. Also, MSVC would crash on the codebase, which is no longer the case.I left the option to use clang-cl, but this is theory as I don't have a Windows system to verify. I removed reference to clang-cl from the doc, so this support is only for devs actively looking for it. It could be removed from .bazelrc if you prefer. As a side note, I don't think adding clang-cl to CI has value since we already build with clang on Linux.