Address review

Author: arjan-bal

.github/workflows/CI.yml

@@ -52,6 +52,10 @@ jobs:
           # Share repository cache between workflows.
           repository-cache: true
           module-root: ./protoc-gen-rust-grpc
+      # Building the protoc plugin from scratch takes 6–14 minutes, depending on
+      # the OS. This delays the execution of workflows that use the plugin in
+      # build.rs files. We try to avoid rebuilding the plugin if it hasn't
+      # changed.
       - name: Build protoc plugin
         if: steps.cache-plugin.outputs.cache-hit != 'true'
         working-directory: ./protoc-gen-rust-grpc

grpc/Cargo.toml

@@ -28,5 +28,4 @@ dns = ["dep:hickory-resolver"]
 allowed_external_types = [
     "tonic::*",
     "futures_core::stream::Stream",
-    "protobuf::codegen_traits::Message",
 ]

grpc/src/macros.rs

@@ -51,10 +51,10 @@
 /// ```rust,ignore
 /// mod protos {
 ///     // Include message code.
-///     include!("/protobuf/directory/protos/generated.rs");
+///     include!("relative/protobuf/directory/generated.rs");
 ///
 ///     // Include service code.
-///     include!("/protobuf/directory/protos/helloworld_grpc.pb.rs");
+///     include!("relative/protobuf/directory/helloworld_grpc.pb.rs");
 /// }
 /// ```
 ///
@@ -64,12 +64,12 @@
 /// ```rust,ignore
 /// mod protos {
 ///     // Include message code.
-///     include!("/protobuf/directory/protos/generated.rs");
+///     include!("relative/protobuf/directory/generated.rs");
 /// }
 ///
 /// mod grpc {
 ///     // Include service code.
-///     include!("/protobuf/directory/proto/helloworld_grpc.pb.rs");
+///     include!("relative/protobuf/directory/helloworld_grpc.pb.rs");
 /// }
 /// ```
 ///

interop/build.rs

