From fa32eb36b3c39ce596f075d471c35eb28a344aab Mon Sep 17 00:00:00 2001 From: Florian Ferstl <64520639+ferstlf@users.noreply.github.com> Date: Mon, 12 Jun 2023 23:36:24 +0200 Subject: [PATCH] [bazel] Auto-generate .pyi files in py_proto_library rule (#32872) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With some delay, this is a PR for https://github.com/grpc/grpc/issues/32564 (and previously https://github.com/grpc/grpc/pull/31791). I looked into adding a regular `py_test` for this change [as suggested](https://github.com/grpc/grpc/pull/31791#issuecomment-1423245116) but I am not aware of any effect that the presence of a .pyi stub file would have at runtime and where some sort of type-checking in a .py script would be affected. Stub files are only for use by type checkers & IDE's. I mean, something like this would work: ``` import helloworld_pb2 py_file = helloworld_pb2.__file__ pyi_file = py_file + 'i’ self.assertTrue(os.path.exists(pyi_file)) ``` But that seems really hacky to me. Instead I created a simple rule test for `py_proto_library` with Bazel Skylib which tests the declared outputs for an example `py_proto_library` target. Indirectly, this also tests that the declared output files are actually generated. Please let me know if this is sufficient. --- bazel/grpc_deps.bzl | 6 +- bazel/python_rules.bzl | 5 +- src/python/.gitignore | 1 + test/distrib/bazel/python/BUILD | 5 ++ test/distrib/bazel/python/WORKSPACE | 4 ++ .../bazel/python/python_rules_test.bzl | 70 +++++++++++++++++++ 6 files changed, 87 insertions(+), 4 deletions(-) create mode 100644 test/distrib/bazel/python/python_rules_test.bzl diff --git a/bazel/grpc_deps.bzl b/bazel/grpc_deps.bzl index 2fb3564d0c4..bd60253f172 100644 --- a/bazel/grpc_deps.bzl +++ b/bazel/grpc_deps.bzl @@ -362,10 +362,10 @@ def grpc_deps(): http_archive( name = "bazel_skylib", urls = [ - "https://mirror.bazel.build/github.com/bazelbuild/bazel-skylib/releases/download/1.0.2/bazel-skylib-1.0.2.tar.gz", - "https://github.com/bazelbuild/bazel-skylib/releases/download/1.0.2/bazel-skylib-1.0.2.tar.gz", + "https://mirror.bazel.build/github.com/bazelbuild/bazel-skylib/releases/download/1.0.3/bazel-skylib-1.0.3.tar.gz", + "https://github.com/bazelbuild/bazel-skylib/releases/download/1.0.3/bazel-skylib-1.0.3.tar.gz", ], - sha256 = "97e70364e9249702246c0e9444bccdc4b847bed1eb03c5a3ece4f83dfe6abc44", + sha256 = "1c531376ac7e5a180e0237938a2536de0c54d93f5c278634818e0efc952dd56c", ) if "bazel_compdb" not in native.existing_rules(): diff --git a/bazel/python_rules.bzl b/bazel/python_rules.bzl index 97d97360585..648af98e68c 100644 --- a/bazel/python_rules.bzl +++ b/bazel/python_rules.bzl @@ -28,6 +28,7 @@ load( ) _GENERATED_PROTO_FORMAT = "{}_pb2.py" +_GENERATED_PROTO_STUB_FORMAT = "{}_pb2.pyi" _GENERATED_GRPC_PROTO_FORMAT = "{}_pb2_grpc.py" PyProtoInfo = provider( @@ -59,7 +60,8 @@ def _gen_py_aspect_impl(target, context): protos.append(get_staged_proto_file(target.label, context, p)) includes = depset(direct = protos, transitive = [target[ProtoInfo].transitive_imports]) - out_files = declare_out_files(protos, context, _GENERATED_PROTO_FORMAT) + out_files = (declare_out_files(protos, context, _GENERATED_PROTO_FORMAT) + + declare_out_files(protos, context, _GENERATED_PROTO_STUB_FORMAT)) generated_py_srcs = out_files tools = [context.executable._protoc] @@ -68,6 +70,7 @@ def _gen_py_aspect_impl(target, context): arguments = ([ "--python_out={}".format(out_dir.path), + "--pyi_out={}".format(out_dir.path), ] + [ "--proto_path={}".format(get_include_directory(i)) for i in includes.to_list() diff --git a/src/python/.gitignore b/src/python/.gitignore index 41813129bdb..095ab8bbae1 100644 --- a/src/python/.gitignore +++ b/src/python/.gitignore @@ -1,4 +1,5 @@ gens/ *_pb2.py +*_pb2.pyi *_pb2_grpc.py *.egg-info/ diff --git a/test/distrib/bazel/python/BUILD b/test/distrib/bazel/python/BUILD index 127a43568c8..74b69249d02 100644 --- a/test/distrib/bazel/python/BUILD +++ b/test/distrib/bazel/python/BUILD @@ -20,6 +20,7 @@ load( "py_grpc_library", "py_proto_library", ) +load("//:python_rules_test.bzl", "python_rules_test_suite") package( default_testonly = 1, @@ -155,3 +156,7 @@ py_test( ":helloworld_py_pb2", ], ) + +python_rules_test_suite( + name = "python_rules_test", +) diff --git a/test/distrib/bazel/python/WORKSPACE b/test/distrib/bazel/python/WORKSPACE index 3ffc4a4b2d8..04c41ccca8e 100644 --- a/test/distrib/bazel/python/WORKSPACE +++ b/test/distrib/bazel/python/WORKSPACE @@ -16,6 +16,10 @@ load("@com_github_grpc_grpc//bazel:grpc_extra_deps.bzl", "grpc_extra_deps") grpc_extra_deps() +load("@bazel_skylib//:workspace.bzl", "bazel_skylib_workspace") + +bazel_skylib_workspace() + local_repository( name = "some_other_repo", path = "../python_second_test_repo", diff --git a/test/distrib/bazel/python/python_rules_test.bzl b/test/distrib/bazel/python/python_rules_test.bzl new file mode 100644 index 00000000000..afb35c8edf8 --- /dev/null +++ b/test/distrib/bazel/python/python_rules_test.bzl @@ -0,0 +1,70 @@ +# Copyright 2021 The gRPC Authors +# +# 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. +"""Bazel rule tests of bazel/python_rules.bzl""" + +load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") + +def _assert_in(env, item, container): + asserts.true( + env, + item in container, + "Expected " + str(item) + " to be in " + str(container), + ) + +# Tests the declared outputs of the 'py_proto_library' rule and, indirectly, also tests that +# these outputs are actually generated (building ":helloworld_py_pb2" will fail if not all of +# the declared output files are actually generated). +def _py_proto_library_provider_contents_test_impl(ctx): + env = analysistest.begin(ctx) + + target = analysistest.target_under_test(env) + + files = [file.short_path for file in target.files.to_list()] + runfiles = [file.short_path for file in target.default_runfiles.files.to_list()] + py_info_transitive_sources = [ + file.short_path + for file in target[PyInfo].transitive_sources.to_list() + ] + + _assert_in(env, "helloworld_pb2.py", files) + _assert_in(env, "helloworld_pb2.pyi", files) + _assert_in(env, "subdir/hello_dep_pb2.py", files) + _assert_in(env, "subdir/hello_dep_pb2.pyi", files) + + _assert_in(env, "helloworld_pb2.py", runfiles) + _assert_in(env, "helloworld_pb2.pyi", runfiles) + _assert_in(env, "subdir/hello_dep_pb2.py", runfiles) + _assert_in(env, "subdir/hello_dep_pb2.pyi", runfiles) + + _assert_in(env, "helloworld_pb2.py", py_info_transitive_sources) + _assert_in(env, "helloworld_pb2.pyi", py_info_transitive_sources) + _assert_in(env, "subdir/hello_dep_pb2.py", py_info_transitive_sources) + _assert_in(env, "subdir/hello_dep_pb2.pyi", py_info_transitive_sources) + + return analysistest.end(env) + +_py_proto_library_provider_contents_test = analysistest.make(_py_proto_library_provider_contents_test_impl) + +def python_rules_test_suite(name): + _py_proto_library_provider_contents_test( + name = "py_proto_library_provider_contents_test", + target_under_test = ":helloworld_py_pb2", + ) + + native.test_suite( + name = name, + tests = [ + "py_proto_library_provider_contents_test", + ], + )