-
Notifications
You must be signed in to change notification settings - Fork 479
Added rust_prost_transform
rule for modifying granular proto_library
#3083
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
Conversation
ddd03ca
to
576332b
Compare
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 seems really handy!
The placement on the proto_library
is interesting compared to other languages... It feels like in other languages, each proto_library
would have a lang_proto_library
target wrapping it, and that that would be an obvious place to put this data. Whereas we don't expect one prost_proto_library
per proto_library
but instead glom together a bunch of proto_library
targets into one prost_proto_library
, which means any attributes on that are a bit disconnected from the .proto
files they're applying to...
I'd be slightly tempted to say we should add this as a dict[ProtoLabel, TransformLabel]
to prost_proto_library
rather than adding it to the proto_library
itself (using data
for this feels... Convenient, but wrong?)
But also, at the end of the day this seems useful and fine, so... Feel free to merge, but would be interested in your thoughts...
We discussed this offline but for posterity, the way the rules work is by using an aspect to perform the protogen and rustc compile on the I think this is something that would be easy to phase out if a better solution comes along but I haven't seen one yet. Pull-requests welcome! |
You should be able to just use |
I’m happy to take a look at other pull requests that can provide the same functionality in a cleaner way 😄 |
This new rule is to support granular modifications to existing
proto_library
targets without adding duplicate actions to the build graph depending on whatrust_prost_library
target was consumed.closes #2045