Skip to content

Commit

Permalink
bazel: Migrate py_proto_library to @com_github_grpc_grpc (#74)
Browse files Browse the repository at this point in the history
py_proto_library provided by @com_google_protobuf//:protobuf.bzl has been deprecated for a while now:

This is provided for backwards compatibility only. Bazel 5.3 will
introduce support for py_proto_library, which should be used instead.
https://github.com/protocolbuffers/protobuf/blob/32af7d211b85f71920acdfa9b796db008f8c2a45/protobuf.bzl#L642-L644

However, native py_proto_library has never been provided, see bazelbuild/bazel#3935. Instead @rules_python//python:proto.bzl is recommended. I attempted switching to this library, but it's not compatible with @com_google_googleapis's py_proto_library targets, see #69. I found a hacky workaround by using cc_proto_library to generate python targets, but downstream integration into Envoy failed (envoyproxy/envoy#30159).

This PR migrates py_proto_library implementation to to @com_github_grpc_grpc. This implementation is used by @com_google_googleapis's, and, more importantly, uses bazel aspects. Which decouples cncf/xds and Envoy's dependencies from concrete upstream py_proto_library implementations.

This resulted in a significant code improvement of bazel/api_build_system.bzl:

No more custom @com_google_googleapis dependency mapping needed via py_proto_library rules override.
No more hardcoded dependencies _xds_py_proto_library - proto dependency tree is determined from proto_library definitions via Basel aspects.
No more EXTERNAL_PROTO_PY_BAZEL_DEP_MAP dependency map needed - for the same reason.
Similar work in Envoy: envoyproxy/envoy#30834.

Signed-off-by: Sergii Tkachenko <[email protected]>
  • Loading branch information
sergiitk committed Nov 16, 2023
1 parent 523115e commit 3a472e5
Show file tree
Hide file tree
Showing 39 changed files with 71 additions and 111 deletions.
72 changes: 10 additions & 62 deletions bazel/api_build_system.bzl
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
load("@com_envoyproxy_protoc_gen_validate//bazel:pgv_proto_library.bzl", "pgv_cc_proto_library")
load("@com_google_protobuf//:protobuf.bzl", _py_proto_library = "py_proto_library")
load("@com_github_grpc_grpc//bazel:python_rules.bzl", _py_proto_library = "py_proto_library")
load("@io_bazel_rules_go//go:def.bzl", "go_test")
load("@io_bazel_rules_go//proto:def.bzl", "go_grpc_library", "go_proto_library")
load("@rules_proto//proto:defs.bzl", "proto_library")
load(
"//bazel:external_proto_deps.bzl",
"EXTERNAL_PROTO_CC_BAZEL_DEP_MAP",
"EXTERNAL_PROTO_GO_BAZEL_DEP_MAP",
"EXTERNAL_PROTO_PY_BAZEL_DEP_MAP",
)

_PY_PROTO_SUFFIX = "_py_proto"
Expand Down Expand Up @@ -43,65 +42,6 @@ def _go_proto_mapping(dep):
def _cc_proto_mapping(dep):
return _proto_mapping(dep, EXTERNAL_PROTO_CC_BAZEL_DEP_MAP, _CC_PROTO_SUFFIX)

def _py_proto_mapping(dep):
return _proto_mapping(dep, EXTERNAL_PROTO_PY_BAZEL_DEP_MAP, _PY_PROTO_SUFFIX)

# TODO(htuch): Convert this to native py_proto_library once
# https://github.com/bazelbuild/bazel/issues/3935 and/or
# https://github.com/bazelbuild/bazel/issues/2626 are resolved.
def _xds_py_proto_library(name, srcs = [], deps = []):
mapped_deps = [_py_proto_mapping(dep) for dep in deps]
mapped_unique_deps = []
[mapped_unique_deps.append(d) for d in mapped_deps if d not in mapped_unique_deps]
_py_proto_library(
name = name + _PY_PROTO_SUFFIX,
srcs = srcs,
default_runtime = "@com_google_protobuf//:protobuf_python",
protoc = "@com_google_protobuf//:protoc",
deps = mapped_unique_deps + [
"@com_envoyproxy_protoc_gen_validate//validate:validate_py",
"@com_google_googleapis//google/rpc:status_py_proto",
"@com_google_googleapis//google/api:annotations_py_proto",
"@com_google_googleapis//google/api:http_py_proto",
"@com_google_googleapis//google/api:httpbody_py_proto",
],
visibility = ["//visibility:public"],
)

# This defines googleapis py_proto_library. The repository does not provide its definition and requires
# overriding it in the consuming project (see https://github.com/grpc/grpc/issues/19255 for more details).
def py_proto_library(name, deps = [], plugin = None):
srcs = [dep[:-6] + ".proto" if dep.endswith("_proto") else dep for dep in deps]
proto_deps = []

# py_proto_library in googleapis specifies *_proto rules in dependencies.
# By rewriting *_proto to *.proto above, the dependencies in *_proto rules are not preserved.
# As a workaround, manually specify the proto dependencies for the imported python rules.
if name == "annotations_py_proto":
proto_deps = proto_deps + [":http_py_proto"]

# checked.proto depends on syntax.proto, we have to add this dependency manually as well.
if name == "checked_py_proto":
proto_deps = proto_deps + [":syntax_py_proto"]

# Special handling for expr_proto target
if srcs[0] == ":expr_moved.proto":
srcs = ["checked.proto", "eval.proto", "explain.proto", "syntax.proto", "value.proto",]
proto_deps = proto_deps + ["@com_google_googleapis//google/rpc:status_py_proto"]


# py_proto_library does not support plugin as an argument yet at gRPC v1.25.0:
# https://github.com/grpc/grpc/blob/v1.25.0/bazel/python_rules.bzl#L72.
# plugin should also be passed in here when gRPC version is greater than v1.25.x.
_py_proto_library(
name = name,
srcs = srcs,
default_runtime = "@com_google_protobuf//:protobuf_python",
protoc = "@com_google_protobuf//:protoc",
deps = proto_deps + ["@com_google_protobuf//:protobuf_python"],
visibility = ["//visibility:public"],
)

def _xds_cc_py_proto_library(
name,
visibility = ["//visibility:private"],
Expand Down Expand Up @@ -129,7 +69,15 @@ def _xds_cc_py_proto_library(
deps = [relative_name],
visibility = ["//visibility:public"],
)
_xds_py_proto_library(name, srcs, deps)

# Uses gRPC implementation of py_proto_library.
# https://github.com/grpc/grpc/blob/v1.59.1/bazel/python_rules.bzl#L160
_py_proto_library(
name = name + _PY_PROTO_SUFFIX,
# Actual dependencies are resolved automatically from the proto_library dep tree.
deps = [relative_name],
visibility = ["//visibility:public"],
)

# Optionally define gRPC services
if has_services:
Expand Down
15 changes: 12 additions & 3 deletions bazel/dependency_imports.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,34 @@ load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies", "go_repository")
load("@com_google_googleapis//:repository_rules.bzl", "switched_rules_by_language")
load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies")
load("@com_envoyproxy_protoc_gen_validate//bazel:repositories.bzl", "pgv_dependencies")
load("@rules_proto//proto:repositories.bzl", "rules_proto_dependencies", "rules_proto_toolchains")

# go version for rules_go
GO_VERSION = "1.20.2"

def xds_dependency_imports(go_version = GO_VERSION):
rules_proto_dependencies()
rules_proto_toolchains()
protobuf_deps()
go_rules_dependencies()
go_register_toolchains(go_version)
gazelle_dependencies()
pgv_dependencies()

# Needed for grpc's @com_github_grpc_grpc//bazel:python_rules.bzl
# Used in place of calling grpc_deps() because it needs to be called before
# loading `grpc_extra_deps.bzl` - which is not allowed in a method def context.
native.bind(
name = "protocol_compiler",
actual = "@com_google_protobuf//:protoc",
)

switched_rules_by_language(
name = "com_google_googleapis_imports",
cc = True,
go = True,
python = True,
grpc = True,
rules_override = {
"py_proto_library": ["@com_github_cncf_xds//bazel:api_build_system.bzl", "",],
},
)

# These dependencies, like most of the Go in this repository, exist only for the API.
Expand Down
6 changes: 0 additions & 6 deletions bazel/external_proto_deps.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,3 @@ EXTERNAL_PROTO_CC_BAZEL_DEP_MAP = {
"@com_google_googleapis//google/api/expr/v1alpha1:checked_proto": "@com_google_googleapis//google/api/expr/v1alpha1:checked_cc_proto",
"@com_google_googleapis//google/api/expr/v1alpha1:syntax_proto": "@com_google_googleapis//google/api/expr/v1alpha1:syntax_cc_proto",
}

# This maps from the Bazel proto_library target to the Python language binding target for external dependencies.
EXTERNAL_PROTO_PY_BAZEL_DEP_MAP = {
"@com_google_googleapis//google/api/expr/v1alpha1:checked_proto": "@com_google_googleapis//google/api/expr/v1alpha1:expr_py_proto",
"@com_google_googleapis//google/api/expr/v1alpha1:syntax_proto": "@com_google_googleapis//google/api/expr/v1alpha1:expr_py_proto",
}
4 changes: 4 additions & 0 deletions bazel/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ def xds_api_dependencies():
"io_bazel_rules_go",
locations = REPOSITORY_LOCATIONS,
)
xds_http_archive(
"rules_proto",
locations = REPOSITORY_LOCATIONS,
)

# Old name for backward compatibility.
# TODO(roth): Remove once all callers are updated to use the new name.
Expand Down
17 changes: 11 additions & 6 deletions bazel/repository_locations.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ REPOSITORY_LOCATIONS = dict(
urls = ["https://github.com/envoyproxy/protoc-gen-validate/archive/refs/tags/v0.6.1.tar.gz"],
),
com_github_grpc_grpc = dict(
sha256 = "13e7c6460cd979726e5b3b129bb01c34532f115883ac696a75eb7f1d6a9765ed",
strip_prefix = "grpc-1.40.0",
urls = ["https://github.com/grpc/grpc/archive/refs/tags/v1.40.0.tar.gz"],
sha256 = "916f88a34f06b56432611aaa8c55befee96d0a7b7d7457733b9deeacbc016f99",
strip_prefix = "grpc-1.59.1",
urls = ["https://github.com/grpc/grpc/archive/refs/tags/v1.59.1.tar.gz"],
),
com_google_googleapis = dict(
# TODO(dio): Consider writing a Skylark macro for importing Google API proto.
Expand All @@ -24,9 +24,9 @@ REPOSITORY_LOCATIONS = dict(
urls = ["https://github.com/googleapis/googleapis/archive/114a745b2841a044e98cdbb19358ed29fcf4a5f1.tar.gz"],
),
com_google_protobuf = dict(
sha256 = "52b6160ae9266630adb5e96a9fc645215336371a740e87d411bfb63ea2f268a0",
strip_prefix = "protobuf-3.18.0",
urls = ["https://github.com/protocolbuffers/protobuf/releases/download/v3.18.0/protobuf-all-3.18.0.tar.gz"],
sha256 = "8242327e5df8c80ba49e4165250b8f79a76bd11765facefaaecfca7747dc8da2",
strip_prefix = "protobuf-3.21.5",
urls = ["https://github.com/protocolbuffers/protobuf/archive/v3.21.5.zip"],
),
io_bazel_rules_go = dict(
sha256 = "6dc2da7ab4cf5d7bfc7c949776b1b7c733f05e56edc4bcd9022bb249d2e2a996",
Expand All @@ -35,4 +35,9 @@ REPOSITORY_LOCATIONS = dict(
"https://github.com/bazelbuild/rules_go/releases/download/v0.39.1/rules_go-v0.39.1.zip",
],
),
rules_proto = dict(
sha256 = "80d3a4ec17354cccc898bfe32118edd934f851b03029d63ef3fc7c8663a7415c",
strip_prefix = "rules_proto-5.3.0-21.5",
urls = ["https://github.com/bazelbuild/rules_proto/archive/refs/tags/5.3.0-21.5.tar.gz",],
),
)
2 changes: 1 addition & 1 deletion go/udpa/annotations/migrate.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/udpa/annotations/security.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/udpa/annotations/sensitive.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/udpa/annotations/status.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/udpa/annotations/versioning.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/udpa/data/orca/v1/orca_load_report.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/udpa/service/orca/v1/orca.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/udpa/type/v1/typed_struct.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/xds/annotations/v3/migrate.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/xds/annotations/v3/security.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/xds/annotations/v3/sensitive.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/xds/annotations/v3/status.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/xds/annotations/v3/versioning.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/xds/core/v3/authority.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/xds/core/v3/cidr.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/xds/core/v3/collection_entry.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/xds/core/v3/context_params.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/xds/core/v3/extension.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/xds/core/v3/resource.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/xds/core/v3/resource_locator.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/xds/core/v3/resource_name.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/xds/data/orca/v3/orca_load_report.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/xds/service/orca/v3/orca.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 3a472e5

Please sign in to comment.