feat: Add protoc-gen-prost #492

Tuetuopay · 2021-06-25T10:37:11Z

As the title says, this adds a new subcrate, protoc-gen-prost, a protoc plugin for code generation.

Features

Is supports all the options the regular prost-build does, with the addition of a full Cargo crate generation, with a Cargo.toml file and feature flags for each protobuf package, with proper feature dependency resolution.

Motivation

While the recommended way to use prost in Rust projects is with prost-build and running protoc through Cargo and build.rs, a protoc plugin allows to get a standard workflow in the protobuf ecosystem. Also, precompiling proto files to Rust code as files has some advantages:

easier to share compiled code across multiple projects, since they don't all need to setup the prost build
rust code can be stored in an easy to browse fashion
integrates well with standard protobuf/grpc/... tooling

Notes

The generation API is public, and not restricted to prost, as it is useful for other codegen tools for downstream RPC crates based off prost, like tonic (for which I have a similar PR incoming).

Examples

All examples assume protoc-gen-prost was installed and can be found by protoc, and the output directory was created:

 ~/prost$ cargo install protoc-gen-prost --path protoc-gen-prost --release
 ~/prost$ mkdir hello-rs

Basic usage

protoc --prost_out=hello-rs -I prost-build/third-party/protobuf/include -I prost-build/src hello.proto

Results in:

hello-rs/
└── helloworld.rs

Module generation

This is suitable to generate a full module, typically directly in your crate source:

protoc --prost_out=hello-rs --prost_opt=include_file=mod.rs \
    -I prost-build/third-party/protobuf/include -I prost-build/src hello.proto

Results in

hello-rs/
├── helloworld.rs
└── mod.rs

Generated mod.rs:

pub mod helloworld {
    include!("helloworld.rs");
}

Crate generation

This is suitable for larger proto codebases, as feature flags allow to turn individual package on and off during Rust compilation, greatly speeding up build times. It also allows for easier sharing of the proto across multiple projects by making it "just another dependency".

It uses a Cargo.toml template:

[package]
name = "hello-rs"
version = "1.0.0"
authors = []
edition = "2018"

[dependencies]
prost = "0.8"
prost-types = "0.8"
tonic = "0.5"

[features]
{{ features }}

protoc --prost_out=hello-rs --prost_opt=gen_crate=hello-rs/Cargo.toml.tpl \
    -I prost-build/third-party/protobuf/include -I prost-build/src hello.proto

Results in the full crate:

hello-rs/
├── Cargo.toml
├── Cargo.toml.tpl
├── gen
│   └── helloworld.rs
└── src
    └── lib.rs

With the following lib.rs:

pub mod helloworld {
    #[cfg(feature = "helloworld")]
    include!("../gen/helloworld.rs");
}

This can be combined with the include_file option to name lib.rs something else, if some custom, non-generated code, needs to be added to the crate.

LucioFranco · 2021-07-06T16:56:51Z

This seems really interesting! How does the protoc plugin get bundled with protoc? How would this affect our release schedule, etc? I will have to dig in a bit more here before I review but I like this idea!

Tuetuopay · 2021-07-06T17:06:35Z

How does the protoc plugin get bundled with protoc?

