Skip to content

Commit

Permalink
Breaking change: prohibit using Bazel+MSVC to build protobuf
Browse files Browse the repository at this point in the history
An opt-out flag `--define=protobuf_allow_msvc=true` will be available until our 2026 breaking release 34.0. See #20085 for more details.

#test-continuous

PiperOrigin-RevId: 720822739
  • Loading branch information
mkruskal-google authored and copybara-github committed Jan 29, 2025
1 parent 4503547 commit 117e7bb
Show file tree
Hide file tree
Showing 12 changed files with 93 additions and 3 deletions.
5 changes: 5 additions & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion .github/workflows/test_bazel.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,4 @@ jobs:
credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
bazel-cache: examples
version: ${{ matrix.bazelversion }}
bash: cd examples && bazel build //... $BAZEL_FLAGS --subcommands --verbose_failures --enable_bzlmod=${{ matrix.bzlmod }} --enable_workspace=${{ !matrix.bzlmod }} ${{ matrix.toolchain_resolution }}
bash: cd examples && bazel build //... $BAZEL_FLAGS --verbose_failures --enable_bzlmod=${{ matrix.bzlmod }} --enable_workspace=${{ !matrix.bzlmod }} ${{ matrix.toolchain_resolution }}
8 changes: 7 additions & 1 deletion .github/workflows/test_cpp.yml
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,11 @@ jobs:
bazel: build --cpu=darwin_arm64 //src/... //third_party/utf8_range/... //conformance:conformance_framework_tests
- name: Windows Bazel
os: windows-2022
cache_key: windows-2022-bazel7
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 }}
Expand Down Expand Up @@ -524,6 +528,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 }}
Expand All @@ -550,6 +555,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
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/test_csharp.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 25 additions & 1 deletion build_defs/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
4 changes: 4 additions & 0 deletions ci/Windows.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions examples/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions examples/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -185,3 +185,16 @@ 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",
# This is necessary for Bazel 7 compatibility with a MODULE.bazel file that still works in
# Bazel 8. Using cc_configure_extension from rules_cc produces a @local_config_cc
# repository that's not compatible with @bazel_tools//tools/cpp:clang-cl from before
# Bazel 8. See https://github.com/bazelbuild/rules_cc/issues/330.
"@rules_cc//cc/private/toolchain:clang-cl",
],
)
5 changes: 5 additions & 0 deletions examples/MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@ local_path_override(
)

bazel_dep(name = "bazel_skylib", version = "1.7.1")
bazel_dep(name = "platforms", version = "0.0.10")
bazel_dep(name = "rules_cc", version = "0.0.17")
bazel_dep(name = "rules_java", version = "8.6.1")
bazel_dep(name = "rules_pkg", version = "1.0.1")
bazel_dep(name = "rules_python", version = "1.0.0")

# For clang-cl configuration
cc_configure = use_extension("@rules_cc//cc:extensions.bzl", "cc_configure_extension")
use_repo(cc_configure, "local_config_cc")
18 changes: 18 additions & 0 deletions examples/WORKSPACE
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
workspace(name = "com_google_protobuf_examples")

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

# This protobuf repository is required for proto_library rule.
# It provides the protocol compiler binary (i.e., protoc).
#
Expand All @@ -26,6 +28,16 @@ local_repository(
path = "..",
)

# Bazel platform rules, for clang-cl.
http_archive(
name = "platforms",
sha256 = "218efe8ee736d26a3572663b374a253c012b716d8af0c07e842e82f238a0a7ee",
urls = [
"https://mirror.bazel.build/github.com/bazelbuild/platforms/releases/download/0.0.10/platforms-0.0.10.tar.gz",
"https://github.com/bazelbuild/platforms/releases/download/0.0.10/platforms-0.0.10.tar.gz",
],
)

load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")

protobuf_deps()
Expand All @@ -41,3 +53,9 @@ rules_java_toolchains()
load("@rules_python//python:repositories.bzl", "py_repositories")

py_repositories()

load("@rules_cc//cc:repositories.bzl", "rules_cc_dependencies", "rules_cc_toolchains")

rules_cc_dependencies()

rules_cc_toolchains()
6 changes: 6 additions & 0 deletions src/google/protobuf/stubs/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
7 changes: 7 additions & 0 deletions src/google/protobuf/stubs/port.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@
#include <byteswap.h> // 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. For feedback or discussion, see github.com/protocolbuffers/protobuf/issues/20085."
#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)
Expand Down

0 comments on commit 117e7bb

Please sign in to comment.