PR #26477 broke Stardoc on Windows with MSVC - now failing with 'Protobuf only supports C++17 and newer.'

[tetromino]

It appears that something proto-related in #26477 broke Stardoc on Windows. It now fails with error C2338: static_assert failed: 'Protobuf only supports C++17 and newer.'

Originally observed on Buildkite - https://buildkite.com/bazel/stardoc/builds/2123#0197f5bb-3dcb-4d21-a1ba-1331895b634c - and can be reproduced locally:

Fails with Bazel at e6979d3:

arostovtsev@WINDOWS-PLAYGRO C:\Users\arostovtsev\bazelbuild\stardoc>set USE_BAZEL_VERSION=e6979d3f878ddd663f15a10b09407ecbb9380fee

arostovtsev@WINDOWS-PLAYGRO C:\Users\arostovtsev\bazelbuild\stardoc>bazel --output_user_root=C:/b build //...
Starting local Bazel server (no_version) and connecting to it...
WARNING: For repository 'com_google_protobuf', the root module requires module version protobuf@29.0, but got protobuf@31.1 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'rules_java', the root module requires module version rules_java@8.6.1, but got rules_java@8.13.0 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'rules_jvm_external', the root module requires module version rules_jvm_external@6.6, but got rules_jvm_external@6.7 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: C:/users/arostovtsev/bazelbuild/stardoc/test/BUILD:428:10: in _diff_test rule //test:stardoc_maven_artifacts_consistency_test: target '//test:stardoc_maven_artifacts_consistency_test' depends on deprecated target '@@bazel_tools//src/conditions:host_windows_x64_constraint': No longer used by Bazel and will be removed in the future. Migrate to toolchains or define your own version of this setting.
WARNING: C:/users/arostovtsev/bazelbuild/stardoc/test/BUILD:451:10: in _diff_test rule //test:stardoc_version_consistency_test: target '//test:stardoc_version_consistency_test' depends on deprecated target '@@bazel_tools//src/conditions:host_windows_x64_constraint': No longer used by Bazel and will be removed in the future. Migrate to toolchains or define your own version of this setting.
INFO: From Compiling third_party/utf8_range/utf8_range.c [for tool]:
external/protobuf+/third_party/utf8_range/utf8_range.c(32): warning C4141: 'inline': used more than once
external/protobuf+/third_party/utf8_range/utf8_range.c(39): warning C4141: 'inline': used more than once
external/protobuf+/third_party/utf8_range/utf8_range.c(43): warning C4141: 'inline': used more than once
external/protobuf+/third_party/utf8_range/utf8_range.c(178): warning C4141: 'inline': used more than once
INFO: Analyzed 209 targets (206 packages loaded, 7715 targets and 2 aspects configured).
INFO: From Compiling upb/mem/arena.c [for tool]:
external/protobuf+/upb/mem/arena.c(226): warning C4047: '=': 'uintptr_t' differs in levels of indirection from 'void *'
external/protobuf+/upb/mem/arena.c(231): warning C4047: '=': 'uintptr_t' differs in levels of indirection from 'void *'
external/protobuf+/upb/mem/arena.c(216): warning C4047: 'initializing': 'uintptr_t' differs in levels of indirection from 'void *'
external/protobuf+/upb/mem/arena.c(271): warning C4047: '=': 'uintptr_t' differs in levels of indirection from 'void *'
external/protobuf+/upb/mem/arena.c(257): warning C4047: 'initializing': 'uintptr_t' differs in levels of indirection from 'void *'
external/protobuf+/upb/mem/arena.c(268): warning C4047: 'initializing': 'uintptr_t' differs in levels of indirection from 'void *'
external/protobuf+/upb/mem/arena.c(281): warning C4047: 'initializing': 'uintptr_t' differs in levels of indirection from 'void *'
external/protobuf+/upb/mem/arena.c(333): warning C4047: 'initializing': 'size_t' differs in levels of indirection from 'void *'
external/protobuf+/upb/mem/arena.c(381): warning C4047: 'initializing': 'uintptr_t' differs in levels of indirection from 'void *'
external/protobuf+/upb/mem/arena.c(512): warning C4047: '=': 'uintptr_t' differs in levels of indirection from 'void *'
external/protobuf+/upb/mem/arena.c(507): warning C4047: 'initializing': 'uintptr_t' differs in levels of indirection from 'void *'
external/protobuf+/upb/mem/arena.c(549): warning C4047: 'initializing': 'uintptr_t' differs in levels of indirection from 'void *'
external/protobuf+/upb/mem/arena.c(600): warning C4047: 'initializing': 'uintptr_t' differs in levels of indirection from 'void *'
external/protobuf+/upb/mem/arena.c(609): warning C4047: 'initializing': 'uintptr_t' differs in levels of indirection from 'void *'
external/protobuf+/upb/mem/arena.c(732): warning C4047: 'initializing': 'uintptr_t' differs in levels of indirection from 'void *'
INFO: From Compiling upb/mini_descriptor/decode.c [for tool]:
external/protobuf+/upb/mini_descriptor/decode.c(312): warning C4116: unnamed type definition in parentheses
ERROR: C:/b/cghxau54/external/protobuf+/src/google/protobuf/compiler/BUILD.bazel:128:11: Compiling src/google/protobuf/compiler/code_generator_lite.cc [for tool] failed: (Exit 2): cl.exe failed: error executing CppCompile command (from cc_library rule target @@protobuf+//src/google/protobuf/compiler:code_generator_lite) C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.39.33519\bin\HostX64\x64\cl.exe ... (remaining 1 argument skipped)
bazel-out/x64_windows-opt-exec-ST-d57f47055a04/bin/external/protobuf+/src/google/protobuf/_virtual_includes/port\google/protobuf/port_def.inc(119): error C2338: static_assert failed: 'Protobuf only supports C++17 and newer.'
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 30.697s, Critical Path: 5.52s
INFO: 424 processes: 563 action cache hit, 187 internal, 237 local.
ERROR: Build did NOT complete successfully

Builds fine with Bazel at previous commit, cf54cca:

arostovtsev@WINDOWS-PLAYGRO C:\Users\arostovtsev\bazelbuild\stardoc>set USE_BAZEL_VERSION=cf54cca5e589effc289066b4f87bcaecdf1da482   

arostovtsev@WINDOWS-PLAYGRO C:\Users\arostovtsev\bazelbuild\stardoc>bazel --output_user_root=C:/b build //...
Extracting Bazel installation...
Starting local Bazel server (no_version) and connecting to it...
WARNING: For repository 'rules_java', the root module requires module version rules_java@8.6.1, but got rules_java@8.12.0 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: C:/users/arostovtsev/bazelbuild/stardoc/test/BUILD:451:10: in _diff_test rule //test:stardoc_version_consistency_test: target '//test:stardoc_version_consistency_test' depends on deprecated target '@@bazel_tools//src/conditions:host_windows_x64_constraint': No longer used by Bazel and will be removed in the future. Migrate to toolchains or define your own version of this setting.
WARNING: C:/users/arostovtsev/bazelbuild/stardoc/test/BUILD:428:10: in _diff_test rule //test:stardoc_maven_artifacts_consistency_test: target '//test:stardoc_maven_artifacts_consistency_test' depends on deprecated target '@@bazel_tools//src/conditions:host_windows_x64_constraint': No longer used by Bazel and will be removed in the future. Migrate to toolchains or define your own version of this setting.
INFO: Analyzed 209 targets (194 packages loaded, 7429 targets and 2 aspects configured).
INFO: From Compiling third_party/utf8_range/utf8_range.c [for tool]:
external/protobuf+/third_party/utf8_range/utf8_range.c(38): warning C4141: 'inline': used more than once
external/protobuf+/third_party/utf8_range/utf8_range.c(45): warning C4141: 'inline': used more than once
external/protobuf+/third_party/utf8_range/utf8_range.c(49): warning C4141: 'inline': used more than once
external/protobuf+/third_party/utf8_range/utf8_range.c(178): warning C4141: 'inline': used more than once
[... log lines omitted for brevity ...]
INFO: Found 209 targets...
INFO: Elapsed time: 109.845s, Critical Path: 38.52s
INFO: 851 processes: 623 action cache hit, 147 internal, 688 local, 16 worker.
INFO: Build completed successfully, 851 total actions

CC-ing @comius and @Wyverald in case they have suggestions.

[tetromino]

Possibly related: protocolbuffers/protobuf#20085 - Protobuf is dropping support for MSVC + Bazel.

[tetromino]

I can build Stardoc successfully with Bazel at e6979d3 by setting --host_cxxopt=/std:c++17 --define=protobuf_allow_msvc=true build flags - but simply adding those flags to Stardoc CI is not a solution because our transitive deps would still be broken...

[tetromino]

Honestly not sure what the right thing to do here is. Do we switch Bazel on Windows by default to use clang instead of msvc?

[meteorcloudy]

@tetromino Yes, protobuf dropping MSVC support on Windows would be pretty bad, I'm trying to prevent that from happening. The main motivation was to avoid long path issue on windows, which could be mitigated by #26005, I'll try to get that change land asap. In the meantime, please set --define=protobuf_allow_msvc=true to workaround this.

FYI @mkruskal-google, once I get #26005 land, can we make a patch release of protobuf to remove the check, since e6979d3, protobuf build will be broken for all Windows users using MSVC with Bazel@HEAD.

[tetromino]

@meteorcloudy - note that --define=protobuf_allow_msvc=true is not sufficient, protobuf now also needs --host_cxxopt=/std:c++17 when using msvc.