Plugins are not usually bundled along protoc (except maybe for protoc-gen-go, idk), but installed separately. While most of the language generation plugins are released along protoc (e.g. https://github.com/protocolbuffers/protobuf/releases/tag/v3.17.3), you still download the plugins separately from protoc. Hence the cargo install protoc-gen-prost in my examples, which is the kind of workflow I would expect should the crate be published on crates.io:

Install protoc, by downloadin it from the protobuf release page
Install the plugin through cargo install or by downloading a precompiled version from a prost release page
Enjoy!

How would this affect our release schedule, etc?

My wild guess would be it to be pretty inoffensive, as it is just a wrapper around the rest of prost: it will follow whatever changes prost does.
I'd expect most changes would come when prost itself adds a new compilation options, which would need to be propagated to the options accepted by this plugin.

cab · 2021-07-07T11:06:18Z

prost-build/src/lib_generator.rs

+}
+
+impl<'a> LibGenerator<'a> {
+    pub fn generate_librs(config: &'a mut Config, mods: &Mod, buf: &'a mut String) {


i like this a lot! it's similar to how i'm currently using prost, but this is nicer.

what do you think of optionally adding a pointer to the FileDescriptorSet in the generated lib.rs? something like

pub const FILE_DESCRIPTOR_SET: &[u8] = include_bytes!("./file_descriptor_set.bin");

Yup, good idea. I actually forgot about this option!

Though it will be a bit more complicated, as outputting the .bin is not possible through the protoc plugin interface, as the protocol uses the proto string type, which is checked for valid utf8 contents by protoc.

So either the file is written by the plugin itself, which is not how protoc plugins are supposed to work (though it would work), or the contents is outputted to some other .rs file, already expanded to some vec![0x00, 0x43, 0xa3, ..] expression. That'd be verbose, but it'd work a treat.

I'll try adding the second option.

Done in 2a97b84. It adds the file_descriptor_set option to the plugin, that can optionally take the name of the generated file (defaults to file_descriptor_set).

It adds a include!("../gen/file_descriptor_set.rs"); to lib.rs (or mod.rs if not generating a full crate), and the .rs file is a vec of all bytes, e.g.

Tuetuopay · 2021-08-09T08:44:31Z

@cab @LucioFranco force-pushed a rebase on master (for prost 0.8)

visortelle · 2021-08-23T09:58:01Z

Holy moly! 😀 Please merge this PR.
It's needed to easily integrate the prost with buf.
Thank you!

Tuetuopay · 2021-08-23T10:00:14Z

Holy moly! 😀 Please merge this PR.
It's needed to easily integrate the prost with buf.
Thank you!

@visortelle super happy for you to see this, as I did this plugin exactly to integrate with buf!

henrikrudstrom · 2021-08-25T19:32:54Z

yes, exactly what i need!

LucioFranco · 2021-08-26T13:17:20Z

Sorry for the delay on this, I hope to get to reviewing this soon.

GregBowyer · 2021-09-11T01:15:11Z

@Tuetuopay In wiring this up to https://github.com/rules-proto-grpc/rules_proto_grpc there is an additional change that I think needs to be considered. To add to pull-ception I have made a PR here Tuetuopay#1.

I am going to keep that branch ongoing and see if anything else is needed to get the rest of it to work.

Tuetuopay · 2021-09-11T10:58:02Z

@GregBowyer good catch, thanks!

I need to get back to this, as I need to refactor mine quite a bit to account for #433 (did you say PRception?) which a) conflicts with my code and b) makes some of mine somewhat redundant, but does so in a radically different way (and without feature flags, which is a requirement).

I hope to get some time this weekend, but I’ll have some for sure next week.

GregBowyer · 2021-09-14T14:47:12Z

@Tuetuopay while I am messing with your work, do you have tonic-build changes to share?

Tuetuopay · 2021-09-14T14:59:52Z

@GregBowyer yup, here they are https://github.com/Tuetuopay/tonic/tree/protoc-gen-tonic . They're much simpler as it's just a wrapper around prost (e.g. crate delegation is handled by prost-build), so there's not much code in there.

I would have preferred a more idiomatic protoc way, where you invoke both compilers and protoc-gen-tonic only adds the rpc part, but due to the way tonic-build is made (calls prost-build itself), it would be quite a lot of changes.

(that makes me notice I need to rebase this one too)

Tuetuopay · 2021-09-22T13:38:45Z

@LucioFranco I finally got time to get back to this. I rewrote part of the PR to account for #433, building upon it, as you now can choose the name of the include file when building a full crate: it will default to lib.rs, but it can be something else with the include_file option. This is useful when you want to add custom code to the crate, as it allows you to write the lib.rs yourself, with non-generated code.

I also rewrote the arg string parsing to allow for escape characters, which is useful along with the type_attr argument (e.g. to pass #[derive(Serialize, Deserialize)] which contains a comma).

@GregBowyer I read your PR, and it seems fine to me, though I don't see how it relates to a protoc plugin. Is it a condition that's hit only for ahead of time compilation? (we should discuss it in your PR tho)

Tuetuopay · 2021-09-29T13:49:00Z

@LucioFranco rebased on master. Do you have an idea when you might get time to look at this?

UebelAndre · 2021-10-11T20:03:13Z

Friendly ping here. Any updates? Would love to see this change go in 😄

LucioFranco · 2021-10-12T14:56:22Z

Hi all, yes I still plan to review this but I have been focusing on the https://github.com/tokio-rs/prost/milestone/1 release as it has been blocking some from upgrading and I deem that to be slightly high priority than this atm. I have already started to play with this a bit and collect some feedback but have not had time to fully review, but I have not forgotten. Sorry for the super long delay.

LucioFranco · 2021-10-14T14:32:10Z

So we have a similar task for this in tonic that I would like to tackle at the same time. That said I think its critical a tonic release gets cut w/ a few bug fixes & support the new prost before we focus on this. Just wanted to keep everyone updated, since this crate doesn't need to follow the traditional release cycle of prost its a bit off cycle now.

UebelAndre · 2021-11-02T17:47:52Z

Hello, friendly ping again. How's all the releases looking?

This has the advantage of letting the caller handle file I/O, so we only need to care about the file contents itself. It also forces it to be self-contained and not rely on whether we're using the OUT_PATH env variable or an override through options (because yes, `include!` is relative to the current file, so `./...` works in both cases).

UebelAndre · 2021-11-22T15:17:56Z

@LucioFranco Pinging one more time 😅 (sorry to be a bother!)

UebelAndre · 2021-11-29T17:29:48Z

@LucioFranco @Tuetuopay Sorry to ping again, any updates for this PR? Is it still something that might be merged? 😅

Tuetuopay · 2021-11-29T17:32:11Z

@UebelAndre well for me I still hope to get it merged 😄

UebelAndre · 2021-12-07T18:57:04Z

@LucioFranco sorry to keep pinging. But any chance you could take another look here? 🙏 😅

LucioFranco · 2021-12-07T23:08:17Z

Hi all sorry for the delay, ive been busy with some personal things and work (its that time of year xD). My progress on this will be slow, we still have some high priority fixes needed for tonic that I need to get too. I might suggest that we put this in a separate repo for now. I honestly don't think I have the bandwidth right now to add this into prost and maintain it.

UebelAndre · 2021-12-08T16:59:32Z

All good! Hopefully my pinging wasn't annoying 😅. I think the changes here would be a good addition to to prost and don't think the maintenance would be super high. I'm happy to do what I can to help move this PR (and new crate) forward but totally understand wanting to have a more focused scope. Would it help to reconvene in a few weeks on this or would you still lean towards not merging this?

LucioFranco · 2021-12-09T23:36:18Z

Not at all, pinging was good and helpful :) I appreciate the pings. We can leave it open and see how much time I get in a few weeks. If you want more immediate progress I might suggest your own crate/repo.

UebelAndre · 2021-12-10T00:32:31Z

From my side of things, folks are able to generally get by by using the changes here in another repo. The interest I have in this PR though is it becomes a way to unify efforts around the use of prost in Bazel (bazelbuild/rules_rust#915). There are solutions out there but none of them feel particularly good. This PR contains some things that would solve for some of the clunkyness both by the changes proposed but also because it'd be a crate that's a part of prost and a unified place to contribute to. I'm happy to wait a few weeks though but still have great interest in seeing changes like these make it through 😄

Maher4Ever · 2022-01-06T10:04:58Z

@Tuetuopay Have you used this with the Remote Plugin Execution feature for buf.build? If so, would you mind sharing how you managed to get it working?

Tuetuopay · 2022-01-06T13:30:52Z

@Maher4Ever No I have not, because we don't have a need for it (we have a CI that builds on every commit, so I don't have a need to build locally). I will try!

As for getting it to work, did you set the buf strategy to all? Due to the nature of Rust generation, the default strategy is not supported by this plugin.

Maher4Ever · 2022-01-07T14:50:54Z

@Tuetuopay Please let me know when you try and succeed to get it working :)

Regarding the strategy: Thanks for letting me know! I used the default one which is directory. I'll check with all

UebelAndre · 2022-01-28T14:04:36Z

@LucioFranco Hello, I'm wondering if you have some extra cycles to revisit this 😅

Happy new year and hope your holiday was good!

Maher4Ever · 2022-02-11T15:56:36Z

@LucioFranco Is there any chance this feature could be released soon? It opens up the possibility for using buf.build with rust which allows having a unified pipeline for supporting protobuf in polyglot applications.

LucioFranco · 2022-02-11T16:11:39Z

Its probably best if this is done outside of prost for now. I don't think there is anything holding it back from being outside this repo. But I will not have the time going forward to maintain this and it would be unfair the users that use other current features in prost for me to not spend time on maintaining the subset that we have now. Sorry about this but I think its the right choice. I appreciate everyone who has contributed for this, but unfortunately mt time is limited right now.

I am going to close this out for now, thanks everyone.

Tuetuopay · 2022-02-11T16:24:08Z

Welp expect another PR soon-ish that exposes what is required by the protoc plugin. Ish because I don't have that much bandwidth right now to rewrite this whole thing. Do you expect to have some bandwidth in the future for a PR that exposes a bit more of prost-gen internals?

Thanks for the heads up. However, this does frighten me a bit as for the future of Prost to see maintainers that don't have enough time to review a PR in 8 months (except for @cab, thank you for your time!). Don't be mistaken, I'm not saying that I am entitled for a review or anything else, you all already made an awesome library and owe me nothing, and OSS is definitely time consuming. I really hope prost does not dwindles due to the lack of maintainers. Good luck!

To all people here that want this protoc plugin: I will continue maintaining the forks (both prost and tonic) for the time being as I use both in production. Thanks all for the support :)

