namespacing issues with services when not using prost-types for well-known protobuf types

[tdyas]

Bug Report

Version

tonic v0.3.1
tonic-build v0.3.1
prost at danburkert/prost@a1cccbc (v0.6.x at the commit with Bytes support)

Platform

macOS 10.15.7

Crates

tonic-build

Description

I am using Prost pinned to master at danburkert/prost@a1cccbc (which includes the use of Bytes for binary fields).

I would like to have Prost compile the well-known types (instead of using prost-types) so that Any fields use Bytes. However, when I enable that mode via config.compile_well_known_types() in build.rs, generated grpc service stubs fail to compile because they are in a _client submodule and are missing an extra super reference necessary to get the identifier path out of that submodule.

For example, when compiling the Google Operations API protos, errors like the following occur:

error[E0433]: failed to resolve: could not find `protobuf` in `super`
   --> XXX/target/debug/build/bazel_protos-4307470ac76a8035/out/google.longrunning.rs:183:40
    |
183 |     ) -> Result<tonic::Response<super::protobuf::Empty>, tonic::Status> {
    |                                        ^^^^^^^^ could not find `protobuf` in `super`

This code was within an operations_client submodule. The reference needs to be super::super::protobuf::Empty in this case, not super::protobuf::Empty. (The service in this example returns a google.protobuf.Empty.)

I assume the issue is the special casing of identifiers in google.protobuf performed in the service generator at

tonic/tonic-build/src/prost.rs

Line 96 in 3c2a4a2

let request = if self.input_proto_type.starts_with(".google.protobuf")

and

tonic/tonic-build/src/prost.rs

Line 106 in 3c2a4a2

let response = if self.output_proto_type.starts_with(".google.protobuf")

. That special casing does not apply the proto_path to google.protobuf types but implicitly assumes that they will be found in prost-types, which is not the case if they are being compiled by Prost.

I am willing to contribute a fix, but would need some basic guidance on what the right fix is. I assume the fix be as simple as just taking the else branch when well-known types are being compiled?

[tdyas]

I also assume that a compile_well_known_types function would need to be added to tonic-build because prost_build::Config does not expose the prost_types field (nor for that matter does Config expose any of the configured values to callers outside of prost_build). Thus, there is no way currently for Tonic to know that well-known types are being compiled. The new function in tonic-build would set a config in Tonic and then call the underlying .compile_well_known_types function in prost_build::Config.

[tdyas]

I wrote an initial attempt to solve in #522. It still needs guidance though.

[tvolk131]

I'm running into this exact same issue. Thanks for reporting/fixing!

[davidpdrsn]

@tdyas was this fixed by #522?

[tdyas]

@tdyas was this fixed by #522?

I'll have to check since I haven't had a chance to get back to the original issue that prompted me to need that functionality. pantsbuild/pants#11411

I'll be able to get to it later this week.

[davidpdrsn]

@tdyas any updates?

[tdyas]

I have a branch on Pants where I tried to use it with tonic-build v0.4.2 and prost-build v0.7.0.

My main remaining issue is how to retain the default mapping of google.protobuf.Empty to () (and any other mappings to primitives). I used config.extern_path(".google.protobuf.Empty", "()"); in build.rs, but this results in the error:

   Compiling bazel_protos v0.0.1 (REDACTED/TC/pants/src/rust/engine/bazel_protos)
error: failed to run custom build command for `bazel_protos v0.0.1 (REDACTED/pants/src/rust/engine/bazel_protos)`

Caused by:
  process didn't exit successfully: `REDACTED/pants/src/rust/engine/target/debug/build/bazel_protos-19e3354872bde210/build-script-build` (exit status: 101)
  --- stderr
  thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error("expected identifier")', REDACTED/.cargo/registry/src/github.com-1ecc6299db9ec823/tonic-build-0.4.2/src/prost.rs:123:18
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Commenting out that line fails as expected at every site using () because the build step now generates code expecting bazel_protos::gen::google::protobuf::Empty instead. So tonic-build and prost-build are somewhat doing the right thing in this case, but it is unclear how to retain the mapping to ().

Any advice?

[tdyas]

https://github.com/tdyas/pants/tree/use_bytes_for_any is the branch in question.

[tdyas]

#734 fixes the error when trying to convert google.protobuf.Empty to ().

[LucioFranco]

@tdyas what is left for this issue? or can we close it?