-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Support proto_library targets in @bazel_tools//src/main/protobuf #28428
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
base: master
Are you sure you want to change the base?
Support proto_library targets in @bazel_tools//src/main/protobuf #28428
Conversation
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
The pull request effectively addresses the problem of unclear error messages for unsupported language-specific proto targets by replacing deprecation warnings with explicit failures. The use of a genrule to provide a descriptive error message is a significant improvement for user experience, guiding developers on how to resolve the issue.
fmeum
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.
This won't provide an immediate benefit as long as Protobuf still depends on all the language rulesets, right?
If we already require folks to migrate their usage of some of these targets, I think that it would be better to move these protos out into their own module that still only provides the proto_library targets. It could be versioned together with Bazel and Bazel could even include a nodep to bump the version of the proto module.
Yes, this is just a temporary fix that provides a better error message and makes the proto_library work again in Bazel 9. I agree moving those to their own module should be the long term solution. But we need to figure out how to setup the modue (or modules?). Do you think we need to create a new github project or we can just publish it from the Bazel repo (pointing Bazel source archive)? |
|
We could release a module from a subdirectory and could even build a dedicated release archive in CI with just that subdirectory. |
|
How would the release artifacts be published? On the release page? I'm a bit worried confusing users with normal Bazel releases. |
|
As we would release together with a regular Bazel release, it could be just one more archive attached to a Bazel release. The Publish to BCR workflow can handle the subdirectory and the source.json template can point to the particular artifact. |
|
I'm thinking about other stuff in bazel_tools, for example, the C++ launcher binary. Probably need a more comprehensive plan for this. So let's make this change for now to stop the bleed. Filed #28434 to track the bigger issue. |
This addresses #28400.
Bazel 9 breaks usage of
proto_librarytargets in@bazel_tools//src/main/protobufbecause theBUILDfile there loads rules and depends on repositories (like@grpc-java,@com_github_grpc_grpc, etc.) that are not visible from the@bazel_toolsrepository.This change introduces
src/main/protobuf/BUILD.tools, which will be used as theBUILDfile for@bazel_tools//src/main/protobuf. This version:proto_librarytargets, avoiding problematic loads and cross-repo dependencies.aliastargets for the language-specific targets (e.g.,_java_proto) that were previously defined in theBUILDfile.@bazel_toolsand users should generate their own code from theproto_library.This ensures that users can still depend on the
proto_librarytargets while providing a better experience for those attempting to use the unsupported language-specific targets.