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

Update prost to 0.8 #544

Merged
merged 3 commits into from
Sep 25, 2021
Merged

Update prost to 0.8 #544

merged 3 commits into from
Sep 25, 2021

Conversation

pingyu
Copy link
Contributor

@pingyu pingyu commented Sep 13, 2021

Update Prost to 0.8, to avoid security issue of 0.7 (RUSTSEC-2021-0073).
Related issue: tikv/tikv#10905, tikv/protobuf-build#55

Note that grpcio-proto still depends on prost 0.7 because of its build-dependency to protobuf-build 0.12. As protobuf-build 0.12 depends on grpcio-compiler 0.9, there is a circular dependency between grps-rs and protobuf-build.

I think we should release grps-rs 0.10.0 first, then protobuf-build 0.13, and at last update grpcio-proto to depend on protobuf-build 0.13.

Signed-off-by: pingyu shui.yu@126.com

@sticnarf
Copy link
Contributor

Well, I think it is okay to release protobuf-build 0.13 first because it allows all recent grpcio-compiler versions.

Then, we can release v0.10 of grpc-rs and grpcio-compiler together.

@BusyJay What do you think?

@BusyJay
Copy link
Member

BusyJay commented Sep 17, 2021

Well, I think it is okay to release protobuf-build 0.13 first because it allows all recent grpcio-compiler versions.

Then, we can release v0.10 of grpc-rs and grpcio-compiler together.

Agree. In long term, we better land #545 to solve the problem completely.

Cargo.toml Outdated
@@ -1,6 +1,6 @@
[package]
name = "grpcio"
version = "0.9.0"
version = "0.10.0"
Copy link
Member

Choose a reason for hiding this comment

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

Better keep the old version. We will send a standalone PR to mutate the version and update changelog.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment addressed. And change version of depending protobuf-build to >=0.12, to use the coming 0.13 version without modification again.
PTAL~

BTW, why better keep the old version ?

Copy link
Member

Choose a reason for hiding this comment

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

Because we may not just include one change in next minor version. This is also friendly for people who want to try the change using patch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. Thanks~

@BusyJay
Copy link
Member

BusyJay commented Sep 18, 2021

Is there any risk to upgrade to 0.8? I can see a comment here: tokio-rs/prost#526 (comment).

@pymongo
Copy link

pymongo commented Sep 19, 2021

Is there any risk to upgrade to 0.8? I can see a comment here: tokio-rs/prost#526 (comment).

prost 0.8 Timestamp need TryInto to convert to std::time::SystemTime, before 0.7 can use Into trait to convert

but I think this convert is safe on Linux in prost 0.8

prost <= 0.7 is compile failed on rustc >= 1.56.0

Signed-off-by: pingyu <shui.yu@126.com>
Signed-off-by: pingyu <shui.yu@126.com>
@BusyJay BusyJay merged commit b0ad175 into tikv:master Sep 25, 2021
christian-oudard pushed a commit to mobilecoinofficial/grpc-rs that referenced this pull request Oct 20, 2021
Signed-off-by: pingyu <shui.yu@126.com>
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.

5 participants