Skip to content

Commit

Permalink
fix(py_runtime): make py_runtime_pair return builtin PyRuntimeInfo un…
Browse files Browse the repository at this point in the history
…der Bazel 6; make python_bootstrap_template public (#1611)

This fixes two bugs from
#1574:
* Bazel 6's py_binary rejects the rules_python Starlark implemented
PyRuntimeInfo.
* python_bootstrap_template.txt isn't public; this prevents py_runtime
from being
  used outside rules_python itself (e.g. in the python toolchain repos)

With Bazel 6, the `py_binary` rule performs a type check of the
PyRuntimeInfo value it gets from the toolchain to verify it is an
instance of the Java-implemented PyRuntimeInfo class. This type check
fails when the provider is implemented in rules_python in Starlark.

To fix, make the `py_runtime_info` prefer the builtin PyRuntimeInfo
provider when running under Bazel 6. The two providers are (currently)
the same, so are mostly interchangable. This satisfies the type check
that `py_binary` performs.

`py_runtime` as an implicit dependency on
`//python/private:python_bootstrap_template.txt`,
but that target is only visible to rules python itself. This means the
py_runtime targets
created in the toolchain repos fail. To fix, make the file public.

Fixes #1610
  • Loading branch information
rickeylev authored Dec 14, 2023
1 parent e7d5c8d commit bbec4c2
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 1 deletion.
7 changes: 7 additions & 0 deletions python/private/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,13 @@ exports_files(
visibility = ["//:__subpackages__"],
)

exports_files(
["python_bootstrap_template.txt"],
# Not actually public. Only public because it's an implicit dependency of
# py_runtime.
visibility = ["//visibility:public"],
)

# Used to determine the use of `--stamp` in Starlark rules
stamp_build_setting(name = "stamp")

Expand Down
7 changes: 6 additions & 1 deletion python/private/py_runtime_pair_rule.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

load("//python:py_runtime_info.bzl", "PyRuntimeInfo")
load("//python/private:reexports.bzl", "BuiltinPyRuntimeInfo")
load("//python/private:util.bzl", "IS_BAZEL_7_OR_HIGHER")

def _py_runtime_pair_impl(ctx):
if ctx.attr.py2_runtime != None:
Expand Down Expand Up @@ -45,7 +46,11 @@ def _py_runtime_pair_impl(ctx):
)]

def _get_py_runtime_info(target):
if PyRuntimeInfo in target:
# Prior to Bazel 7, the builtin PyRuntimeInfo object must be used because
# py_binary (implemented in Java) performs a type check on the provider
# value to verify it is an instance of the Java-implemented PyRuntimeInfo
# class.
if IS_BAZEL_7_OR_HIGHER and PyRuntimeInfo in target:
return target[PyRuntimeInfo]
else:
return target[BuiltinPyRuntimeInfo]
Expand Down
41 changes: 41 additions & 0 deletions tests/py_runtime_pair/py_runtime_pair_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ load("@rules_testing//lib:analysis_test.bzl", "analysis_test")
load("@rules_testing//lib:test_suite.bzl", "test_suite")
load("@rules_testing//lib:truth.bzl", "matching", "subjects")
load("@rules_testing//lib:util.bzl", rt_util = "util")
load("//python:py_binary.bzl", "py_binary")
load("//python:py_runtime.bzl", "py_runtime")
load("//python:py_runtime_pair.bzl", "py_runtime_pair")
load("//python/private:reexports.bzl", "BuiltinPyRuntimeInfo") # buildifier: disable=bzl-visibility
Expand Down Expand Up @@ -99,6 +100,46 @@ def _test_builtin_py_info_accepted_impl(env, target):

_tests.append(_test_builtin_py_info_accepted)

def _test_py_runtime_pair_and_binary(name):
rt_util.helper_target(
py_runtime,
name = name + "_runtime",
interpreter_path = "/fake_interpreter",
python_version = "PY3",
)
rt_util.helper_target(
py_runtime_pair,
name = name + "_pair",
py3_runtime = name + "_runtime",
)
native.toolchain(
name = name + "_toolchain",
toolchain = name + "_pair",
toolchain_type = "//python:toolchain_type",
)
rt_util.helper_target(
py_binary,
name = name + "_subject",
srcs = [name + "_subject.py"],
)
analysis_test(
name = name,
target = name + "_subject",
impl = _test_py_runtime_pair_and_binary_impl,
config_settings = {
"//command_line_option:extra_toolchains": [
"//tests/py_runtime_pair:{}_toolchain".format(name),
"//tests/cc:all",
],
},
)

def _test_py_runtime_pair_and_binary_impl(env, target):
# Building indicates success, so nothing to assert
_ = env, target # @unused

_tests.append(_test_py_runtime_pair_and_binary)

def py_runtime_pair_test_suite(name):
test_suite(
name = name,
Expand Down

0 comments on commit bbec4c2

Please sign in to comment.