-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
proto: unify envoy_proto_library/api_proto_library. #4233
Changes from 1 commit
9bbc159
3c48acf
0d4384b
2b7be84
5515ef1
f440098
f848653
d921afe
f2394a1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,6 @@ | ||
load("@com_google_protobuf//:protobuf.bzl", "cc_proto_library", "py_proto_library") | ||
load("@com_lyft_protoc_gen_validate//bazel:pgv_proto_library.bzl", "pgv_cc_proto_library") | ||
load("@envoy_api//bazel:api_build_system.bzl", "api_proto_library") | ||
|
||
def envoy_package(): | ||
native.package(default_visibility = ["//visibility:public"]) | ||
|
@@ -398,54 +400,8 @@ def _proto_header(proto_path): | |
return None | ||
|
||
# Envoy proto targets should be specified with this function. | ||
def envoy_proto_library( | ||
name, | ||
srcs = [], | ||
deps = [], | ||
external_deps = [], | ||
generate_python = True): | ||
# Ideally this would be native.{proto_library, cc_proto_library}. | ||
# Unfortunately, this doesn't work with http_api_protos due to the PGV | ||
# requirement to also use them in the non-native protobuf.bzl | ||
# cc_proto_library; you end up with the same file built twice. So, also | ||
# using protobuf.bzl cc_proto_library here. | ||
cc_proto_deps = [] | ||
py_proto_deps = ["@com_google_protobuf//:protobuf_python"] | ||
|
||
if "api_httpbody_protos" in external_deps: | ||
cc_proto_deps.append("@googleapis//:api_httpbody_protos") | ||
py_proto_deps.append("@googleapis//:api_httpbody_protos_py") | ||
|
||
if "http_api_protos" in external_deps: | ||
cc_proto_deps.append("@googleapis//:http_api_protos") | ||
py_proto_deps.append("@googleapis//:http_api_protos_py") | ||
|
||
if "well_known_protos" in external_deps: | ||
# WKT is already included for Python as part of standard deps above. | ||
cc_proto_deps.append("@com_google_protobuf//:cc_wkt_protos") | ||
|
||
cc_proto_library( | ||
name = name, | ||
srcs = srcs, | ||
default_runtime = "@com_google_protobuf//:protobuf", | ||
protoc = "@com_google_protobuf//:protoc", | ||
deps = deps + cc_proto_deps, | ||
# Avoid generating .so, we don't need it, can interfere with builds | ||
# such as OSS-Fuzz. | ||
linkstatic = 1, | ||
alwayslink = 1, | ||
visibility = ["//visibility:public"], | ||
) | ||
|
||
if generate_python: | ||
py_proto_library( | ||
name = name + "_py", | ||
srcs = srcs, | ||
default_runtime = "@com_google_protobuf//:protobuf_python", | ||
protoc = "@com_google_protobuf//:protoc", | ||
deps = deps + py_proto_deps, | ||
visibility = ["//visibility:public"], | ||
) | ||
def envoy_proto_library(name, **kwargs): | ||
return api_proto_library(name, visibility = ["//visibility:public"], **kwargs) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think you need just make this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That doesn't quite work, because |
||
|
||
# Envoy proto descriptor targets should be specified with this function. | ||
# This is used for testing only. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,7 +60,7 @@ actions { | |
stream_action { | ||
stream_id: 0 | ||
response { | ||
continue_100_headers {} | ||
continue_headers {} | ||
} | ||
} | ||
} | ||
|
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.
Is this used?
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.
No, I will fix. But, now you have volunteered yourself as reviewer :)