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

Protobuf Version #302

Open
brpatel12 opened this issue Apr 13, 2020 · 13 comments
Open

Protobuf Version #302

brpatel12 opened this issue Apr 13, 2020 · 13 comments

Comments

@brpatel12
Copy link

Hi from the cargo.toml file it looks like this is using protobuf 1.6.0. Is there any way this can be updated? I need some features which have recently been implemented so that I can use the Timestamp type in a proto message declaration. Let me know if you have any advice, thanks!

@colin353
Copy link
Collaborator

I agree that we should bump protobuf, I will look into doing this

@damienmg
Copy link
Collaborator

Hi, the proto toolchain used by the repository does not need to be used by downstream dependency. See https://github.com/bazelbuild/rules_rust/tree/master/proto#custom-deps on how to use a custom version of protobuf.

That being said, there is no reason to not update if you want to sent a PR :)

@colin353
Copy link
Collaborator

I bumped the protobuf+grpc to newer versions in this PR, which seems to be passing tests: #306

@dfreese
Copy link
Collaborator

dfreese commented Apr 22, 2020

#310 bumped protobuf to 2.8.2, but for compatibility with grpc-rust's latest release (0.6.2), I wasn't able to get as close to master as you were hoping @Bergie1994

@UebelAndre
Copy link
Collaborator

Has anything changed in the last few months? Would the upgrade work now?

@UebelAndre
Copy link
Collaborator

Hey, friendly ping here 😅

@damienmg
Copy link
Collaborator

damienmg commented Dec 1, 2020

@UebelAndre You need to look at the grpc-rust crates to see the protobuf compatibility, this was the blocker last time and it is not under our control.

@UebelAndre
Copy link
Collaborator

UebelAndre commented Dec 1, 2020

@damienmg right but how do I identify the compatible version? Are you referring to https://crates.io/crates/grpc ? I can't seem to find any mention of a compatible protobuf version

@depthwise
Copy link

depthwise commented Jul 10, 2021

Then there's the issue of the author of grpc crate saying that it's not good enough for prod. Under the circumstances it's questionable if grpc rules should even use that crate. Seems to me like prost/tonic would be more desirable, and there's a stalled PR for that here. Maybe instead of fixing these it'd be more profitable to spend the time getting tonic to work.

@UebelAndre
Copy link
Collaborator

Then there's the issue of the author of grpc crate saying that it's not good enough for prod. Under the circumstances it's questionable if grpc rules should even use that crate. Seems to me like prost/tonic would be more desirable, and there's a stalled PR for that here. Maybe instead of fixing these it'd be more profitable to spend the time getting tonic to work.

I personally agree that it'd be better to focus on getting the tonic PR working but have heard some folks say they prefer the grpc crate. Either way, I don't think the maintainers have bandwidth to drive changes to the proto package. Any changes would require some passionate soul to submit a well tested and documented PR.

I do think if tonic/prost rules could be written that didn't violate any package boundaries then it would have a higher chance of making it in (not that the current draft does, just saying). It might be easier to start a discussion associated with the draft to make questions are more visible.

Anyhow... Hope that information answers some questions 😅

@depthwise
Copy link

Thing is, though, if grpc crate is the default option and it has significant flaws (I couldn't find any info on why the author thinks the crate is inadequate though, so maybe that's outdated info), people are going to use it, and encounter those flaws. At a minimum the docs should mention that grpc rules are not fit for production use, since they will pull in the grpc crate.

@UebelAndre
Copy link
Collaborator

That's a great point. If you'd be willing to submit a PR to update the docs, I'd be happy to review, though I think this is really @dfreese domain 😅 interested to hear their thoughts

@rdelfin
Copy link

rdelfin commented Jul 13, 2021

I'd be glad to pick up the Tonic PR mentioned earlier. It's in the last legs and, frankly, just needs a bit more work to get out the door. The main issue seems to be with forcing specific dependencies on users, and I put a few proposed ideas on the PR. Thoughts would be appareciated

github-merge-queue bot pushed a commit that referenced this issue Jul 12, 2024
This PR provides documentation of Bazelmod and several code examples
that addresses a number of issues related to Bazelmod.

Preview of the documentation:
https://github.com/marvin-hansen/rules_rust/blob/main/docs/crate_universe_bzlmod.md

First and foremost it paves the way for a meaningful update the Bazelmod
documentation that references these and existing code examples. This
touches at least the following issues:
* #2670
* #2181


The compile_opt example addresses or resolves:
*  #515
* #2701

The musl_cross_compilling example addresses or resolves
* #390 
* #276

The oci_container does not relate to any open issue, 
although the tokio example in it gives a nice end to end example so 
this definitely helps those looking for something non-trivial.

The proto example addresses or resolves:
*  #2668
*  #302
* #2534
* Possibly a few more if I were to search longer

Formalities
* I've signed the CLA
* I've signed all commits

---------

Signed-off-by: Marvin Hansen <[email protected]>
Co-authored-by: Daniel Wagner-Hall <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants