revert: upgrade to protobuf 27.0 and remove py_proto_library (bazel-contrib#1933) (bazel-contrib#1948)

aignas · web-flow · commit 8b0eaed84198 · 2024-06-11T05:35:48.000Z
This reverts commit d0e25cf. We have the following concerns with the associated change: - `sphinxdocs` is excluded entirely. Because protobuf fails to compile? But why? Protos are needed as part of our docgen, but it is unclear how the docgen is still working. Maybe we can get some clarification here? - The `py_proto_library` tests in the `bzlmod` example are excluded from CI. Because protos also fail to compile? But why? It's an example and should Just Work. - Adding the `copts` needs to be done by all downstream users now. But fixing that in protobuf blocks removing legacy struct providers? I don't understand the connection. Also @alexeagle noted extra [regression](bazel-contrib#1933 (comment)) being caused by the associated PR. Reverting in order to unblock a new release of `rules_python` and then we can work together with @comius on reverting the revert. Reverts bazel-contrib#1933
diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml
@@ -147,14 +147,6 @@ tasks:
     <<: *reusable_config
     name: "Default: Debian"
     platform: debian11
-    build_flags:
-      # For protobuf compilation
-      - '--host_copt=-Wno-deprecated-declarations'
-      - '--copt=-Wno-deprecated-declarations'
-    test_flags:
-      # For protobuf compilation
-      - '--host_copt=-Wno-deprecated-declarations'
-      - '--copt=-Wno-deprecated-declarations'
   macos:
     <<: *reusable_config
     name: "Default: MacOS"
@@ -177,39 +169,19 @@ tasks:
       # build kite cc toolchain.
       - "--extra_toolchains=@buildkite_config//config:cc-toolchain"
       - "--build_tag_filters=-docs"
-    build_targets:
-      - "--"
-      - "..."
-      - '-//sphinxdocs/...' # protobuf compilation fails
     test_flags:
       - "--test_tag_filters=-integration-test,-acceptance-test,-docs"
       # BazelCI sets --action_env=BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN=1,
       # which prevents cc toolchain autodetection from working correctly
       # on Bazel 5.4 and earlier. To workaround this, manually specify the
       # build kite cc toolchain.
       - "--extra_toolchains=@buildkite_config//config:cc-toolchain"
-    test_targets:
-      - "--"
-      - "..."
-      - '-//sphinxdocs/...' # protobuf compilation fails
   rbe:
     <<: *reusable_config
     name: "RBE: Ubuntu"
     platform: rbe_ubuntu1604
-    build_flags:
-      - "--build_tag_filters=-docs"
-    build_targets:
-      - "--"
-      - "..."
-      - '-//sphinxdocs/...' # protobuf compilation fails
-      - '-//docs/...'
     test_flags:
-      - "--test_tag_filters=-integration-test,-acceptance-test,-docs"
-    test_targets:
-      - "--"
-      - "..."
-      - '-//sphinxdocs/...' # protobuf compilation fails
-      - '-//docs/...'
+      - "--test_tag_filters=-integration-test,-acceptance-test"
 
   integration_test_build_file_generation_ubuntu_minimum_supported_workspace:
     <<: *minimum_supported_version
@@ -262,21 +234,6 @@ tasks:
     name: "examples/bzlmod: Debian"
     working_directory: examples/bzlmod
     platform: debian11
-    build_targets:
-      # For protobuf compilation
-      - "--"
-      - "..."
-      - "-//py_proto_library/..."
-    test_targets:
-      # For protobuf compilation
-      - "--"
-      - "..."
-      - "-//py_proto_library/..."
-    coverage_targets:
-      # For protobuf compilation
-      - "--"
-      - "..."
-      - "-//py_proto_library/..."
   integration_test_bzlmod_macos:
     <<: *reusable_build_test_all
     <<: *coverage_targets_example_bzlmod
@@ -438,14 +395,6 @@ tasks:
     name: "examples/py_proto_library: Debian, workspace"
     working_directory: examples/py_proto_library
     platform: debian11
-    build_flags:
-      # For protobuf compilation
-      - '--host_copt=-Wno-deprecated-declarations'
-      - '--copt=-Wno-deprecated-declarations'
-    test_flags:
-      # For protobuf compilation
-      - '--host_copt=-Wno-deprecated-declarations'
-      - '--copt=-Wno-deprecated-declarations'
   integration_test_py_proto_library_macos_workspace:
     <<: *reusable_build_test_all
     <<: *common_workspace_flags
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -25,9 +25,6 @@ A brief description of the categories of changes:
 [x.x.x]: https://github.com/bazelbuild/rules_python/releases/tag/x.x.x
 
 ### 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.
 * (deps) Upgrade the `pip_install` dependencies to pick up a new version of pip.
 * (toolchains) Optional toolchain dependency: `py_binary`, `py_test`, and
   `py_library` now depend on the `//python:exec_tools_toolchain_type` for build
diff --git a/MODULE.bazel b/MODULE.bazel
@@ -9,9 +9,9 @@ bazel_dep(name = "bazel_skylib", version = "1.6.1")
 bazel_dep(name = "rules_cc", version = "0.0.9")
 bazel_dep(name = "platforms", version = "0.0.4")
 
-# For backwards compatibility, those are loaded only when using py_proto_library
-# Use py_proto_library directly from protobuf repository
-bazel_dep(name = "protobuf", version = "27.0", repo_name = "com_google_protobuf")
+# Those are loaded only when using py_proto_library
+bazel_dep(name = "rules_proto", version = "6.0.0-rc1")
+bazel_dep(name = "protobuf", version = "21.7", repo_name = "com_google_protobuf")
 
 internal_deps = use_extension("//python/private/bzlmod:internal_deps.bzl", "internal_deps")
 use_repo(
diff --git a/WORKSPACE b/WORKSPACE
@@ -148,3 +148,9 @@ http_file(
         "https://files.pythonhosted.org/packages/50/67/3e966d99a07d60a21a21d7ec016e9e4c2642a86fea251ec68677daf71d4d/numpy-1.25.2-cp311-cp311-manylinux_2_17_aarch64.manylinux2014_aarch64.whl",
     ],
 )
+
+# 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/WORKSPACE.bzlmod b/WORKSPACE.bzlmod
@@ -55,3 +55,8 @@ 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
@@ -11,8 +11,11 @@ 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 = "5.3.0-21.7")
+
 # (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")
+bazel_dep(name = "protobuf", version = "21.7", repo_name = "com_google_protobuf")
 
 # We next initialize the python toolchain using the extension.
 # You can set different Python versions in this block.
diff --git a/examples/bzlmod/py_proto_library/example.com/another_proto/BUILD.bazel b/examples/bzlmod/py_proto_library/example.com/another_proto/BUILD.bazel
@@ -1,5 +1,5 @@
-load("@com_google_protobuf//bazel:proto_library.bzl", "proto_library")
-load("@com_google_protobuf//bazel:py_proto_library.bzl", "py_proto_library")
+load("@rules_proto//proto:defs.bzl", "proto_library")
+load("@rules_python//python:proto.bzl", "py_proto_library")
 
 py_proto_library(
     name = "message_proto_py_pb2",
diff --git a/examples/bzlmod/py_proto_library/example.com/proto/BUILD.bazel b/examples/bzlmod/py_proto_library/example.com/proto/BUILD.bazel
@@ -1,5 +1,5 @@
-load("@com_google_protobuf//bazel:proto_library.bzl", "proto_library")
-load("@com_google_protobuf//bazel:py_proto_library.bzl", "py_proto_library")
+load("@rules_proto//proto:defs.bzl", "proto_library")
+load("@rules_python//python:proto.bzl", "py_proto_library")
 
 py_proto_library(
     name = "pricetag_proto_py_pb2",
diff --git a/examples/py_proto_library/WORKSPACE b/examples/py_proto_library/WORKSPACE
@@ -24,11 +24,24 @@ python_register_toolchains(
 # Then we need to setup dependencies in order to use py_proto_library
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 
+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",
+)
+
+load("@rules_proto//proto:repositories.bzl", "rules_proto_dependencies", "rules_proto_toolchains")
+
+rules_proto_dependencies()
+
+rules_proto_toolchains()
+
 http_archive(
     name = "com_google_protobuf",
-    sha256 = "da288bf1daa6c04d03a9051781caa52aceb9163586bff9aa6cfb12f69b9395aa",
-    strip_prefix = "protobuf-27.0",
-    urls = ["https://github.com/protocolbuffers/protobuf/releases/download/v27.0/protobuf-27.0.tar.gz"],
+    sha256 = "4fc5ff1b2c339fb86cd3a25f0b5311478ab081e65ad258c6789359cd84d421f8",
+    strip_prefix = "protobuf-26.1",
+    urls = ["https://github.com/protocolbuffers/protobuf/archive/v26.1.tar.gz"],
 )
 
 load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")
diff --git a/examples/py_proto_library/example.com/another_proto/BUILD.bazel b/examples/py_proto_library/example.com/another_proto/BUILD.bazel
@@ -1,5 +1,5 @@
-load("@com_google_protobuf//bazel:proto_library.bzl", "proto_library")
-load("@com_google_protobuf//bazel:py_proto_library.bzl", "py_proto_library")
+load("@rules_proto//proto:defs.bzl", "proto_library")
+load("@rules_python//python:proto.bzl", "py_proto_library")
 
 py_proto_library(
     name = "message_proto_py_pb2",
diff --git a/examples/py_proto_library/example.com/proto/BUILD.bazel b/examples/py_proto_library/example.com/proto/BUILD.bazel
@@ -1,5 +1,5 @@
-load("@com_google_protobuf//bazel:proto_library.bzl", "proto_library")
-load("@com_google_protobuf//bazel:py_proto_library.bzl", "py_proto_library")
+load("@rules_proto//proto:defs.bzl", "proto_library")
+load("@rules_python//python:proto.bzl", "py_proto_library")
 
 py_proto_library(
     name = "pricetag_proto_py_pb2",
diff --git a/internal_deps.bzl b/internal_deps.bzl
@@ -164,12 +164,20 @@ 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 = "da288bf1daa6c04d03a9051781caa52aceb9163586bff9aa6cfb12f69b9395aa",
-        strip_prefix = "protobuf-27.0",
+        sha256 = "75be42bd736f4df6d702a0e4e4d30de9ee40eac024c4b845d17ae4cc831fe4ae",
+        strip_prefix = "protobuf-21.7",
         urls = [
-            "https://github.com/protocolbuffers/protobuf/releases/download/v27.0/protobuf-27.0.tar.gz",
+            "https://mirror.bazel.build/github.com/protocolbuffers/protobuf/archive/v21.7.tar.gz",
+            "https://github.com/protocolbuffers/protobuf/archive/v21.7.tar.gz",
         ],
     )
 
diff --git a/internal_setup.bzl b/internal_setup.bzl
@@ -20,6 +20,7 @@ load("@cgrindel_bazel_starlib//:deps.bzl", "bazel_starlib_dependencies")
 load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")
 load("@rules_bazel_integration_test//bazel_integration_test:deps.bzl", "bazel_integration_test_rules_dependencies")
 load("@rules_bazel_integration_test//bazel_integration_test:repo_defs.bzl", "bazel_binaries")
+load("@rules_proto//proto:repositories.bzl", "rules_proto_dependencies", "rules_proto_toolchains")
 load("//:version.bzl", "SUPPORTED_BAZEL_VERSIONS")
 load("//python/pip_install:repositories.bzl", "pip_install_dependencies")
 load("//python/private:internal_config_repo.bzl", "internal_config_repo")  # buildifier: disable=bzl-visibility
@@ -34,6 +35,9 @@ def rules_python_internal_setup():
 
     bazel_skylib_workspace()
 
+    rules_proto_dependencies()
+    rules_proto_toolchains()
+
     protobuf_deps()
 
     bazel_integration_test_rules_dependencies()
diff --git a/python/BUILD.bazel b/python/BUILD.bazel
@@ -113,7 +113,7 @@ bzl_library(
     ],
     visibility = ["//visibility:public"],
     deps = [
-        "@com_google_protobuf//bazel:py_proto_library_bzl",
+        "//python/private/proto:py_proto_library_bzl",
     ],
 )
 
diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel
@@ -29,6 +29,7 @@ filegroup(
     srcs = glob(["**"]) + [
         "//python/private/bzlmod:distribution",
         "//python/private/common:distribution",
+        "//python/private/proto: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
@@ -0,0 +1,46 @@
+# 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("@rules_proto//proto:defs.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:defs_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
diff --git a/python/proto.bzl b/python/proto.bzl

Original file line number	Diff line number	Diff line change
`@@ -55,3 +55,8 @@ http_file(`
`55`	`55`	`],`
`56`	`56`	`)`
`57`	`57`
	`58`	`+# rules_proto expects //external:python_headers to point at the python headers.`
	`59`	`+bind(`
	`60`	`+ name = "python_headers",`
	`61`	`+ actual = "//python/cc:current_py_cc_headers",`
	`62`	`+)`
Original file line number	Diff line number	Diff line change
`@@ -113,7 +113,7 @@ bzl_library(`
`113`	`113`	`],`
`114`	`114`	`visibility = ["//visibility:public"],`
`115`	`115`	`deps = [`
`116`		`- "@com_google_protobuf//bazel:py_proto_library_bzl",`
	`116`	`+ "//python/private/proto:py_proto_library_bzl",`
`117`	`117`	`],`
`118`	`118`	`)`
`119`	`119`