fix: upgrade Protobuf and gRPC in WORKSPACE#17882
Conversation
ef2531f to
1300086
Compare
There was a problem hiding this comment.
Code Review
This pull request upgrades Bazel to version 7.7.1, restructures the WORKSPACE to load modern dependencies first (including Protobuf 35.1, rules_python v1.1.0, and gRPC), and introduces a compatibility wrapper rule in py_gapic.bzl to bridge legacy CustomProtoInfo with modern ProtoInfo. Feedback highlights two critical improvements in py_gapic.bzl: first, the compatibility wrapper should forward the original ProtoInfo provider alongside CustomProtoInfo to prevent downstream resolution failures; second, the testonly attribute must be propagated to the dynamically generated compatibility targets to avoid Bazel dependency analysis errors.
e7f6983 to
299b32e
Compare
299b32e to
1cb12cf
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the Bazel build configuration, upgrading several dependencies including Bazel (to 7.7.1), rules_python (to 1.1.0), and gRPC (to 1.83.0). It also introduces a compatibility adapter rule (gapic_compat_proto_library) to bridge modern Starlark ProtoInfo with rules_gapic's CustomProtoInfo. Feedback on these changes highlights three key issues: first, the temporary gRPC patch applied in the main WORKSPACE is missing from repositories.bzl, which will break downstream builds; second, defaulting testonly to None instead of False in the compatibility adapter can trigger Starlark evaluation errors; and third, using python3 -c in patch_cmds compromises Bazel's hermeticity and cross-platform compatibility, suggesting the use of a standard patch file instead.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request upgrades Bazel to version 7.7.1 and updates several key dependencies in the WORKSPACE file, including Protobuf, rules_python, rules_cc, bazel_skylib, and gRPC. It also registers multi-version hermetic Python toolchains and introduces a compatibility adapter rule (gapic_compat_proto_library) in py_gapic.bzl to bridge modern rules_proto Starlark ProtoInfo with rules_gapic's CustomProtoInfo. A critical issue was identified in the adapter implementation where the CustomProtoInfo provider is instantiated with a non-existent transitive_descriptor_sets field, which will cause a Bazel analysis-phase error.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request upgrades Bazel to version 7.7.1 and updates several dependencies in the WORKSPACE file, including Protobuf, rules_python, rules_cc, and gRPC. It also introduces a compatibility adapter rule gapic_compat_proto_library in py_gapic.bzl to bridge modern rules_proto Starlark ProtoInfo with rules_gapic's expected CustomProtoInfo provider. The review feedback suggests forwarding the DefaultInfo provider in the compatibility adapter to preserve default outputs and runfiles, and recommends using Label() instead of a hardcoded workspace name when referencing the gRPC patch file in repositories.bzl.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request upgrades Bazel to 7.7.1 and updates several dependencies, including Protobuf to 35.1, rules_python to 1.1.0, and gRPC to 1.83.0 (with a temporary patch). It also introduces a compatibility adapter rule, gapic_compat_proto_library, to bridge modern Starlark ProtoInfo with rules_gapic's expected CustomProtoInfo. The feedback suggests propagating the tags attribute to the dynamically created helper targets in the py_gapic_library macro to ensure wildcard builds behave correctly.
ignore_root_user_error = Truefor Github Actions compatibility.gapic_compat_proto_libraryinpy_gapic.bzlto bridge modernrules_protoStarlark ProtoInfo with rules_gapic'sCustomProtoInfo.