-
Notifications
You must be signed in to change notification settings - Fork 16.1k
fix: cc_toolchain should prefer protoc when prebuilt flag is flipped.#25168
fix: cc_toolchain should prefer protoc when prebuilt flag is flipped.#25168thesayyn wants to merge 2 commits intoprotocolbuffers:mainfrom
Conversation
| # 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*")) |
There was a problem hiding this comment.
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
|
Can we also verify |
| cc_proto_library( | ||
| name = "cc_empty_proto", | ||
| deps = [":empty_proto"], | ||
| # We already have a anaylsis test that asserts that the cc_proto_library |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: analysis
|
@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_BROKENFor In file included from bazel-out/k8-fastbuild/bin/src/main/protobuf/command_server.pb.h:34:For |
|
@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 |
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)? |
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
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
|
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 |
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
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
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
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
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
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