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

feat(build): Support prost's include_file option #774

Merged
merged 3 commits into from Oct 20, 2021

Conversation

DerekStrickland
Copy link
Contributor

@DerekStrickland DerekStrickland commented Sep 24, 2021

This PR adds pass-through calls to post to allow tonic users to make use of a new function in prost called include_file.

Motivation

The include_file function provides users with a way to more elegantly handle compiling a set of .proto files with complex dependency hierarchies. Traditionally, users would either need to emulate the folder structure of their .proto sources or manually configure a mod.rs that accurately mapped the source .proto folder structure to a module graph. Now, include_file will do that for you.

Solution

Assuming some complex .proto source folder structure and dependency graph, users can add a call to include_file in their configuration like this.

tonic_build::configure()
        .include_file("mod.rs")
        .compile(
            &[
                "somesource/plugins/base/proto/base.proto",
                "somesource/plugins/drivers/proto/driver.proto",
                "somesource/plugins/shared/foo/foo.proto",
                "somesource/plugins/shared/structs/proto/attribute.proto",
            ],
            &["somesource"],
        )
        .unwrap();

By configuring include_file in the config as shown, prost will now generate a mod.rs that looks like this.

pub mod somesource {
      pub mod plugins {
          pub mod shared {
              pub mod foo {
                  include!("somesource.plugins.shared.foo.rs");
              }
              pub mod structs {
                  include!("somesource.plugins.shared.structs.rs");
              }
          }
          pub mod base {
              pub mod proto {
                  include!("somesource.plugins.base.proto.rs");
              }
          }
          pub mod drivers {
              pub mod proto {
                  include!("somesource.plugins.drivers.proto.rs");
              }
          }
      }
}

@DerekStrickland DerekStrickland marked this pull request as ready for review October 14, 2021 10:19
@tobz tobz self-assigned this Oct 18, 2021
@DerekStrickland DerekStrickland changed the title WIP: Add support for prost's new include_file functionality Add support for prost's new include_file functionality Oct 19, 2021
Copy link
Collaborator

@tobz tobz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this locally, with and without .include_file(..) and the results look valid, and most importantly, .include_file(..) in fact does the right thing. :)

Copy link
Member

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@LucioFranco LucioFranco changed the title Add support for prost's new include_file functionality feat(build): Support prost's include_file option Oct 20, 2021
@LucioFranco LucioFranco merged commit 3f9ab80 into hyperium:master Oct 20, 2021
matt9j added a commit to uw-ictd/dAuth that referenced this pull request Nov 28, 2021
Prost builds heirarchical protos with an implicilty assumed rust
module heirarchy corresponding to the proto package names. You can
actually have tonic (via prost) generate this heirarchy for you-- see
hyperium/tonic#774 .

In our case for readability I would like to export a slightly
different module heirarchy than is currently encoded in the
protos (although this should probably get streamlined and changed in
the future), and this commit fixes the rust module heirarchy to match
what prost expects to genrate, with parent modules corresponding to
the rust package name for any protos reused in other proto packages.
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

Successfully merging this pull request may close these issues.

None yet

3 participants