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

protoc-gen-tonic: compression feature always on #6

Closed
barakmich opened this issue May 19, 2022 · 5 comments
Closed

protoc-gen-tonic: compression feature always on #6

barakmich opened this issue May 19, 2022 · 5 comments

Comments

@barakmich
Copy link

First of all, thank you so much for making these -- this is the right way to do protobuf generation across a bigger/multi-language world, as opposed to via Rust's build.rs!

I filed tokio-rs/console#349 over on tokio's console-api because I'm in sort of a catch-22. Because I generated my protos with protoc-gen-tonic, they have the "compression" feature enabled which emits, in the generation, fields that require my library consuming them to have "compression" enabled as well.

That's fine if it's just me, but because console-api has their own generated protos with compression disabled, I get trouble when I try to compile console-subscriber into my application because I've already got tonic and it enables "compression" which breaks their generated protos' expectations.

IMO, they should just have a more general proto committed. But I figured I'd send an issue along in case other folks see it, or to see if there's a convenient way to enable or disable the compression features from the generated tonic files.

@neoeinstein
Copy link
Owner

This is one that I'll need to bring to tonic too. We may want to make compression disabled here by default, but with compression being a compilation feature, I have a hard time making it into an option.

I have a thought about how I might make that work, so let me see if I can do some dependency magic to make compression optional.

@barakmich
Copy link
Author

That'd be fabulous! But yeah, it does seem like it goes to the tonic level too. compression looks like a sort of non-standard feature flag in tonic that explicitly expects you to be generating these things dynamically. Which is bad when you want to pre-generate with other tools 😢

@neoeinstein
Copy link
Owner

Sadly my dependency hacking wasn’t successful right off, but I may have an angle to get compression out of tonic-build so that the generated code doesn’t care about it, but you can get it or not through the tonic compression flag. May require a SemVer bump in tonic, but should make it so that intermediates can check in generated code without causing this type of breakage (and leave the decision to include the compression machinery to the end application).

@tatemz
Copy link
Contributor

tatemz commented Jul 21, 2022

I suppose these compilation errors I get are related? Is there a workaround?

error[E0412]: cannot find type `EnabledCompressionEncodings` in this scope
   --> __generated/proto/rs/schema/src/foobar.services.v1.tonic.rs:556:37
    |
556 |         send_compression_encodings: EnabledCompressionEncodings,
    |                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^ not found in this scope

error[E0283]: type annotations needed
   --> __generated/proto/rs/schema/src/foobar.services.v1.tonic.rs:207:47
    |
207 |                 accept_compression_encodings: Default::default(),
    |                                               ^^^^^^^^^^^^^^^^ cannot infer type

@tatemz
Copy link
Contributor

tatemz commented Jul 21, 2022

Answering my own question. Turns out this is needed in the app Cargo.toml:

[dependencies.tonic]
version = "0.7.2"
features = ["compression"]

bors bot added a commit that referenced this issue Aug 8, 2022
23: chore(deps): Update prost, tonic and pbjson dependencies. r=neoeinstein a=o-agassizii

tonic 0.8.0 was recently released. Said version of tonic depends on an updated version of prost. Likewise, pbjson has recently been updated upstream to depend on the latest version of prost.

See also:

* hyperium/tonic#1004
* #6 (referenced in the above link)

Co-authored-by: Opisthoteuthis agassizii <[email protected]>
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

No branches or pull requests

3 participants