Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proto package name must match filename #1007

Open
jollygreenlaser opened this issue May 28, 2022 · 11 comments
Open

Proto package name must match filename #1007

jollygreenlaser opened this issue May 28, 2022 · 11 comments

Comments

@jollygreenlaser
Copy link

Bug Report

Version

% cargo tree | grep tonic
├── tonic v0.7.2
└── tonic-build v0.7.2

Platform

% uname -a
Darwin MacBook-Pro 20.6.0 Darwin Kernel Version 20.6.0: Wed Jun 23 00:26:31 PDT 2021; root:xnu-7195.141.2~5/RELEASE_X86_64 x86_64

Description

helloworld.proto:

syntax = "proto3";
package helloworld;

import "proto/request.proto";

// The greeting service definition.
service Greeter {
  // Sends a greeting
  rpc SayHello (r.HelloRequest) returns (HelloReply) {}
}

// The response message containing the greetings
message HelloReply {
  string message = 1;
}

request.proto:

syntax = "proto3";

package r;

// The request message containing the user's name.
message HelloRequest {
    string name = 1;
}

This gives a lot of errors that all look like:

error[E0433]: failed to resolve: could not find `r` in the crate root
  --> /Users/alelazar/Documents/helloworld-tonic/target/debug/build/helloworld-tonic-33d7ab904b011773/out/helloworld.rs:75:60
   |
75 |             request: impl tonic::IntoRequest<super::super::r::HelloRequest>,
   |                                                            ^ could not find `r` in the crate root

This works fine if I change the package from r to request (and update relevant references)

Things also work fine if the packages match - changing r to helloworld.

There is a decent chance I'm just doing things wrong.

Side Note

It would be nice if we could eschew package names altogether - in other languages, I normally only see packages specified in order to differentiate similarly named types. Rust/tonic is forcing me to update cross language proto use to have packages.

@g0hl1n
Copy link
Contributor

g0hl1n commented Jun 7, 2022

Hi,
I'm not sure if I get you right, but AFAIK the file name is "ignored" in the compiled proto and therefore the package must not match the file name.
Nonetheless the package name is normally used as parent mod to generate the sources "in".
Therefore it must match the qualifier in your implementation.

For other languages there are file options to override the generated package (e.g. option java_package = "com.example.foo.bar";). Do you want something similar for rust? Or did I misinterpreted your problem?

@beinan
Copy link

beinan commented Jun 16, 2022

Hello @g0hl1n , I'm looking for the solution of overriding the package name in rust as you mentioned. Is this feature available in tonic now? thanks!

@LucioFranco
Copy link
Member

All of the generated code is possible to be generated into the src dir and changed manually. I don't think we will add specific support for renaming the module unless its specifically breaking some weird part of codegen.

@g0hl1n
Copy link
Contributor

g0hl1n commented Jul 5, 2022

Changing generated code doesn't seem like a valid approach to me.

@beinan whats the reason you want to change the crate name of the generated code?

What are the pros/cons of adding something like an option rust_crate (like described in https://developers.google.com/protocol-buffers/docs/proto3#options for java) to tonic build?

@beinan
Copy link

beinan commented Jul 6, 2022

Thank you @g0hl1n and @LucioFranco for the reply!
Actually I just want to put something like codegen or autogen in the crate name of the generated code, e.g. grpc::autogen::my_service_client. This might make it easy to distinguish between generated code and handwritten code.

Another reason is I'm working on a rust client for a Java grpc service, the package names in grpc proto files are a little bit too deep to maintain in rust, so I'm thinking if we could override package name in rust

@LucioFranco
Copy link
Member

Yes, I would recommend doing something like what is done in tokio-console https://github.com/tokio-rs/console/blob/main/console-api/src/resources.rs#L1

@ncs-pl
Copy link

ncs-pl commented Sep 22, 2022

+1 for having a FileOption extension named rust_crate to customize the name of the generated Rust crate/file.
This would be extremely useful in polyglot environments where we tend to follow protoc's way of generating files. With this sort of option, we could add "genproto"/"codegen"/... in the crate's name to follow internal conventions.

The recommendation in this discussion of generating the files in the repository and modifying the files by name is not a production-grade solution, especially in huge and polyglot environments where code generation is automated and/or controlled hermetically (see Bazel's philosophy for example).

I will try to implement a FileOption extension in a local fork and, in case it does work well, might open a merge request.

@jdortiz
Copy link

jdortiz commented Oct 10, 2022

I also believe that having an option rust_crate would be helpful. At the moment I am working with some proto files that have a package name that doesn't seem very rusty to me (grpc.health.v1) and I would rather get it to produce the right code so it needs no manual changing, even if "manual" means doing it in build.rs. Thanks.

@LucioFranco
Copy link
Member

For optional support we will need #674

@Smityz
Copy link

Smityz commented Mar 7, 2023

I also believe that having an option rust_crate would be helpful. At the moment I am working with some proto files that have a package name that doesn't seem very rusty to me (grpc.health.v1) and I would rather get it to produce the right code so it needs no manual changing, even if "manual" means doing it in build.rs. Thanks.

Also need this function!

@mircodz
Copy link

mircodz commented Mar 18, 2024

+1 for having a way to specify the package name outside of the file path. I'm currently dealing with proto files with long paths, which leads to a deep folder structure and having to import the code through com::org::project::.... It's not a deal-breaker, but I think this could lead to cleaner code is some situations.

Edit: I wrote this small hack to solve the import issue.

use std::fs::OpenOptions;
use std::io::Write;

static suffix: &str = r#"
pub mod foobar {
    pub use crate::path::to::package::*;
}
"#;

fn main() -> Result<(), Box<dyn std::error::Error>> {
    tonic_build::configure()
        // ...
        .compile(
            &["pb/foobar.proto"],
            &["pb"]
        )?;

    let mut file = OpenOptions::new()
        .write(true)
        .append(true)
        .open("src/pb/mod.rs")
        .unwrap();

    if let Err(e) = write!(file, "{}", suffix) {
        eprintln!("Could not write to file: {}", e)
    }

    Ok(())
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants