From 8b9c4d68dcd905f7d426e2c0cd97aba23f7541e4 Mon Sep 17 00:00:00 2001 From: Ivo List Date: Fri, 7 Feb 2025 15:44:28 +0100 Subject: [PATCH 1/3] feat: Remove and redirect py_proto_library to protobuf Protobuf team is taking ownership of `py_proto_library` and the implementation was moved to protobuf repository. Remove py_proto_library from rules_python, to prevent divergent implementations. Make a redirect with a deprecation warning, so that this doesn't break any users. Previously this was attempted in: https://github.com/bazelbuild/rules_python/commit/d0e25cfb41446e481da6e85f04ad0ac5bcf7ea80 --- CHANGELOG.md | 3 + MODULE.bazel | 2 +- WORKSPACE | 5 - examples/bzlmod/MODULE.bazel | 3 - .../py_proto_library/foo_external/BUILD.bazel | 4 +- .../foo_external/MODULE.bazel | 1 - internal_dev_deps.bzl | 7 - python/BUILD.bazel | 2 +- python/private/BUILD.bazel | 1 - python/private/proto/BUILD.bazel | 48 ---- python/private/proto/py_proto_library.bzl | 244 ------------------ python/proto.bzl | 6 +- 12 files changed, 10 insertions(+), 316 deletions(-) delete mode 100644 python/private/proto/BUILD.bazel delete mode 100644 python/private/proto/py_proto_library.bzl diff --git a/CHANGELOG.md b/CHANGELOG.md index 61000a1b08..7255e9ffcd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,9 @@ Unreleased changes template. {#v0-0-0-changed} ### Changed +* (rules) `py_proto_library` is deprecated in favour of the + implementation in https://github.com/protocolbuffers/protobuf. It will be + removed in the future release. * (pypi) {obj}`pip.override` will now be ignored instead of raising an error, fixes [#2550](https://github.com/bazelbuild/rules_python/issues/2550). * (rules) deprecation warnings for deprecated symbols have been turned off by diff --git a/MODULE.bazel b/MODULE.bazel index 89f1cd7961..76710e4ac4 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -10,7 +10,7 @@ bazel_dep(name = "rules_cc", version = "0.0.16") bazel_dep(name = "platforms", version = "0.0.4") # Those are loaded only when using py_proto_library -bazel_dep(name = "rules_proto", version = "7.0.2") +# Use py_proto_library directly from protobuf repository bazel_dep(name = "protobuf", version = "29.0-rc2", repo_name = "com_google_protobuf") internal_deps = use_extension("//python/private:internal_deps.bzl", "internal_deps") diff --git a/WORKSPACE b/WORKSPACE index 902af58ec8..b5605fc678 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -167,8 +167,3 @@ http_file( ], ) -# rules_proto expects //external:python_headers to point at the python headers. -bind( - name = "python_headers", - actual = "//python/cc:current_py_cc_headers", -) diff --git a/examples/bzlmod/MODULE.bazel b/examples/bzlmod/MODULE.bazel index d8535a0115..eaed078d63 100644 --- a/examples/bzlmod/MODULE.bazel +++ b/examples/bzlmod/MODULE.bazel @@ -12,9 +12,6 @@ local_path_override( path = "../..", ) -# (py_proto_library specific) We are using rules_proto to define rules_proto targets to be consumed by py_proto_library. -bazel_dep(name = "rules_proto", version = "6.0.0-rc1") - # (py_proto_library specific) Add the protobuf library for well-known types (e.g. `Any`, `Timestamp`, etc) bazel_dep(name = "protobuf", version = "27.0", repo_name = "com_google_protobuf") diff --git a/examples/bzlmod/py_proto_library/foo_external/BUILD.bazel b/examples/bzlmod/py_proto_library/foo_external/BUILD.bazel index 3fa22e06e7..183a3c28d2 100644 --- a/examples/bzlmod/py_proto_library/foo_external/BUILD.bazel +++ b/examples/bzlmod/py_proto_library/foo_external/BUILD.bazel @@ -1,5 +1,5 @@ -load("@rules_proto//proto:defs.bzl", "proto_library") -load("@rules_python//python:proto.bzl", "py_proto_library") +load("@com_google_protobuf//bazel:proto_library.bzl", "proto_library") +load("@com_google_protobuf//bazel:py_proto_library.bzl", "py_proto_library") load("@rules_python//python:py_binary.bzl", "py_binary") package(default_visibility = ["//visibility:public"]) diff --git a/examples/bzlmod/py_proto_library/foo_external/MODULE.bazel b/examples/bzlmod/py_proto_library/foo_external/MODULE.bazel index 5063f9b2d1..aca6f98eab 100644 --- a/examples/bzlmod/py_proto_library/foo_external/MODULE.bazel +++ b/examples/bzlmod/py_proto_library/foo_external/MODULE.bazel @@ -5,4 +5,3 @@ module( bazel_dep(name = "rules_python", version = "1.0.0") bazel_dep(name = "protobuf", version = "28.2", repo_name = "com_google_protobuf") -bazel_dep(name = "rules_proto", version = "7.0.2") diff --git a/internal_dev_deps.bzl b/internal_dev_deps.bzl index 0304fb16b7..cd33475f43 100644 --- a/internal_dev_deps.bzl +++ b/internal_dev_deps.bzl @@ -177,13 +177,6 @@ def rules_python_internal_deps(): ], ) - http_archive( - name = "rules_proto", - sha256 = "904a8097fae42a690c8e08d805210e40cccb069f5f9a0f6727cf4faa7bed2c9c", - strip_prefix = "rules_proto-6.0.0-rc1", - url = "https://github.com/bazelbuild/rules_proto/releases/download/6.0.0-rc1/rules_proto-6.0.0-rc1.tar.gz", - ) - http_archive( name = "com_google_protobuf", sha256 = "23082dca1ca73a1e9c6cbe40097b41e81f71f3b4d6201e36c134acc30a1b3660", diff --git a/python/BUILD.bazel b/python/BUILD.bazel index b747e2fbc7..5c6c6a4175 100644 --- a/python/BUILD.bazel +++ b/python/BUILD.bazel @@ -116,7 +116,7 @@ bzl_library( ], visibility = ["//visibility:public"], deps = [ - "//python/private/proto:py_proto_library_bzl", + "@com_google_protobuf//bazel:py_proto_library_bzl", ], ) diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel index 14f52c541b..2928dab068 100644 --- a/python/private/BUILD.bazel +++ b/python/private/BUILD.bazel @@ -31,7 +31,6 @@ filegroup( name = "distribution", srcs = glob(["**"]) + [ "//python/private/api:distribution", - "//python/private/proto:distribution", "//python/private/pypi:distribution", "//python/private/whl_filegroup:distribution", "//tools/build_defs/python/private:distribution", diff --git a/python/private/proto/BUILD.bazel b/python/private/proto/BUILD.bazel deleted file mode 100644 index dd53845638..0000000000 --- a/python/private/proto/BUILD.bazel +++ /dev/null @@ -1,48 +0,0 @@ -# Copyright 2022 The Bazel Authors. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -load("@bazel_skylib//:bzl_library.bzl", "bzl_library") -load("@com_google_protobuf//bazel/toolchains:proto_lang_toolchain.bzl", "proto_lang_toolchain") - -package(default_visibility = ["//visibility:private"]) - -licenses(["notice"]) - -filegroup( - name = "distribution", - srcs = glob(["**"]), - visibility = ["//python/private:__pkg__"], -) - -bzl_library( - name = "py_proto_library_bzl", - srcs = ["py_proto_library.bzl"], - visibility = ["//python:__pkg__"], - deps = [ - "//python:py_info_bzl", - "@com_google_protobuf//bazel/common:proto_common_bzl", - "@com_google_protobuf//bazel/common:proto_info_bzl", - "@rules_proto//proto:defs", - ], -) - -proto_lang_toolchain( - name = "python_toolchain", - command_line = "--python_out=%s", - progress_message = "Generating Python proto_library %{label}", - runtime = "@com_google_protobuf//:protobuf_python", - # NOTE: This isn't *actually* public. It's an implicit dependency of py_proto_library, - # so must be public so user usages of the rule can reference it. - visibility = ["//visibility:public"], -) diff --git a/python/private/proto/py_proto_library.bzl b/python/private/proto/py_proto_library.bzl deleted file mode 100644 index 1e9df848ab..0000000000 --- a/python/private/proto/py_proto_library.bzl +++ /dev/null @@ -1,244 +0,0 @@ -# Copyright 2022 The Bazel Authors. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -"""The implementation of the `py_proto_library` rule and its aspect.""" - -load("@com_google_protobuf//bazel/common:proto_common.bzl", "proto_common") -load("@com_google_protobuf//bazel/common:proto_info.bzl", "ProtoInfo") -load("//python:py_info.bzl", "PyInfo") -load("//python/api:api.bzl", _py_common = "py_common") - -PY_PROTO_TOOLCHAIN = "@rules_python//python/proto:toolchain_type" - -_PyProtoInfo = provider( - doc = "Encapsulates information needed by the Python proto rules.", - fields = { - "imports": """ - (depset[str]) The field forwarding PyInfo.imports coming from - the proto language runtime dependency.""", - "py_info": "PyInfo from proto runtime (or other deps) to propagate.", - "runfiles_from_proto_deps": """ - (depset[File]) Files from the transitive closure implicit proto - dependencies""", - "transitive_sources": """(depset[File]) The Python sources.""", - }, -) - -def _filter_provider(provider, *attrs): - return [dep[provider] for attr in attrs for dep in attr if provider in dep] - -def _incompatible_toolchains_enabled(): - return getattr(proto_common, "INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION", False) - -def _py_proto_aspect_impl(target, ctx): - """Generates and compiles Python code for a proto_library. - - The function runs protobuf compiler on the `proto_library` target generating - a .py file for each .proto file. - - Args: - target: (Target) A target providing `ProtoInfo`. Usually this means a - `proto_library` target, but not always; you must expect to visit - non-`proto_library` targets, too. - ctx: (RuleContext) The rule context. - - Returns: - ([_PyProtoInfo]) Providers collecting transitive information about - generated files. - """ - _proto_library = ctx.rule.attr - - # Check Proto file names - for proto in target[ProtoInfo].direct_sources: - if proto.is_source and "-" in proto.dirname: - fail("Cannot generate Python code for a .proto whose path contains '-' ({}).".format( - proto.path, - )) - - if _incompatible_toolchains_enabled(): - toolchain = ctx.toolchains[PY_PROTO_TOOLCHAIN] - if not toolchain: - fail("No toolchains registered for '%s'." % PY_PROTO_TOOLCHAIN) - proto_lang_toolchain_info = toolchain.proto - else: - proto_lang_toolchain_info = getattr(ctx.attr, "_aspect_proto_toolchain")[proto_common.ProtoLangToolchainInfo] - - py_common = _py_common.get(ctx) - py_info = py_common.PyInfoBuilder().merge_target( - proto_lang_toolchain_info.runtime, - ).build() - - api_deps = [proto_lang_toolchain_info.runtime] - - generated_sources = [] - proto_info = target[ProtoInfo] - proto_root = proto_info.proto_source_root - if proto_info.direct_sources: - # Generate py files - generated_sources = proto_common.declare_generated_files( - actions = ctx.actions, - proto_info = proto_info, - extension = "_pb2.py", - name_mapper = lambda name: name.replace("-", "_").replace(".", "/"), - ) - - # Handles multiple repository and virtual import cases - if proto_root.startswith(ctx.bin_dir.path): - proto_root = proto_root[len(ctx.bin_dir.path) + 1:] - - plugin_output = ctx.bin_dir.path + "/" + proto_root - - # Import path within the runfiles tree - if proto_root.startswith("external/"): - proto_root = proto_root[len("external") + 1:] - else: - proto_root = ctx.workspace_name + "/" + proto_root - - proto_common.compile( - actions = ctx.actions, - proto_info = proto_info, - proto_lang_toolchain_info = proto_lang_toolchain_info, - generated_files = generated_sources, - plugin_output = plugin_output, - ) - - # Generated sources == Python sources - python_sources = generated_sources - - deps = _filter_provider(_PyProtoInfo, getattr(_proto_library, "deps", [])) - runfiles_from_proto_deps = depset( - transitive = [dep[DefaultInfo].default_runfiles.files for dep in api_deps] + - [dep.runfiles_from_proto_deps for dep in deps], - ) - transitive_sources = depset( - direct = python_sources, - transitive = [dep.transitive_sources for dep in deps], - ) - - return [ - _PyProtoInfo( - imports = depset( - # Adding to PYTHONPATH so the generated modules can be - # imported. This is necessary when there is - # strip_import_prefix, the Python modules are generated under - # _virtual_imports. But it's undesirable otherwise, because it - # will put the repo root at the top of the PYTHONPATH, ahead of - # directories added through `imports` attributes. - [proto_root] if "_virtual_imports" in proto_root else [], - transitive = [dep[PyInfo].imports for dep in api_deps] + [dep.imports for dep in deps], - ), - runfiles_from_proto_deps = runfiles_from_proto_deps, - transitive_sources = transitive_sources, - py_info = py_info, - ), - ] - -_py_proto_aspect = aspect( - implementation = _py_proto_aspect_impl, - attrs = _py_common.API_ATTRS | ( - {} if _incompatible_toolchains_enabled() else { - "_aspect_proto_toolchain": attr.label( - default = ":python_toolchain", - ), - } - ), - attr_aspects = ["deps"], - required_providers = [ProtoInfo], - provides = [_PyProtoInfo], - toolchains = [PY_PROTO_TOOLCHAIN] if _incompatible_toolchains_enabled() else [], -) - -def _py_proto_library_rule(ctx): - """Merges results of `py_proto_aspect` in `deps`. - - Args: - ctx: (RuleContext) The rule context. - Returns: - ([PyInfo, DefaultInfo, OutputGroupInfo]) - """ - if not ctx.attr.deps: - fail("'deps' attribute mustn't be empty.") - - pyproto_infos = _filter_provider(_PyProtoInfo, ctx.attr.deps) - default_outputs = depset( - transitive = [info.transitive_sources for info in pyproto_infos], - ) - - py_common = _py_common.get(ctx) - - py_info = py_common.PyInfoBuilder() - py_info.set_has_py2_only_sources(False) - py_info.set_has_py3_only_sources(False) - py_info.transitive_sources.add(default_outputs) - py_info.imports.add([info.imports for info in pyproto_infos]) - py_info.merge_all([ - pyproto_info.py_info - for pyproto_info in pyproto_infos - ]) - return [ - DefaultInfo( - files = default_outputs, - default_runfiles = ctx.runfiles(transitive_files = depset( - transitive = - [default_outputs] + - [info.runfiles_from_proto_deps for info in pyproto_infos], - )), - ), - OutputGroupInfo( - default = depset(), - ), - py_info.build(), - ] - -py_proto_library = rule( - implementation = _py_proto_library_rule, - doc = """ - Use `py_proto_library` to generate Python libraries from `.proto` files. - - The convention is to name the `py_proto_library` rule `foo_py_pb2`, - when it is wrapping `proto_library` rule `foo_proto`. - - `deps` must point to a `proto_library` rule. - - Example: - -```starlark -py_library( - name = "lib", - deps = [":foo_py_pb2"], -) - -py_proto_library( - name = "foo_py_pb2", - deps = [":foo_proto"], -) - -proto_library( - name = "foo_proto", - srcs = ["foo.proto"], -) -```""", - attrs = { - "deps": attr.label_list( - doc = """ - The list of `proto_library` rules to generate Python libraries for. - - Usually this is just the one target: the proto library of interest. - It can be any target providing `ProtoInfo`.""", - providers = [ProtoInfo], - aspects = [_py_proto_aspect], - ), - } | _py_common.API_ATTRS, - provides = [PyInfo], -) diff --git a/python/proto.bzl b/python/proto.bzl index 3f455aee58..2ea9bdb153 100644 --- a/python/proto.bzl +++ b/python/proto.bzl @@ -11,11 +11,11 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - """ Python proto library. """ -load("//python/private/proto:py_proto_library.bzl", _py_proto_library = "py_proto_library") +load("@com_google_protobuf//bazel:py_proto_library.bzl", _py_proto_library = "py_proto_library") -py_proto_library = _py_proto_library +def py_proto_library(*, deprecation = "Use py_proto_library from protobuf repository", **kwargs): + _py_proto_library(deprecation = deprecation, **kwargs) From d96ebb85528ebcef3a26e72069f7f1b03dc1dd23 Mon Sep 17 00:00:00 2001 From: Ivo List Date: Fri, 7 Feb 2025 16:04:23 +0100 Subject: [PATCH 2/3] Format WORKSPACE --- WORKSPACE | 1 - 1 file changed, 1 deletion(-) diff --git a/WORKSPACE b/WORKSPACE index b5605fc678..b97411e2d5 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -166,4 +166,3 @@ http_file( "https://files.pythonhosted.org/packages/50/67/3e966d99a07d60a21a21d7ec016e9e4c2642a86fea251ec68677daf71d4d/numpy-1.25.2-cp311-cp311-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", ], ) - From 78f418f586f487b436f2a9ab18b6bea3d31e9331 Mon Sep 17 00:00:00 2001 From: Ivo List Date: Fri, 7 Feb 2025 16:07:07 +0100 Subject: [PATCH 3/3] Disable external_import_test --- examples/bzlmod/py_proto_library/BUILD.bazel | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/examples/bzlmod/py_proto_library/BUILD.bazel b/examples/bzlmod/py_proto_library/BUILD.bazel index 24436b48ea..175589fbf9 100644 --- a/examples/bzlmod/py_proto_library/BUILD.bazel +++ b/examples/bzlmod/py_proto_library/BUILD.bazel @@ -20,11 +20,12 @@ py_test( # Regression test for https://github.com/bazelbuild/rules_python/issues/2515 # -# This test failed before https://github.com/bazelbuild/rules_python/pull/2516 +# This test fails before protobuf 30.0 release # when ran with --legacy_external_runfiles=False (default in Bazel 8.0.0). native_test( name = "external_import_test", src = "@foo_external//:py_binary_with_proto", + tags = ["manual"], # TODO: reenable when com_google_protobuf is upgraded # Incompatible with Windows: native_test wrapping a py_binary doesn't work # on Windows. target_compatible_with = select({