Skip to content

fix: cc_toolchain should prefer protoc when prebuilt flag is flipped.#25168

Closed
thesayyn wants to merge 2 commits intoprotocolbuffers:mainfrom
thesayyn:cc_toolchain_prebuilt
Closed

fix: cc_toolchain should prefer protoc when prebuilt flag is flipped.#25168
thesayyn wants to merge 2 commits intoprotocolbuffers:mainfrom
thesayyn:cc_toolchain_prebuilt

Conversation

@thesayyn
Copy link
Collaborator

@thesayyn thesayyn commented Jan 6, 2026

cc_toolchain now respects the prefer_prebuilt_protoc flag. When the flag is set to true, the toolchain uses the full protoc binary instead of protoc_minimal, enabling prebuilt protoc support.

Follow up to this is to have minimal protoc released as an artifact and use that.

@thesayyn thesayyn requested a review from alexeagle January 6, 2026 00:37
@thesayyn thesayyn requested a review from a team as a code owner January 6, 2026 00:37
@thesayyn thesayyn requested review from Logofile and mkruskal-google and removed request for a team January 6, 2026 00:37
@mkruskal-google mkruskal-google added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jan 6, 2026
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jan 6, 2026
@mkruskal-google mkruskal-google added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jan 6, 2026
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jan 6, 2026
# so the cc_toolchain should use the full protoc (not protoc_minimal).
# The protoc path should end with "/protoc" not contain "protoc_minimal"
action.argv().contains_predicate(matching.str_matches("*/protoc"))
action.argv().not_contains_predicate(matching.str_matches("*protoc_minimal*"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we actually do a positive check that this uses the prebuilt? Negative regex checks are kindof brittle (a typo trivially passes) and this might start failing if we switch to a prebuilt protoc_minimal

@meteorcloudy
Copy link
Contributor

Can we also verify java_proto_library and py_proto_library?

cc_proto_library(
name = "cc_empty_proto",
deps = [":empty_proto"],
# We already have a anaylsis test that asserts that the cc_proto_library
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: analysis

@Wyverald
Copy link
Contributor

Wyverald commented Jan 7, 2026

@meteorcloudy reported the following:

I'm using protobuf 33.x with changes from #25168 to build proto libraries in Bazel. Building with the following flags:

common --per_file_copt=external/.*protobuf.*/src/google/protobuf/compiler/main.cc@--THIS_CC_TOOLCHAIN_IS_BROKEN
common --host_per_file_copt=external/.*protobuf.*/src/google/protobuf/compiler/main.cc@--THIS_CC_TOOLCHAIN_IS_BROKEN
common --incompatible_enable_proto_toolchain_resolution
common --@com_google_protobuf//bazel/toolchains:prefer_prebuilt_protoc

For //src/main/protobuf:command_server_cc_proto, it failed with

In file included from bazel-out/k8-fastbuild/bin/src/main/protobuf/command_server.pb.h:34:
bazel-out/k8-fastbuild/bin/src/main/protobuf/failure_details.pb.h:16:2: error: #error "Protobuf C++ gencode is built with an incompatible version of"
   16 | #error "Protobuf C++ gencode is built with an incompatible version of"
      |  ^~~~~

For //src/main/protobuf:command_server_java_proto, it still tried to compile the protoc from source, which triggered:
gcc: error: unrecognized command-line option '--THIS_CC_TOOLCHAIN_IS_BROKEN'

@Logofile Logofile removed their request for review January 7, 2026 17:31
@alexeagle
Copy link
Collaborator

@Wyverald @meteorcloudy that's expected, the cc implementation has a hard requirement of exact version match, and there isn't a published prebuild for the HEAD pre-release version of the repo. It's the same reason @thesayyn mentions that the newly added test target is tagged manual and is expected behavior.

@Wyverald
Copy link
Contributor

Wyverald commented Jan 7, 2026

@Wyverald @meteorcloudy that's expected, the cc implementation has a hard requirement of exact version match, and there isn't a published prebuild for the HEAD pre-release version of the repo. It's the same reason @thesayyn mentions that the newly added test target is tagged manual and is expected behavior.

Ah, thanks for the context! Just for my information, where is protobuf pulling prebuilt protoc's from? I'm assuming that publishing a prebuilt protoc is part of the protobuf release process (so 33.3 would come with a prebuilt protoc)?

copybara-service bot pushed a commit that referenced this pull request Jan 7, 2026
…#25168)

cc_toolchain now respects the prefer_prebuilt_protoc flag. When the flag is set to true, the toolchain uses the full protoc binary instead of protoc_minimal, enabling prebuilt protoc support.

Follow up to this is to have minimal protoc released as an artifact and use that.

Closes #25168

COPYBARA_INTEGRATE_REVIEW=#25168 from thesayyn:cc_toolchain_prebuilt 30451be
FUTURE_COPYBARA_INTEGRATE_REVIEW=#25168 from thesayyn:cc_toolchain_prebuilt 30451be
PiperOrigin-RevId: 853356620
copybara-service bot pushed a commit that referenced this pull request Jan 7, 2026
…#25168)

cc_toolchain now respects the prefer_prebuilt_protoc flag. When the flag is set to true, the toolchain uses the full protoc binary instead of protoc_minimal, enabling prebuilt protoc support.

Follow up to this is to have minimal protoc released as an artifact and use that.

Closes #25168

COPYBARA_INTEGRATE_REVIEW=#25168 from thesayyn:cc_toolchain_prebuilt 30451be
FUTURE_COPYBARA_INTEGRATE_REVIEW=#25168 from thesayyn:cc_toolchain_prebuilt 30451be
PiperOrigin-RevId: 853356620
@alexeagle
Copy link
Collaborator

https://github.com/protocolbuffers/protobuf/pull/24115/changes#diff-b4f2e9f64ea668b0d49eb76f523b1e449fe7add0a23ada273e93552769d5bf02 is a good place to look. Yes, the release process swaps out this file with the tagged version and integrity hashes of the binaries, prior to that we can only fake the version

copybara-service bot pushed a commit that referenced this pull request Jan 7, 2026
…#25168)

cc_toolchain now respects the prefer_prebuilt_protoc flag. When the flag is set to true, the toolchain uses the full protoc binary instead of protoc_minimal, enabling prebuilt protoc support.

Follow up to this is to have minimal protoc released as an artifact and use that.

Closes #25168

COPYBARA_INTEGRATE_REVIEW=#25168 from thesayyn:cc_toolchain_prebuilt 30451be
FUTURE_COPYBARA_INTEGRATE_REVIEW=#25168 from thesayyn:cc_toolchain_prebuilt 30451be
PiperOrigin-RevId: 853356620
copybara-service bot pushed a commit that referenced this pull request Jan 7, 2026
…#25168)

cc_toolchain now respects the prefer_prebuilt_protoc flag. When the flag is set to true, the toolchain uses the full protoc binary instead of protoc_minimal, enabling prebuilt protoc support.

Follow up to this is to have minimal protoc released as an artifact and use that.

Closes #25168

COPYBARA_INTEGRATE_REVIEW=#25168 from thesayyn:cc_toolchain_prebuilt 30451be
FUTURE_COPYBARA_INTEGRATE_REVIEW=#25168 from thesayyn:cc_toolchain_prebuilt 30451be
PiperOrigin-RevId: 853356620
copybara-service bot pushed a commit that referenced this pull request Jan 7, 2026
…#25168)

cc_toolchain now respects the prefer_prebuilt_protoc flag. When the flag is set to true, the toolchain uses the full protoc binary instead of protoc_minimal, enabling prebuilt protoc support.

Follow up to this is to have minimal protoc released as an artifact and use that.

Closes #25168

COPYBARA_INTEGRATE_REVIEW=#25168 from thesayyn:cc_toolchain_prebuilt 30451be
FUTURE_COPYBARA_INTEGRATE_REVIEW=#25168 from thesayyn:cc_toolchain_prebuilt 30451be
PiperOrigin-RevId: 853356620
copybara-service bot pushed a commit that referenced this pull request Jan 8, 2026
…#25168)

cc_toolchain now respects the prefer_prebuilt_protoc flag. When the flag is set to true, the toolchain uses the full protoc binary instead of protoc_minimal, enabling prebuilt protoc support.

Follow up to this is to have minimal protoc released as an artifact and use that.

Closes #25168

COPYBARA_INTEGRATE_REVIEW=#25168 from thesayyn:cc_toolchain_prebuilt 30451be
FUTURE_COPYBARA_INTEGRATE_REVIEW=#25168 from thesayyn:cc_toolchain_prebuilt 30451be
PiperOrigin-RevId: 853356620
copybara-service bot pushed a commit that referenced this pull request Jan 8, 2026
…#25168)

cc_toolchain now respects the prefer_prebuilt_protoc flag. When the flag is set to true, the toolchain uses the full protoc binary instead of protoc_minimal, enabling prebuilt protoc support.

Follow up to this is to have minimal protoc released as an artifact and use that.

Closes #25168

COPYBARA_INTEGRATE_REVIEW=#25168 from thesayyn:cc_toolchain_prebuilt 30451be
FUTURE_COPYBARA_INTEGRATE_REVIEW=#25168 from thesayyn:cc_toolchain_prebuilt 30451be
PiperOrigin-RevId: 853356620
@copybara-service copybara-service bot closed this in 8c857c3 Jan 8, 2026
karenwuz pushed a commit that referenced this pull request Jan 8, 2026
…#25168)

cc_toolchain now respects the prefer_prebuilt_protoc flag. When the flag is set to true, the toolchain uses the full protoc binary instead of protoc_minimal, enabling prebuilt protoc support.

Follow up to this is to have minimal protoc released as an artifact and use that.

Closes #25168

COPYBARA_INTEGRATE_REVIEW=#25168 from thesayyn:cc_toolchain_prebuilt 30451be
PiperOrigin-RevId: 853463775
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants