Skip to content

Conversation

lizan
Copy link
Member

@lizan lizan commented Aug 3, 2018

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

Signed-off-by: Lizan Zhou <zlizan@google.com>
@htuch
Copy link
Member

htuch commented Aug 5, 2018

@lizan the CI tests fail, seems to be related to deps and envoy-filter-example. Can you investigate? Also, can you add to the commit comment any special motivation we have (e.g. is there a specific set of functions/features we're after, security rationale or just generally tracking latest?). Thanks.

@htuch htuch self-assigned this Aug 5, 2018
Signed-off-by: Lizan Zhou <zlizan@google.com>
@@ -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():
Copy link
Member Author

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?

Copy link
Member

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.

native.new_http_archive(
name = "com_github_nanopb_nanopb",
build_file = "@com_github_grpc_grpc//third_party:nanopb.BUILD",
strip_prefix = "nanopb-f8ac463766281625ad710900479130c7fcb4d63b",
Copy link
Contributor

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?

Copy link
Member

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.

@@ -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():
Copy link
Member

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.

native.new_http_archive(
name = "com_github_nanopb_nanopb",
build_file = "@com_github_grpc_grpc//third_party:nanopb.BUILD",
strip_prefix = "nanopb-f8ac463766281625ad710900479130c7fcb4d63b",
Copy link
Member

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>
@qiwzhang
Copy link
Contributor

qiwzhang commented Aug 7, 2018

LGTM

Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Rad.

@htuch htuch merged commit 797e824 into envoyproxy:master Aug 8, 2018
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.

3 participants