Prost: transitive dependencies in the same package are duplicated

[william-smith-skydio]

I have a setup with several protobuf files in the same package split across different proto_library targets. For example, C -> B -> A, and all are in the same package. I find that A and B get generated properly, but C ends up with an extra copy of A inside it. That type doesn't get used, but it can fail to compile if A depends on another type from a different package.

To illustrate see this gist. I added these files in proto/prost/private/tests/transitive_same_package.

With bazel build //proto/prost/private/tests/transitive_same_package:all I get this error:

ERROR: /home/williamsmith/others/rules_rust/proto/prost/private/tests/transitive_same_package/BUILD.bazel:39:14: Compiling Rust rlib c_proto (1 files) failed: (Exit 1): process_wrapper failed: error executing command (from target //proto/prost/private/tests/transitive_same_package:c_proto) bazel-out/k8-opt-exec-2B5CBBC6/bin/util/process_wrapper/process_wrapper --arg-file bazel-out/k8-fastbuild/bin/external/rules_rust_prost__axum-0.6.18/axum_build_script.linksearchpaths --arg-file ... (remaining 117 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
error[E0433]: failed to resolve: could not find `external` in the crate root
 --> bazel-out/k8-fastbuild/bin/proto/prost/private/tests/transitive_same_package/c_proto.lib.rs:9:51
  |
9 |         pub foobar: ::core::option::Option<super::external::External>,
  |                                                   ^^^^^^^^ could not find `external` in the crate root

error: aborting due to previous error

For more information about this error, try `rustc --explain E0433`.

This is because the generated bazel-out/k8-fastbuild/bin/proto/prost/private/tests/stuff/c_proto.lib.rs contains a copy of A that is incorrect:

// @generated

pub mod stuff {
    // @generated
    #[allow(clippy::derive_partial_eq_without_eq)]
    #[derive(Clone, PartialEq, ::prost::Message)]
    pub struct A {
        #[prost(message, optional, tag = "1")]
        pub foobar: ::core::option::Option<super::foobar::Foobar>,
    }
    #[allow(clippy::derive_partial_eq_without_eq)]
    #[derive(Clone, PartialEq, ::prost::Message)]
    pub struct C {
        #[prost(message, optional, tag = "1")]
        pub b: ::core::option::Option<::b_proto::stuff::B>,
    }
    // @@protoc_insertion_point(module)
}

So it seems that the direct dependencies of types in the same package get excluded because they are listed in --prost_opt=extern_path. But transitive dependencies aren't listed there, and prost-build assumes that all types in the same package should be generated in the same file. And it assumes that foobar::Foobar is defined in super, since there was no extern_path specified for it.

Some options that come to mind:

1. Specify extern_path for all transitive dependencies
2. Patch prost-build to not generate all transitive dependencies

But neither seems particularly good. Perhaps I am missing something obvious. I will keep looking into this but I thought I would post an issue in case someone else has insights.

[william-smith-skydio]

This may be better-characterized as an issue with protoc-gen-prost, although I haven't tried replicating without involving rules_rust yet. I am getting mileage out of this patch, and haven't found any issues so far:

diff --git a/protoc-gen-prost/src/lib.rs b/protoc-gen-prost/src/lib.rs
index 2098949..d92ab0a 100644
--- a/protoc-gen-prost/src/lib.rs
+++ b/protoc-gen-prost/src/lib.rs
@@ -84,21 +84,24 @@ impl ModuleRequestSet {
         let requests = proto_file.into_iter().zip(raw_protos.proto_file).fold(
             BTreeMap::new(),
             |mut acc, (proto, raw)| {
-                let module = Module::from_protobuf_package_name(proto.package());
                 let proto_filename = proto.name();
-                let entry = acc
-                    .entry(module)
-                    .or_insert_with(|| ModuleRequest::new(proto.package().to_owned()));
-
-                if entry.output_filename().is_none() && input_protos.contains(proto_filename) {
-                    let filename = match proto.package() {
-                        "" => default_package_filename.to_owned(),
-                        package => format!("{package}.rs"),
-                    };
-                    entry.with_output_filename(filename);
+                if input_protos.contains(proto_filename) {
+                    let module = Module::from_protobuf_package_name(proto.package());
+                    let entry = acc
+                        .entry(module)
+                        .or_insert_with(|| ModuleRequest::new(proto.package().to_owned()));
+
+                    if entry.output_filename().is_none() {
+                        let filename = match proto.package() {
+                            "" => default_package_filename.to_owned(),
+                            package => format!("{package}.rs"),
+                        };
+                        entry.with_output_filename(filename);
+                    }
+
+                    entry.push_file_descriptor_proto(proto, raw);
                 }
 
-                entry.push_file_descriptor_proto(proto, raw);
                 acc
             },
         );

This is obviously related to https://github.com/neoeinstein/protoc-gen-prost/issues/70.

It's not clear to me why we would ever want to generate types for a proto file not specified as one of the direct inputs, but perhaps this is important in other situations that don't happen with rules_rust.

I will follow up with an issue on protoc-gen-prost. I'll keep this issue open since it is also relevant to rules_rust, but feel free to close if you prefer.