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

Upgrade Protobuf to 3.19.1 and Guava to 30.1.1 #8748

Merged
merged 1 commit into from Dec 9, 2021

Conversation

ejona86
Copy link
Member

@ejona86 ejona86 commented Dec 8, 2021

Protobuf uses Guava 30.1.1, so I upgrade it at the same time. It also
caused an update to rules_jvm_external and reworking the Bazel build.
Protobuf no longer requires bind() so they were dropped. Although
Protobuf's protobuf_deps() brings in rules_jvm_external, and so we don't
need to define it ourselves, it seems better to define it directly and
not depend on transitive deps since we use it directly.

Protobuf now has support for maven_install() by exposing
PROTOBUF_MAVEN_ARTIFACTS, which required reorganizing the WORKSPACE to
use maven_install() after loading protobuf. Protobuf still doesn't
define target overrides for itself so we still maintain those. When
reorganizing the WORKSPACE I noticed http_archive should ideally be
above io_grpc_grpc_java as most users will need it there, so I fixed
that since there were lots of other load()-reordering already.

Protobuf uses Guava 30.1.1, so I upgrade it at the same time. It also
caused an update to rules_jvm_external and reworking the Bazel build.
Protobuf no longer requires bind() so they were dropped. Although
Protobuf's protobuf_deps() brings in rules_jvm_external, and so we don't
need to define it ourselves, it seems better to define it directly and
not depend on transitive deps since we use it directly.

Protobuf now has support for maven_install() by exposing
PROTOBUF_MAVEN_ARTIFACTS, which required reorganizing the WORKSPACE to
use maven_install() after loading protobuf. Protobuf still doesn't
define target overrides for itself so we still maintain those. When
reorganizing the WORKSPACE I noticed http_archive should ideally be
above io_grpc_grpc_java as most users will need it there, so I fixed
that since there were lots of other load()-reordering already.
@ejona86 ejona86 added the TODO:backport PR needs to be backported. Removed after backport complete label Dec 8, 2021
Copy link
Member

@dapengzhang0 dapengzhang0 left a comment

Choose a reason for hiding this comment

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

LGTM

@dapengzhang0
Copy link
Member

dapengzhang0 commented Dec 9, 2021

Resolves #8724

@ejona86
Copy link
Member Author

ejona86 commented Dec 9, 2021

Actually, #8724 doesn't need any effort from us to resolve. It is essential that users can upgrade protobuf as will, and we are not aware of any issues doing so (other than Maven being Maven).

@ejona86 ejona86 merged commit efd968b into grpc:master Dec 9, 2021
@ejona86 ejona86 deleted the protobuf-3.19.1 branch December 9, 2021 18:35
@ejona86 ejona86 removed the TODO:backport PR needs to be backported. Removed after backport complete label Dec 9, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants