rules_proto_grpc icon indicating copy to clipboard operation
rules_proto_grpc copied to clipboard

Make it possible to combine/add plugins

Open rfevang opened this issue 2 years ago • 1 comments

Description

I'm trying to run a custom protoc plugin that modifies the output of a different (Java) plugin, using insertion points.

I first tried looking for an argument to java_proto_library or java_proto_compile that let me add my plugin. When I didn't see any I then tried making my own compile rule, specifying both plugins. This attempt was however met by errors saying that the insertion points were not found.

I looked at the source and spotted this:

# Each plugin is isolated to its own execution of protoc, as plugins may have differing
# exclusions that cannot be expressed in a single protoc execution for all plugins.

for plugin in plugins:

As far as I can tell this effectively makes it impossible to use insertion point based protoc plugins with rules-proto-grpc.

Is it possible to change this? It seems like the low level function should support multiple plugins in one call, and then leave it up to higher level functions to split plugins into multiple calls to the low level compile, if that is desired.

Ideally I'd like to see an argument to the high level functions where one could pass insertion point based plugins.

rfevang avatar Dec 29 '22 03:12 rfevang

Hi, do you have an example of such a plugin that depends on the outputs of prior plugins? I wasn't even aware these exist and would need one to test with.

Passing plugins at the top level used to be impossible to support due to the architecture of these rules, but actually since 4.0.0 dropped transitivity it may be feasible to implement a generic_compile rule that takes the plugin list (this already mostly exists in proto_compile_impl). There may be some constraint I am forgetting though...

As for running in a shared execution, that is supported by protoc just fine but the comment you copied about exclusions remains a concern. If the plugins share the same exclusions (which most do) then perhaps the execution could be merged :thinking:

aaliddell avatar Mar 05 '23 21:03 aaliddell