LucioFranco · 2022-02-11T16:37:06Z

Do you expect to have some bandwidth in the future for a PR that exposes a bit more of prost-gen internals?

Yes, I will have bandwidth for this. Though exposing internal stuff may take time to evaluate. Can we start with an issue of what you need exposed before you open a PR?

However, this does frighten me a bit as for the future of Prost to see maintainers that don't have enough time to review a PR in 8 months.

I wouldn't be worried. I am committed to maintaining prost in its current state. For some context. this library was originally written by Dan Burket. He originally ran out of time to maintain it so we ended up moving the project under tokio as an org to maintain, that way we have a few people that have access to merge/publish. My time right now is split between tonic, tokio, tower, and prost (obviously tonic uses prost so there is the link).

What I want to avoid is adding additional things that make the overall maintenance of these projects more more burdensome than it already is. OSS is hard and its a thankless (though yall have been super nice and thankful) task. I would be lying if I didn't say its exhausting and that maintaining all of this has a mental cost. The reason I chose not to include this (and why I have not gotten to a review) is because this is new code that is not related to the core aspects of prost and would increase the maintenance on this project. Any new features/lines of code needs to be evaluated not just how much it adds now but how much it will add in the future.

My current job gives me time to work on OSS (we've had some internal deadlines and holidays that have messed this up a bit) so its not like this is done in my free time but I need to be selective on what to work on and where to use my time. I wish I could satisfy every customer and I am a die hard yes person. So this is nothing against anyone but more so protection for prost (and other projects) as a whole. Something like this could easily be done as an external library and thus its not required to live in this repo.

That said, I am 100% committed to maintaining prost for our customers in the current feature set and what is needed for tonic. I am easily reachable on the tokio discord (https://discord.gg/tokio) and I check that everyday (I am a bit behind on my gh notifications :( ). So if there is something urgent please do reach out.

I appreciate all the support and lets please collaborate on what yall need to make this happen. Hope everyone has a good weekend. :)

Maher4Ever · 2022-02-11T17:17:51Z

@LucioFranco Thank you for make a decision and not leaving us all hanging.

@Tuetuopay Glad to hear you'll continue maintaining the forks. Would it be possible to publish protoc-gen-prost and protoc-gen-tonic to crates.io to simplify installation? Or do you want to do this only when both plugins are released as separate projects after the required internal API's of prost have been exposed?

Tuetuopay · 2022-02-11T18:21:42Z

Can we start with an issue of what you need exposed before you open a PR?

Sure! Let me get back at it and compile what I need (it's been a while since I've touched that code).

Thank you for the thorough explanation! Indeed, the fact that prost is under Tokio gives confidence, and I had more interrogations that worries to be fair, and you answered all of them. And the discord is joined (yay another rust discord!). See you over there too.

Hope everyone has a good weekend. :)

You too :)

Would it be possible to publish protoc-gen-prost and protoc-gen-tonic to crates.io to simplify installation?

I refrained to do this before because I wanted the crates to be under the control of Tokio (prost) and Hyperium (Tonic), both because it's their project, and because it wouldn't be seen as some takeover the name of the projects.

However, if @LucioFranco or any other behind the Tonic and Prost names is fine with it, I can definitely publish them in their current state. Though I need to cleanup a bit, and publish the lib fork, as crates.io won't let me publish a crate dependent on a git repo (which it is since that's a fork).

LucioFranco · 2022-02-11T18:32:28Z

However, if @LucioFranco or any other behind the Tonic and Prost names is fine with it, I can definitely publish them in their current state.

I have no issue with crates using prost-* or tonic-*. We can even link to some of these projects in the readme.

cab · 2022-02-11T19:34:38Z

@Tuetuopay quick correction just to avoid future confusion -- while I appreciate the shout-out, I am not a maintainer, and was just commenting on this PR as an interested user :)

Maher4Ever · 2022-03-14T16:36:15Z

@Tuetuopay Can I help in any way to get this PR and the one in tonic published to crates.io?

Tuetuopay · 2022-03-18T13:51:46Z

@Maher4Ever hi! There is not much to do, since it's mostly splitting stuff, and if I'm to do it I just need to get some free time. I'll have a bit in the coming weeks so I'll get around to it. (in fact, I even started!)

hexfusion · 2022-03-20T15:41:46Z

I am trying to understand why this is closed if it seems that it will be implemented. Can we track this in another way or reopen?

cc @LucioFranco @Tuetuopay

Tuetuopay · 2022-03-20T16:10:06Z

@hexfusion this is closed because most of the code of this PR will live in another repo and not this one. The idea for the next implementation is to keep the changeset in this repo minimal to get the lowest maintainance burden as possible for the prost maintainers.

I started the split few days ago, and so far the only required changes to prost-build are switching one or two functions to public (provided I rewrite instead of patching the write_include function since it does direct file I/O)

Tuetuopay · 2022-03-26T02:05:38Z

Rejoyce! I finally got time to port this, and I am happy to announce that I now have a protoc-gen-prost plugin that lives outside of this repository that is on par feature-wise with this PR! I'd even say that it is better: all options are supported, with a cleaner code, and better formatting than ever (it passes the generated code through prettyplease).

With the merge of #598 it will even not require any change to prost (although it still needs a git dep since it's unreleased yet). But as soon as this is released, the plugin can be pushed to crates.io. There are a few things that are left to do, mainly refacto a teeny bit to expose the internals as a lib, so that a future protoc-gen-tonic (my actual goal) can build on it. That'll be easy since I wrote the current code with this in mind.

Find it here: https://github.com/Tuetuopay/protoc-gen-prost

Tuetuopay · 2022-03-26T16:21:26Z

Aaaaaand now the Tonic plugin: https://github.com/Tuetuopay/protoc-gen-tonic

It does require one MR on tonic though, which is now open

Tuetuopay force-pushed the protoc-gen branch from 89187be to 12d0e93 Compare June 25, 2021 10:46

cab reviewed Jul 7, 2021

View reviewed changes

Tuetuopay force-pushed the protoc-gen branch from 34fffa7 to 2a97b84 Compare July 7, 2021 15:23

Tuetuopay force-pushed the protoc-gen branch from 2a97b84 to 3b134f8 Compare August 9, 2021 08:42

bufdev mentioned this pull request Aug 25, 2021

rust language bufbuild/buf#448

Closed

Tuetuopay force-pushed the protoc-gen branch from 616729e to f38cf4f Compare September 22, 2021 13:32

Tuetuopay force-pushed the protoc-gen branch from f38cf4f to 6108221 Compare September 22, 2021 14:18

GregBowyer mentioned this pull request Sep 24, 2021

FR: Add tonic/prost rules bazelbuild/rules_rust#915

Closed

aaliddell mentioned this pull request Sep 24, 2021

Add Rust Tonic and Prost support rules-proto-grpc/rules_proto_grpc#143

Closed

Tuetuopay force-pushed the protoc-gen branch from 8609524 to be75655 Compare September 29, 2021 13:48

LucioFranco mentioned this pull request Oct 13, 2021

implement a protoc plugin hyperium/tonic#750

Closed

Tuetuopay added 2 commits November 3, 2021 10:51

build: Add compilation from protoc CodeGeneratorRequest

ebde168

LucioFranco closed this Feb 11, 2022

LucioFranco mentioned this pull request Feb 24, 2022

How to compile protos using protoc directly #330

Closed

Tuetuopay mentioned this pull request Mar 26, 2022

feat(build): Expose Prost generation plugin hyperium/tonic#947

Merged

Maher4Ever mentioned this pull request Apr 7, 2022

Is this the repo for the offical protoc-gen-prost from crates.io? Tuetuopay/protoc-gen-prost#1

Closed

feat: Add protoc-gen-prost #492

feat: Add protoc-gen-prost #492

Conversation

Tuetuopay commented Jun 25, 2021 • edited Loading

Features

Motivation

Notes

Examples

Basic usage

Module generation

Crate generation

LucioFranco commented Jul 6, 2021

Tuetuopay commented Jul 6, 2021

cab Jul 7, 2021 • edited Loading

Choose a reason for hiding this comment

Tuetuopay Jul 7, 2021

Choose a reason for hiding this comment

Tuetuopay Jul 7, 2021 • edited Loading

Choose a reason for hiding this comment

Tuetuopay commented Aug 9, 2021

visortelle commented Aug 23, 2021

Tuetuopay commented Aug 23, 2021

henrikrudstrom commented Aug 25, 2021

LucioFranco commented Aug 26, 2021

GregBowyer commented Sep 11, 2021

Tuetuopay commented Sep 11, 2021

GregBowyer commented Sep 14, 2021

Tuetuopay commented Sep 14, 2021

Tuetuopay commented Sep 22, 2021 • edited Loading

Tuetuopay commented Sep 29, 2021

UebelAndre commented Oct 11, 2021

LucioFranco commented Oct 12, 2021

LucioFranco commented Oct 14, 2021

UebelAndre commented Nov 2, 2021 • edited Loading

UebelAndre commented Nov 22, 2021

UebelAndre commented Nov 29, 2021

Tuetuopay commented Nov 29, 2021

UebelAndre commented Dec 7, 2021

LucioFranco commented Dec 7, 2021

UebelAndre commented Dec 8, 2021

LucioFranco commented Dec 9, 2021

UebelAndre commented Dec 10, 2021

Maher4Ever commented Jan 6, 2022

Tuetuopay commented Jan 6, 2022

Maher4Ever commented Jan 7, 2022

UebelAndre commented Jan 28, 2022

Maher4Ever commented Feb 11, 2022 • edited Loading

LucioFranco commented Feb 11, 2022

Tuetuopay commented Feb 11, 2022

LucioFranco commented Feb 11, 2022

Maher4Ever commented Feb 11, 2022

Tuetuopay commented Feb 11, 2022

LucioFranco commented Feb 11, 2022

cab commented Feb 11, 2022

Maher4Ever commented Mar 14, 2022

Tuetuopay commented Mar 18, 2022

hexfusion commented Mar 20, 2022

Tuetuopay commented Mar 20, 2022

Tuetuopay commented Mar 26, 2022

Tuetuopay commented Mar 26, 2022

Tuetuopay commented Jun 25, 2021 •

edited

Loading

cab Jul 7, 2021 •

edited

Loading

Tuetuopay Jul 7, 2021 •

edited

Loading

Tuetuopay commented Sep 22, 2021 •

edited

Loading

UebelAndre commented Nov 2, 2021 •

edited

Loading

Maher4Ever commented Feb 11, 2022 •

edited

Loading