Skip to content

Commit

Permalink
fix(build): Don't replace extern_paths (#261)
Browse files Browse the repository at this point in the history
* don't replace extern_path'd paths

tonic-build will replace all paths except for google well known types
with one rooted at the super module. For paths that have already been
replaced with a fully qualified path via the extern_path config option,
(e.g. "::uuid::Uuid"), this results in an invalid path
(e.g. "super::::uuid::Uuid"), and a build failure. These paths should
also be excluded when prefixing relative modules with super.

* add doc in tonic-build clarifying extern_path

extern_path expects fully qualified proto and rust paths

* add test cast case for extern path fix

add a test that extern path does indeed result in service types using
the specified type from an external crate. we test this by creating a
service type that has a proto from a different crate, and asserting that
it does indeed impl a trait from that crate

* add license/publish to extern_path test crates
  • Loading branch information
cthulhua authored Feb 18, 2020
1 parent e2bff8f commit 1b3d107
Show file tree
Hide file tree
Showing 10 changed files with 132 additions and 2 deletions.
2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,6 @@ members = [
"tests/included_service",
"tests/same_name",
"tests/wellknown",
"tests/extern_path/uuid",
"tests/extern_path/my_application"
]
18 changes: 18 additions & 0 deletions tests/extern_path/my_application/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
[package]
name = "my_application"
version = "0.1.0"
authors = ["Danny Hua <[email protected]>"]
edition = "2018"
publish = false
license = "MIT"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
tonic = { path= "../../../tonic" }
prost = "0.6"
prost-types = "0.6"
uuid = { path= "../uuid" }

[build-dependencies]
tonic-build = { path= "../../../tonic-build" }
11 changes: 11 additions & 0 deletions tests/extern_path/my_application/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
fn main() -> Result<(), std::io::Error> {
tonic_build::configure()
.build_server(false)
.build_client(true)
.extern_path(".uuid", "::uuid")
.compile(
&["service.proto", "uuid.proto"],
&["../proto/my_application", "../proto/uuid"],
)?;
Ok(())
}
26 changes: 26 additions & 0 deletions tests/extern_path/my_application/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
use uuid::DoSomething;
mod pb {
tonic::include_proto!("my_application");
}
fn main() {
// verify that extern_path to replace proto's with impl's from other crates works.
let message = pb::MyMessage {
message_id: Some(::uuid::Uuid {
uuid_str: "".to_string(),
}),
some_payload: "".to_string(),
};
dbg!(message.message_id.unwrap().do_it());
}
#[cfg(test)]
#[test]
fn service_types_have_extern_types() {
// verify that extern_path to replace proto's with impl's from other crates works.
let message = pb::MyMessage {
message_id: Some(::uuid::Uuid {
uuid_str: "not really a uuid".to_string(),
}),
some_payload: "payload".to_string(),
};
assert_eq!(message.message_id.unwrap().do_it(), "Done");
}
25 changes: 25 additions & 0 deletions tests/extern_path/proto/my_application/service.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
syntax = "proto3";

package my_application;

option go_package = "my_applicationpb";
option java_multiple_files = true;
option java_outer_classname = "ServiceProto";
option java_package = "com.my_application";


import "uuid.proto";

message MyMessage {
uuid.Uuid message_id = 1;
string some_payload = 2;
}

service MyService {
rpc GetUuid(MyMessage) returns (uuid.Uuid){

}
rpc GetMyMessage(uuid.Uuid) returns (MyMessage){

}
}
13 changes: 13 additions & 0 deletions tests/extern_path/proto/uuid/uuid.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
syntax = "proto3";

package uuid;

option go_package = "uuidpb";
option java_multiple_files = true;
option java_outer_classname = "UuidProto";
option java_package = "com.uuid";


message Uuid {
string uuid_str = 1;
}
15 changes: 15 additions & 0 deletions tests/extern_path/uuid/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
[package]
name = "uuid"
version = "0.1.0"
authors = ["Danny Hua <[email protected]>"]
edition = "2018"
publish = false
license = "MIT"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
prost = "0.6"
bytes = "0.5"
[build-dependencies]
prost-build = "0.6"
3 changes: 3 additions & 0 deletions tests/extern_path/uuid/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
prost_build::compile_protos(&["uuid/uuid.proto"], &["../proto/"]).unwrap();
}
11 changes: 11 additions & 0 deletions tests/extern_path/uuid/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
include!(concat!(env!("OUT_DIR"), "/uuid.rs"));

pub trait DoSomething {
fn do_it(&self) -> String;
}

impl DoSomething for Uuid {
fn do_it(&self) -> String {
"Done".to_string()
}
}
10 changes: 8 additions & 2 deletions tonic-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ impl Builder {
/// Declare externally provided Protobuf package or type.
///
/// Passed directly to `prost_build::Config.extern_path`.
/// Note that both the Protobuf path and the rust package paths should both be fully qualified.
/// i.e. Protobuf paths should start with "." and rust paths should start with "::"
pub fn extern_path(mut self, proto_path: impl AsRef<str>, rust_path: impl AsRef<str>) -> Self {
self.extern_path.push((
proto_path.as_ref().to_string(),
Expand Down Expand Up @@ -318,15 +320,19 @@ fn generate_doc_comments<T: AsRef<str>>(comments: &[T]) -> TokenStream {
}

fn replace_wellknown(proto_path: &str, method: &Method) -> (TokenStream, TokenStream) {
let request = if method.input_proto_type.starts_with(".google.protobuf") {
let request = if method.input_proto_type.starts_with(".google.protobuf")
|| method.input_type.starts_with("::")
{
method.input_type.parse::<TokenStream>().unwrap()
} else {
syn::parse_str::<syn::Path>(&format!("{}::{}", proto_path, method.input_type))
.unwrap()
.to_token_stream()
};

let response = if method.output_proto_type.starts_with(".google.protobuf") {
let response = if method.output_proto_type.starts_with(".google.protobuf")
|| method.output_type.starts_with("::")
{
method.output_type.parse::<TokenStream>().unwrap()
} else {
syn::parse_str::<syn::Path>(&format!("{}::{}", proto_path, method.output_type))
Expand Down

0 comments on commit 1b3d107

Please sign in to comment.