uber / prototool

Your Swiss Army Knife for Protocol Buffers
MIT License
5.04k stars 345 forks source link

Well-Known Plugins/Plugin Groups #2

Closed bufdev closed 5 years ago

bufdev commented 6 years ago

In Protoeasy, I took the tact that Protoeasy should know the entire world, and it will only handle known plugins. This worked well for basic usage, but proved too limiting, as I eventually had to add the --extra-plugin flag, and keeping up to date with all handled plugins was not scalable in any sense of the word. In prototyping Prototool, I took the compete opposite tact, that I like to think of as the "dependency injection approach", where Prototool is not aware of any plugins, instead leaving you to specify what to compile and where to output it. Through discussions with the Protobuf team at Google, it's become apparent that a middle-ground approach is desired, where we do handle "well-known plugins" in a consistent manner, but leave room to "break glass" if necessary.

The current setup requires something like this:

gen:
  go_options:
    import_path: github.com/uber/prototool/example/idl/uber
    extra_modifiers:
      google/api/annotations.proto: google.golang.org/genproto/googleapis/api/annotations
      google/api/http.proto: google.golang.org/genproto/googleapis/api/annotations
  plugins:
    - name: gogoslick
      type: gogo
      flags: plugins=grpc
      output: ../../gen/proto/go
    - name: yarpc-go
      type: gogo
      output: ../../gen/proto/go
    - name: java
      output: ../../gen/proto/java
    - name: grpc-gateway
      type: go
      output: ../../gen/proto/go

This will output stubs for github.com/gogo/protobuf including grpc-go stubs, github.com/yarpc/yarpc-go, java (the built-in "plugin" for java), and github.com/grpc-ecosystem/grpc-gateway. We denote the type for a Golang plugin to say whether to do Well-Known Type imports from github.com/golang/protobuf or github.com/gogo/protobuf. The import_path will probably be needed in some form regardless, unless we get too cute. extra_modifiers are needed for grpc-gateway, and all the plugins need to know where to output their result.

One of the goals of Prototool is for users to not have to think about these things, and for output to be consistent - a user can start using Prototool, and all output will be in the same location. Roughly, I have a few potential ideas in my head of how to accomplish this.

gen:
  base_output: ../../gen/proto
  plugins:
    - name: gogoslick-grpc
    - name: yarpc-go
    - name: java
       output: java/example
    - name: grpc-gateway

Where we say output is go for gogoslick-grpc, yarpc-go, grpc-gateway, and the output is java/example for java (the default would have been java).

The Well-Known Plugins off the top of my head would be all the builtins (cpp, csharp, java, etc), all the golang/protobuf and gogo/protobuf plugins, grpc variants of all of these that we can, along with grpc-gateway and yarpc-go (yarpc-go is needed for Uber).

gen:
  plugin_group:
    name: uber
    exclude:
      - cpp
      - python
amckinney commented 6 years ago

This is awesome, and can actually be introduced in a backwards-compatible way since the configuration keys you're suggesting are either equivalent to the current state, or additional.

I would even rename base_output to simply output, which could otherwise be overridden by each of the configured plugins.

Also, we could rely on a custom UnmarshalYAML implementation so that users can configure their well-known plugins without the name key.

So, your full example would become:

gen:
  output: ../../gen/proto
  plugins:
    - gogoslick-grpc
    - yarpc-go
    - name: java
      output: java/example
    - grpc-gateway

This way, the well-known plugins are clearly visible, needing nothing more than a simple identifier. But users are still free to override some of their behavior, as in the java example.

bufdev commented 6 years ago

I don't think this is needed for v1.0 @amckinney @AlexAPB

bufdev commented 5 years ago

Closing this - working towards Inbox Zero, I don't think we're going to do this, and I've actually soured on the idea as it's a Pandora's Box in terms of support.