-
Notifications
You must be signed in to change notification settings - Fork 641
Fix grpc, protoc-gen-validate and envoy_api, c-ares for Bazel 9 #6422
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix grpc, protoc-gen-validate and envoy_api, c-ares for Bazel 9 #6422
Conversation
|
Hello @mering, modules you maintain (protoc-gen-validate) have been updated in this PR. |
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.
Code Review
This pull request adds version 1.2.1.bcr.2 of the protoc-gen-validate module to fix compatibility with Bazel 9. The changes include adding a new module version with updated dependencies and a patch to fix load statements. The presubmit.yml is also updated to test against Bazel 9. The changes are generally good and follow the BCR guidelines. I have a couple of suggestions to improve the maintainability of the MODULE.bazel file by using for loops instead of list comprehensions for side effects, which is the idiomatic way in Starlark.
|
Hello @mering, modules you maintain (envoy_api, protoc-gen-validate) have been updated in this PR. |
|
Hello @veblush, @pawbhard, @eugeneo, @drfloob, @rishesh007, @markdroth, @yashykt, @apolcyn, @sergiitk, @sreenithi, @stanley-cheung, modules you maintain (grpc) have been updated in this PR. |
mering
left a comment
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.
Thanks for working on this!
| bazel: | ||
| - 7.x | ||
| - 8.x | ||
| - 9.* |
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.
Out of curiosity: What is the difference between .x and .*? Should we align on one approach?
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.
9.* fetches the latest release or release rc. 9.x only fetches the latest official release, which doesn't exist yet.
| - cfg = "host", | ||
| + cfg = "exec", |
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: I would expect protoc to be used at build time and therefore u sing host instead of exec. Can you elaborate on this change?
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.
The execution platform is not necessarily the host platform when doing remote execution. This change is needed for --incompatible_disable_starlark_host_transitions, also upstreamed at bufbuild/protoc-gen-validate@0ca681b
Require module maintainers' approval for newly pushed changes.
|
Hello @bazelbuild/bcr-maintainers, modules without existing maintainers (c-ares) have been updated in this PR. |
This PR adds
Patches upstreamed at