From 586ee89dec123775f1750ac085a35edab60ec3e0 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Wed, 22 Jan 2025 14:18:45 -0800 Subject: [PATCH] Breaking change: prohibit using Bazel+MSVC to build protobuf An opt-out flag `--define=protobuf_allow_msvc=true` will be available until our 2026 breaking release 34.0. See https://github.com/protocolbuffers/protobuf/issues/20085 for more details. #test-continuous PiperOrigin-RevId: 718525948 --- .bazelrc | 5 +++++ .github/workflows/test_cpp.yml | 12 +++++++++++- .github/workflows/test_csharp.yml | 1 + build_defs/BUILD.bazel | 26 +++++++++++++++++++++++++- ci/Windows.bazelrc | 4 ++++ examples/.bazelrc | 1 + examples/BUILD.bazel | 9 +++++++++ examples/MODULE.bazel | 5 +++++ examples/WORKSPACE | 10 ++++++++++ src/google/protobuf/stubs/BUILD.bazel | 6 ++++++ src/google/protobuf/stubs/port.h | 7 +++++++ 11 files changed, 84 insertions(+), 2 deletions(-) diff --git a/.bazelrc b/.bazelrc index c2cb371657a2d..9964276b70849 100644 --- a/.bazelrc +++ b/.bazelrc @@ -40,3 +40,8 @@ common --noenable_bzlmod build --features=layering_check common --repo_env=BAZEL_NO_APPLE_CPP_TOOLCHAIN=1 + +common --enable_platform_specific_config + +# Use clang-cl by default on Windows (see https://github.com/protocolbuffers/protobuf/issues/20085). +build:windows --extra_toolchains=@local_config_cc//:cc-toolchain-x64_windows-clang-cl --extra_execution_platforms=//build_defs:x64_windows-clang-cl --host_platform=//build_defs:x64_windows-clang-cl diff --git a/.github/workflows/test_cpp.yml b/.github/workflows/test_cpp.yml index 19a1a5149bbf2..1e9463b50da77 100644 --- a/.github/workflows/test_cpp.yml +++ b/.github/workflows/test_cpp.yml @@ -414,9 +414,17 @@ jobs: # Current github runners are all Intel based, so just build/compile # for Apple Silicon to detect issues there. bazel: build --cpu=darwin_arm64 //src/... //third_party/utf8_range/... //conformance:conformance_framework_tests - - name: Windows Bazel + - name: Windows Bazel (failing) os: windows-2022 cache_key: windows-2022-bazel7 + bazel: test //src/... @com_google_protobuf_examples//... --config=msvc-cl --test_tag_filters=-conformance --build_tag_filters=-conformance + - name: Windows Bazel + os: windows-2022 + cache_key: windows-2022-msvc-cl + bazel: test //src/... @com_google_protobuf_examples//... --config=msvc-cl --test_tag_filters=-conformance --build_tag_filters=-conformance --define=protobuf_allow_msvc=true + - name: Windows Bazel clang-cl + os: windows-2022 + cache_key: windows-2022-clang-cl bazel: test //src/... @com_google_protobuf_examples//... --test_tag_filters=-conformance --build_tag_filters=-conformance name: ${{ matrix.continuous-only && inputs.continuous-prefix || '' }} ${{ matrix.name }} runs-on: ${{ matrix.os }} @@ -528,6 +536,7 @@ jobs: if: ${{ matrix.install-flags && (!matrix.continuous-only || inputs.continuous-run) }} uses: protocolbuffers/protobuf-ci/bash@v4 with: + bazel-version: 7.1.2 credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} command: >- cmake . -DCMAKE_CXX_STANDARD=17 ${{ matrix.install-flags }} @@ -554,6 +563,7 @@ jobs: uses: protocolbuffers/protobuf-ci/bash@v4 with: credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} + bazel-version: 7.1.2 command: >- cmake . -DCMAKE_CXX_STANDARD=17 ${{ matrix.flags }} ${{ env.SCCACHE_CMAKE_FLAGS }} -Dprotobuf_ALLOW_CCACHE=ON diff --git a/.github/workflows/test_csharp.yml b/.github/workflows/test_csharp.yml index 252dcd0295140..035601e60fc3a 100644 --- a/.github/workflows/test_csharp.yml +++ b/.github/workflows/test_csharp.yml @@ -74,6 +74,7 @@ jobs: - name: Run tests uses: protocolbuffers/protobuf-ci/bash@v4 with: + bazel-version: 7.1.2 credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} command: | dotnet build csharp/src/Google.Protobuf.sln diff --git a/build_defs/BUILD.bazel b/build_defs/BUILD.bazel index 8745e1d6188a8..a35c408c085ed 100644 --- a/build_defs/BUILD.bazel +++ b/build_defs/BUILD.bazel @@ -18,12 +18,36 @@ create_compiler_config_setting( value = "msvc-cl", ) -# Caveat: clang-cl support in protobuf is only best-effort / untested for now. create_compiler_config_setting( name = "config_clang_cl", value = "clang-cl", ) +platform( + name = "x64_windows-clang-cl", + constraint_values = [ + "@platforms//cpu:x86_64", + "@platforms//os:windows", + "@bazel_tools//tools/cpp:clang-cl", + ], +) + +platform( + name = "x64_windows-msvc-cl", + constraint_values = [ + "@platforms//cpu:x86_64", + "@platforms//os:windows", + "@bazel_tools//tools/cpp:msvc", + ], +) + +config_setting( + name = "protobuf_allow_msvc", + values = { + "define": "protobuf_allow_msvc=true", + }, +) + selects.config_setting_group( name = "config_msvc", match_any = [ diff --git a/ci/Windows.bazelrc b/ci/Windows.bazelrc index 97c3ef99e240e..9341c0be10784 100644 --- a/ci/Windows.bazelrc +++ b/ci/Windows.bazelrc @@ -5,3 +5,7 @@ build --cxxopt=/std:c++17 --host_cxxopt=/std:c++17 startup --output_user_root=C:/tmp --windows_enable_symlinks common --enable_runfiles +build --extra_toolchains=@local_config_cc//:cc-toolchain-x64_windows-clang-cl +build:clang-cl --extra_execution_platforms=//build_defs:x64_windows-clang-cl --host_platform=//build_defs:x64_windows-clang-cl +build:msvc-cl --extra_execution_platforms=//build_defs:x64_windows-msvc-cl --host_platform=//build_defs:x64_windows-msvc-cl +build --config=clang-cl diff --git a/examples/.bazelrc b/examples/.bazelrc index 7873d890ec7ef..8c49fd990bf8b 100644 --- a/examples/.bazelrc +++ b/examples/.bazelrc @@ -3,6 +3,7 @@ common --enable_platform_specific_config build:linux --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 build:macos --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 +build:windows --extra_toolchains=@local_config_cc//:cc-toolchain-x64_windows-clang-cl --extra_execution_platforms=//:x64_windows-clang-cl common:windows --cxxopt=/std:c++17 --host_cxxopt=/std:c++17 --enable_runfiles build --experimental_remote_cache_eviction_retries=5 diff --git a/examples/BUILD.bazel b/examples/BUILD.bazel index 4b701c40389e0..4012b1d3f109b 100644 --- a/examples/BUILD.bazel +++ b/examples/BUILD.bazel @@ -185,3 +185,12 @@ pkg_files( strip_prefix = strip_prefix.from_root(""), visibility = ["//visibility:public"], ) + +platform( + name = "x64_windows-clang-cl", + constraint_values = [ + "@platforms//cpu:x86_64", + "@platforms//os:windows", + "@bazel_tools//tools/cpp:clang-cl", + ], +) diff --git a/examples/MODULE.bazel b/examples/MODULE.bazel index 820ea9f8cac35..518312dbdc1ee 100644 --- a/examples/MODULE.bazel +++ b/examples/MODULE.bazel @@ -7,7 +7,12 @@ local_path_override( ) bazel_dep(name = "bazel_skylib", version = "1.0.3") +bazel_dep(name = "platforms", version = "0.0.8") bazel_dep(name = "rules_cc", version = "0.0.16") bazel_dep(name = "rules_java", version = "8.3.2") bazel_dep(name = "rules_pkg", version = "0.7.0") bazel_dep(name = "rules_python", version = "0.25.0") + +# For clang-cl configuration +cc_configure = use_extension("@bazel_tools//tools/cpp:cc_configure.bzl", "cc_configure_extension") +use_repo(cc_configure, "local_config_cc") diff --git a/examples/WORKSPACE b/examples/WORKSPACE index 58c3eaeaae02e..fc8bd108b054d 100644 --- a/examples/WORKSPACE +++ b/examples/WORKSPACE @@ -28,6 +28,16 @@ local_repository( path = "..", ) +# Bazel platform rules, for clang-cl. +http_archive( + name = "platforms", + sha256 = "3a561c99e7bdbe9173aa653fd579fe849f1d8d67395780ab4770b1f381431d51", + urls = [ + "https://mirror.bazel.build/github.com/bazelbuild/platforms/releases/download/0.0.7/platforms-0.0.7.tar.gz", + "https://github.com/bazelbuild/platforms/releases/download/0.0.7/platforms-0.0.7.tar.gz", + ], +) + # Needed because protobuf_deps brings rules_python 0.26.0 which is broken: # https://github.com/bazelbuild/rules_python/issues/1543 http_archive( diff --git a/src/google/protobuf/stubs/BUILD.bazel b/src/google/protobuf/stubs/BUILD.bazel index fb044714fcebd..11958740f6e56 100644 --- a/src/google/protobuf/stubs/BUILD.bazel +++ b/src/google/protobuf/stubs/BUILD.bazel @@ -22,6 +22,12 @@ cc_library( "status_macros.h", ], copts = COPTS, + defines = ["GOOGLE_PROTOBUF_USING_BAZEL=1"] + select({ + "//build_defs:protobuf_allow_msvc": [ + "GOOGLE_PROTOBUF_MSVC_BAZEL_OVERRIDE=1", + ], + "//conditions:default": [], + }), linkopts = LINK_OPTS, strip_include_prefix = "/src", deps = [ diff --git a/src/google/protobuf/stubs/port.h b/src/google/protobuf/stubs/port.h index cd4025f6ae38a..0464a3bb59e0a 100644 --- a/src/google/protobuf/stubs/port.h +++ b/src/google/protobuf/stubs/port.h @@ -31,6 +31,13 @@ #include // IWYU pragma: export #endif +#if defined(_MSC_VER) && !defined(__clang__) && \ + defined(GOOGLE_PROTOBUF_USING_BAZEL) && \ + !defined(GOOGLE_PROTOBUF_MSVC_BAZEL_OVERRIDE) +#error \ + "Protobuf will be dropping support for MSVC + Bazel in 34.0. To continue using it until then, use the flag --define=protobuf_allow_msvc=true. See github.com/protocolbuffers/protobuf/issues/20085 for more information." +#endif + // Legacy: some users reference these (internal-only) macros even though we // don't need them any more. #if defined(_MSC_VER) && defined(PROTOBUF_USE_DLLS)