@@ -5,7 +5,7 @@ fn main() {
     tonic_protobuf_build::CodeGen::new()
         .include("proto/grpc/testing")
         .inputs(["test.proto", "empty.proto", "messages.proto"])
-        .generate_and_compile()
+        .compile()
         .unwrap();
 
     // prevent needing to rebuild if files (or deps) haven't changed

tonic-protobuf-build/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "tonic-protobuf-build"
-version = "0.9.0-alpha.1"
+version = "0.14.0"
 edition = "2021"
 authors = ["gRPC Authors"]
 license = "MIT"

tonic-protobuf-build/README.md

@@ -11,6 +11,7 @@ Required dependencies
 [dependencies]
 tonic = "<tonic-version>"
 protobuf = "<protobuf-version>"
+tonic-protobuf =  "<tonic-protobuf-version>"
 
 [build-dependencies]
 tonic-protobuf-build = "<tonic-protobuf-build-version>"
@@ -31,7 +32,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
     tonic_protobuf_build::CodeGen::new()
         .include("proto")
         .inputs(["service.proto"])
-        .generate_and_compile()?;
+        .compile()?;
     Ok(())
 }
 ```
@@ -40,18 +41,20 @@ Or configure the generated code deeper via
 
 ```rust,no_run
 fn main() -> Result<(), Box<dyn std::error::Error>> {
-   tonic_protobuf_build::configure()
+    let dependency = tonic_protobuf_build::Dependency::builder()
+        .crate_name("external_protos".to_string())
+        .proto_import_paths(vec![PathBuf::from("external/message.proto")])
+        .proto_files(vec!["message.proto".to_string()])
+        .build()?;
+
+    tonic_protobuf_build::CodeGen::new()
         .generate_message_code(false)
         .inputs(["proto/helloworld/helloworld.proto"])
         .include("external")
         .message_module_path("super::proto")
-        .dependencies(vec![tonic_protobuf_build::Dependency {
-            crate_name: "external_protos".to_string(),
-            proto_import_paths: vec![PathBuf::from("external/message.proto")],
-            proto_files: vec!["message.proto".to_string()],
-        }])
+        .dependencies(vec![dependency])
         //.out_dir("src/generated")  // you can change the generated code's location
-        .generate_and_compile()?;
+        .compile()?;
    Ok(())
 }
 ```

tonic-protobuf-build/src/lib.rs

@@ -28,26 +28,74 @@ use std::path::{Path, PathBuf};
 
 use syn::parse_file;
 
-/// Details about a crate containing proto files with symbols refferenced in
+/// Details about a crate containing proto files with symbols referenced in
 /// the file being compiled currently.
 #[derive(Debug, Clone)]
 pub struct Dependency {
+    crate_name: String,
+    proto_import_paths: Vec<PathBuf>,
+    proto_files: Vec<String>,
+}
+
+impl Dependency {
+    pub fn builder() -> DependencyBuilder {
+        DependencyBuilder::default()
+    }
+}
+
+#[derive(Default, Debug)]
+pub struct DependencyBuilder {
+    crate_name: Option<String>,
+    proto_import_paths: Vec<PathBuf>,
+    proto_files: Vec<String>,
+}
+
+impl DependencyBuilder {
     /// Name of the external crate.
-    pub crate_name: String,
+    pub fn crate_name(mut self, name: impl Into<String>) -> Self {
+        self.crate_name = Some(name.into());
+        self
+    }
+
     /// List of paths .proto files whose codegen is present in the crate. This
     /// is used to re-run the build command if required.
-    pub proto_import_paths: Vec<PathBuf>,
+    pub fn proto_import_path(mut self, path: impl Into<PathBuf>) -> Self {
+        self.proto_import_paths.push(path.into());
+        self
+    }
+
     /// List of .proto file names whose codegen is present in the crate.
-    pub proto_files: Vec<String>,
+    pub fn proto_import_paths(mut self, paths: Vec<PathBuf>) -> Self {
+        self.proto_import_paths = paths;
+        self
+    }
+
+    pub fn proto_file(mut self, file: impl Into<String>) -> Self {
+        self.proto_files.push(file.into());
+        self
+    }
+
+    pub fn proto_files(mut self, files: Vec<String>) -> Self {
+        self.proto_files = files;
+        self
+    }
+
+    pub fn build(self) -> Result<Dependency, &'static str> {
+        let crate_name = self.crate_name.ok_or("crate_name is required")?;
+        Ok(Dependency {
+            crate_name,
+            proto_import_paths: self.proto_import_paths,
+            proto_files: self.proto_files,
+        })
+    }
 }
 
 impl From<&Dependency> for protobuf_codegen::Dependency {
     fn from(val: &Dependency) -> Self {
         protobuf_codegen::Dependency {
             crate_name: val.crate_name.clone(),
             proto_import_paths: val.proto_import_paths.clone(),
-            // TODO: Is this useful to expose the following field? It's not used
-            // by protobuf codegen.
+            // The following field is not used by protobuf codegen.
             c_include_paths: Vec::new(),
             proto_files: val.proto_files.clone(),
         }
@@ -146,7 +194,7 @@ impl CodeGen {
         self
     }
 
-    pub fn generate_and_compile(&self) -> Result<(), String> {
+    pub fn compile(&self) -> Result<(), String> {
         // Generate the message code.
         if self.generate_message_code {
             protobuf_codegen::CodeGen::new()

tonic-protobuf/Cargo.toml

@@ -1,9 +1,10 @@
 [package]
 name = "tonic-protobuf"
-version = "0.9.0-alpha.1"
+version = "0.14.0"
 edition = "2021"
 authors = ["gRPC Authors"]
 license = "MIT"
+publish = false
 
 [dependencies]
 tonic = { version = "0.14.0", path = "../tonic", default-features = false, features = ["codegen"] }

tonic-protobuf/src/lib.rs

@@ -81,6 +81,10 @@ impl<T: Message> Encoder for ProtoEncoder<T> {
     type Error = Status;
 
     fn encode(&mut self, item: Self::Item, buf: &mut EncodeBuf<'_>) -> Result<(), Self::Error> {
+        // The protobuf library doesn't support serializing into a user-provided
+        // buffer. Instead, it allocates its own buffer, resulting in an extra
+        // copy and allocation.
+        // TODO: Find a way to avoid this extra copy.
         let serialized = item.serialize().map_err(from_decode_error)?;
         buf.put_slice(serialized.as_slice());
         Ok(())