neoeinstein / protoc-gen-prost

Apache License 2.0
150 stars 41 forks source link

Proto Dependencies Generated Not Included In Main File When Generating Code With Buf #24

Open LuisOsta opened 2 years ago

LuisOsta commented 2 years ago

Here's a short snippet of our proto file:

syntax = "proto3";

package issuer_api.v1;

import "common/v1/common.proto";

...

service IssuerService {
  rpc Authenticate(AuthenticateRequest) returns (AuthenticateResponse);
  rpc GetVerifier(GetVerifierRequest) returns (GetVerifierResponse);
  rpc Authorize(stream AuthorizeRequest) returns (stream AuthorizeResponse);
  rpc Issue(IssueRequest) returns (stream IssueResponse);
}

I generated the code via:

buf generate --path issuer_api

And the main file issuer_api.v1.rs only includes the tonic service definition included:

include!("issuer_api.v1.tonic.rs");

And also the generated code uses the dependency like this:

    pub authentication_signature: ::core::option::Option<super::super::common::v1::AuthenticationSignature>,

Instead of

    pub authentication_signature: ::core::option::Option<AuthenticationSignature>,

By including it

LuisOsta commented 2 years ago

I had found https://github.com/neoeinstein/protoc-gen-prost/issues/19 when investigating this issue but it seems to be a separate issue

neoeinstein commented 2 years ago

Three separate items here. The first is that buf, and by proxy the plugins called by protoc, will only generate code for the items specified. So, by specifying only issuer_api, the items in common will not be generated. To generate the items in common, they must also be specified in the generate command. This part is indeed the same issue as #19. If you want to generate all imports, you can specify --include-imports on the buf generate command. See buf generate -h for more information here.

Second, it appears that you only have protoc-gen-tonic in your buf.gen.yaml file. I can only assume that, since you didn't provide it, but the tonic generator expects that the prost generator was run previously as part of the same run. (See the example in the README) Without that, the tonic generator will generate the include, but there will be no messages defined by Prost.

Third, the pathing used is the normal pathing used by prost. By default, Prost will presume that types will be at a module path relative to the module being generated. Your file generates the issuer_api.v1 package. AuthenticationSignature is in the common.v1 package. These translate to the module paths issuer_api::v1 and common::v1 respectively. The path between them is expected to be super::super::common::v1. If this is not true, there is an option to specify an extern_path (see Options). This may be needed if the generated type is actually in a different crate than the one being generated (as is used for pbjson types).

LuisOsta commented 2 years ago

@neoeinstein Thanks for the quick response! Sorry for not providing the buf.gen.yaml file. Here it is below

version: v1
plugins:
  - remote: buf.build/prost/plugins/prost:v0.1.4-1
    out: src/gen
    opt:
      - compile_well_known_types
      - file_descriptor_set
  - remote: buf.build/prost/plugins/tonic:v0.1.1-1
    out: src/gen
    opt:
      - no_client

So we are doing the generation of the prost plugins. And we are doing the generating of the code by doing buf generate --include-imports, I had pasted the wrong one above sorry about that.

I'll look into the pathing option. Also do you know if its possible to use the extern_path option to effectively rename the package name for common? (through module definition)

gluax commented 2 weeks ago

Hi!!

I'm having a similar issue.

buf generate --template buf.gen.rust.yaml --include-imports
Failure: plugin "buf.build/community/neoeinstein-prost-crate:v0.4.1" failed: No such file or directory (os error 2)

where I have the following layout closely mirroring the example from this repo:

ls -la
.rw-r--r-- 328 gluax  4 Oct 12:33 buf.gen.rust.yaml
.rw-r--r--  35 gluax  4 Oct 11:55 buf.work.yaml
.rw-r--r-- 243 gluax  4 Oct 11:45 Cargo.toml
.rwxr-xr-x 238 gluax  4 Oct 10:28 generate.sh
drwxr-xr-x   - gluax  4 Oct 12:16 proto
drwxr-xr-x   - gluax  4 Oct 12:33 src

ls -la proto/
.rw-r--r-- 1.1k gluax  4 Oct 12:16 buf.lock
.rw-r--r--  453 gluax  4 Oct 11:55 buf.yaml
drwxr-xr-x    - gluax  4 Oct 11:58 sedachain

If I comment out the neoeinstein-prost-crate, the first part works as expected, but I would like this part to work as well.

Here's the contents of buf.gen.rust.yaml:

version: v1
plugins:
  - plugin: buf.build/community/neoeinstein-prost:v0.4.0
    out: src/gen
    opt:
      - compile_well_known_types
      - file_descriptor_set
      - enable_type_names
  - plugin: buf.build/community/neoeinstein-prost-crate:v0.4.1
    out: .
    opt:
      - include_file=src/gen/mod.rs
      - gen_crate

I did try pointing gen_crate directly to the Cargo.toml but that didn't change the error.

Is neoeinstein-prost suppored to generate a mod.rs? As it doesn't currently generate one, and I'm unsure if I'm doing something wrong here :c

neoeinstein commented 2 weeks ago

As noted in the README, the remote plugin is explicitly not compatible with the gen_crate option, as that requires the ability to interact with the local file system. That context is not sent to the remote plugin, so gen_crate cannot be used in this way.

gluax commented 2 weeks ago

As noted in the README, the remote plugin is explicitly not compatible with the gen_crate option, as that requires the ability to interact with the local file system. That context is not sent to the remote plugin, so gen_crate cannot be used in this way.

Ah, ok, sorry I missed that. Is there a link/docs on how to install these plugins locally then? Do I just cargo install/bininstall?

Then to generate a mod.rs is it the same issue for neoeinstein-prost? Or is that a different problem?

EDIT:

plugin: buf.build/community/neoeinstein-prost-crate:v0.4.1
    out: .
    opt:
      - include_file=src/gen/mod.rs

That complains it can't read the Cargo.toml, I'm really lost on how to use these...

kle1n-com commented 1 week ago

Hi gluax,

a few years ago - therefore the configuration/code might require updates - my buf.gen.yaml looked like:

version: v1
managed:
  enabled: true
plugins:
  ###############
  ### generic ###
  ###############
  - name: doc
    out: gen/doc
    opt: markdown,README.md
    strategy: all
  - name: openapiv2
    out: gen/openapi

  ############
  ### rust ###
  ############
  # cargo install protoc-gen-prost
  - name: prost
    out: gen/rust/src
    opt:
      - bytes=.
      - file_descriptor_set
      - compile_well_known_types
      - extern_path=.google.protobuf=::pbjson_types
  # cargo install protoc-gen-prost-serde
  - name: prost-serde
    out: gen/rust/src

  # cargo install protoc-gen-prost-tonic
  - name: tonic
    out: gen/rust/src
    opt:
      - compile_well_known_types
      - extern_path=.google.protobuf=::pbjson_types

  # cargo install protoc-gen-prost-crate
  - name: prost-crate
    out: gen/rust
    strategy: all
    opt:
      - gen_crate=assets/Cargo.toml

You can see that I installed the binaries via cargo install <name>. Of course, there are many ways to install it. I also put a Cargo.toml in an asset directory and referenced it in opt: of the prost-crate plugin definition - this might be outdated.

In my server/client application (was just a "hello-world") I included the crate by adding

[dependencies]
template-api = { version = "0.1.0", path = "../../gen/rust/" }

to my project's Cargo.toml.

I'm sure this does not follow best practices, and I never used it in a bigger project or even in production. But hopefully it helps, in one or another way.