-
Notifications
You must be signed in to change notification settings - Fork 5.1k
deps: update gRPC to 1.14.0 #4047
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
Conversation
Signed-off-by: Lizan Zhou <zlizan@google.com>
@lizan the CI tests fail, seems to be related to deps and |
Signed-off-by: Lizan Zhou <zlizan@google.com>
bazel/repositories.bzl
Outdated
@@ -484,7 +484,20 @@ def _com_google_protobuf(): | |||
def _com_github_grpc_grpc(): | |||
_repository_impl("com_github_grpc_grpc") | |||
|
|||
if "com_github_nanopb_nanopb" not in native.existing_rules(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@htuch This is the reason for the previous CI error. I agree it looks kind of hacky but seems to be the best way to include it in Envoy today. Loading @com_google_grpc_grpc//bazel:grpc_deps.bzl
and call grpc_deps()
won't work at all in current structure we have in this file. Any better suggestion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is a gRPC C core hard dependency, we can add it as an external dep, sure. We do this for other gRPC deps below, e.g. cares
.
bazel/repositories.bzl
Outdated
native.new_http_archive( | ||
name = "com_github_nanopb_nanopb", | ||
build_file = "@com_github_grpc_grpc//third_party:nanopb.BUILD", | ||
strip_prefix = "nanopb-f8ac463766281625ad710900479130c7fcb4d63b", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we move SHA into repository_locations.bzl?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, ideally we do this.
bazel/repositories.bzl
Outdated
@@ -484,7 +484,20 @@ def _com_google_protobuf(): | |||
def _com_github_grpc_grpc(): | |||
_repository_impl("com_github_grpc_grpc") | |||
|
|||
if "com_github_nanopb_nanopb" not in native.existing_rules(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is a gRPC C core hard dependency, we can add it as an external dep, sure. We do this for other gRPC deps below, e.g. cares
.
bazel/repositories.bzl
Outdated
native.new_http_archive( | ||
name = "com_github_nanopb_nanopb", | ||
build_file = "@com_github_grpc_grpc//third_party:nanopb.BUILD", | ||
strip_prefix = "nanopb-f8ac463766281625ad710900479130c7fcb4d63b", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, ideally we do this.
Signed-off-by: Lizan Zhou <zlizan@google.com>
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rad.
Signed-off-by: Lizan Zhou zlizan@google.com
Description:
Pick up latest gRPC 1.14 for partially local credentials support, and keep update with latest stable generally.
Risk Level: Low
Testing: CI
Docs Changes: N/A
Release Notes: N/A