twitchtv / twirp

A simple RPC framework with protobuf service definitions
https://twitchtv.github.io/twirp/docs/intro.html
Apache License 2.0
7.05k stars 328 forks source link

Import types/funcs from github.com/twitchtv/twirp/... instead of generating them. #328

Closed f0mster closed 2 years ago

f0mster commented 3 years ago

I have multiple proto files and I want all of them to be generated into a single directory. As a result, I have "redeclared in this package" error caused by redeclaring types like HTTPClient, TwirpServer and functions like newServerOpts, WriteError, writeError, and others. Maybe it would be better just to import them instead of generating them in every .twirp.go file.

3ventic commented 3 years ago

The duplicate definitions are omitted as long as all the clients are generated by the same protoc call. See this test package as an example of multiple services in the same directory. You can find the command used to generate the code in gen.go

jeffreylo commented 3 years ago

The duplicate definitions are omitted as long as all the clients are generated by the same protoc call.

@3ventic I believe this doesn’t hold if you structure the service directory differently while generating clients in the same protoc call The resultant api.twirp.go for service2 will not have the shared Twirp types, e.g., HTTPClient as the filesHandled check at https://github.com/twitchtv/twirp/blob/82820bb18559d5a0cc4af61eb189beeaefd9a76d/protoc-gen-twirp/generator.go#L238-L240 assumes we only need to handle a single output directory.

protoc -I=./rpc --go_out=paths=source_relative:./rpc --twirp_out=paths=source_relative:./rpc ./rpc/service1/api.proto ./rpc/service2/api.proto

rpc
└── service1
    └── api.proto
└── service2
    └── api.proto
jeffreylo commented 2 years ago

@3ventic here’s an example: https://github.com/jeffreylo/twirp-multiple-services/blob/806fbd4dc88a73af8ebc10c8baae0a0e9f99ecca/rpc/cordswainer/v1/service.twirp.go#L40. The HTTPClient is missing from the generated code. This was generated with the Makefile command:

protoc \
    -I=./rpc \
    --go_out=paths=source_relative:./rpc \
    --twirp_out=paths=source_relative:./rpc
    rpc/haberdasher/v1/*.proto rpc/cordswainer/v1/*.proto
github-actions[bot] commented 2 years ago

This issue is stale because it has been open 60 days with no activity. Remove stale label / comment or this will be closed in 5 days

wmatveyenko commented 2 years ago

Over the years we’ve used different protobuf-related tools, and we’ve found that many have an expectation that the protoc command gets run a single time for each directory. We’ve built Twirp with that simplification. If you’re using a different structure for your project and use enough protobuf-related tools with it, you may encounter others with the same expectation. We don’t have plans to change Twirp to support that alternative, at least without more indication that it provides broad utility to our user-base.