tower-rs / tower-grpc

A gRPC client & server implementation.
MIT License
562 stars 73 forks source link

Building multiple .proto files as clients together doesn't work #204

Open hrydgard opened 5 years ago

hrydgard commented 5 years ago

This seems similar to #9, but when you compile multiple files together instead of having multiple services in one file.

Basically, I have first.proto:

package MyPackage;
service MyService
{
    rpc Compute(Inputs) returns (Outputs);
}

And second.proto:

package MyPackage;
service MySecondService
{
    rpc ComputeSecond(Inputs) returns (Outputs);
}

Trying to build them with a build.rs like this fails:

fn main() {
    println!("Compiling protos...");
    tower_grpc_build::Config::new()
        .enable_client(true)
        .build(&["proto/first.proto", "proto/second.proto"], &["proto/"])
        .unwrap_or_else(|e| panic!("protobuf compilation failed: {}", e));
}

The issue is that the generated code gets multiple "mod client" which really could be one without issue.

LucioFranco commented 5 years ago

@hrydgard Hi! So the issue here is that they are under the same package. The issue is that the codegen currently does not have knowledge of the other services and packages when generating the codegen. I think the best solution here is to update tower-grpc-build to nest every service under the same mod. So this would require the https://github.com/tower-rs/tower-grpc/blob/master/tower-grpc-build/src/lib.rs#L20 to collect all the generated services. Then in https://github.com/tower-rs/tower-grpc/blob/master/tower-grpc-build/src/lib.rs#L99 output one set of mod client. The issue here is that finalize gets called multiple times and its impossible to know when the last time it will be called. I think this relates to large issues around codegen in prost.

I'm happy to think through on this if you want to ping me on gitter to talk some more in the tower channel :)

hrydgard commented 5 years ago

I worked around it for now by pasting the two .proto files together into one, which worked perfectly. So multiple services in one file works fine. When you specify a list of multiple files, maybe it could just join the ones that have the same package before parsing them? Not sure if that would require less reworking though than your proposal, I haven't looked much into the source of this project yet.

Alternatively, another workaround would be possible by having control of the generated filename from build.rs. That would let you do multiple generate calls, with the filenames namespacing them so they don't collide.

LucioFranco commented 5 years ago

@hrydgard Ok that work around seems fine, pretty much either change will have to make a large change to tower-grpc-build (which is nasty right now due to codegen) or change how prost-build works. I believe there might be some changes coming along in that crate that may support your use case better.

I think there is a better long term solution but I will need to explore it a bit more.