Starlark-based py_runtime with Bazel 6 doesn't work

[rickeylev]

🐞 bug report

Affected Rule

py_runtime

Is this a regression?

Yes. In older rules_python versions, the rules_python py_runtime symbol can be used. After #1574, one has to use the Bazel builtin symbol instead.

Description

Using Bazel 6 and the rules_python Starlark based py_runtime doesn't work. This was caused by #1574. It went unnoticed because that PR missed converting the py_runtime call in the hermetic toolchain setup to use rules_python symbol. There should probably be an explicit test for this, too.

There are two bugs:

The first is py_runtime has an implicit dependency on python_bootstrap_template.txt, but that file is only visible to rules_python itself. This means the toolchain repos can't reference the file, resulting in a visibility error.

The is a bit convoluted to explain due to the overloading of terms and several layers involved.

Basically, in Bazel 6, py_binary validates that the py3_runtime value it gets from the toolchain is an instance of PyRuntimeInfo. This is possible because, in Bazel 6, py_binary is implemented in Java, so it's checking that the object is an instance of the Java PyRuntimeInfo class that implements the Starlark PyRuntimeInfo type.

Under Bazel 7, this isn't an issue, because py_binary is implemented in Starlark and that type checking is gone.

🔬 Minimal Reproduction

A minimal reproduction requires setting up a toolchain and having a py_binary use it. The validation occurs during py_binary figuring out its toolchain details.

load("@rules_python//python:py_runtime.bzl", "py_runtime")
load("@rules_python//python:py_runtime_pair.bzl", "py_runtime_pair")
load("@rules_python//python:py_binary.bzl", "py_binary")

py_runtime(name = "runtime", interpreter_path="/fake", python_version="PY3")
py_runtime_pair(name = "pair", py3_runtime=":runtime")
toolchain(name="tc", toolchain = ":pair", toolchain_type="@rules_python//python:toolchain_type")
py_binary(name="foo", srcs=["foo.py"])

# CLI
bazel build //:foo --extra_toolchains=//:tc

🔥 Exception or Error

ERROR: /home/...: in py_binary rule ...: Error parsing the Python toolchain's ToolchainInfo: Expected a PyRuntimeInfo in field 'py3_runtime', but got 'struct'

There's some more to the error message, but that's the core part. That error is being generated by this code: https://github.com/bazelbuild/bazel/blob/6.4.0/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java#L435

[rickeylev]

I think fixing this should be pretty easy. I think all we really have to do is make py_runtime_pair extract the builtin PyRuntimeInfo object from py_runtime when Bazel 6 is used. Right now it always prefers the rules_python Starlark version.

Worst case, we just switch py_runtime back to using the bazel builtin version when bazel 6 is being used.

[snakethatlovesstaticlibs]

This might not be related to this ticket, but with a bazel project using a WORKSPACE file, I'm trying to add the reproduction to a BUILD file:

load("@rules_python//python:py_runtime.bzl", "py_runtime")
load("@rules_python//python:py_runtime_pair.bzl", "py_runtime_pair")
load("@rules_python//python:py_binary.bzl", "py_binary")

It looks like it's failing because of a visibility issue. Am I using it right?

ERROR: /srv/x/bazel/thirdparty/BUILD:12:11: in py_runtime rule //bazel/thirdparty:python3_runtime: target '@rules_python//python/private:python_bootstrap_template.txt' is not visible from target '//bazel/thirdparty:python3_runtime'. Check the visibility declaration of the former target if you think the dependency is legitimate. To set the visibility of that source file target, use the exports_files() function

The same occurs even if I do

load("@rules_python//python:defs.bzl", "py_runtime")
load("@rules_python//python:defs.bzl", "py_runtime_pair")
load("@rules_python//python:defs.bzl", "py_binary")

as the docs seem to suggest. I'm on bazel 6.2.1 and commit e7d5c8db6dccbea2c1a8ee731f9a472edee680cf

[rickeylev]

Yes, thank you for mentioning that. I ran across that, too, but forgot about it. The same PR is responsible for that.