fix(PyRuntimeInfo): use builtin PyRuntimeInfo unless pystar is enabled. (#1748)
This fixes the bug where the PyRuntimeInfo symbol rules_python exported
wasn't matching the provider identity that `py_binary` actually
provided.
The basic problem was, when pystar is disabled:
* PyRuntimeInfo is the rules_python defined provider
* py_binary is `native.py_binary`, which provides only the builtin
PyRuntimeInfo provider.
Thus, when users loaded the rules_python PyRuntimeInfo symbol, it was
referring to a provider that the underlying py_binary didn't actually
provide. Pystar is always disabled on Bazel 6.4,
and enabling it for Bazel 7 will happen eminently.
This typically showed up when users had a custom rule with an attribute
definition that used the rules_python PyRuntimeInfo.
To fix, only use the rules_python define PyRuntimeInfo if pystar is
enabled. This ensures the providers the underlying rules are providing
match the symbols that rules_python is exported.
* Also fixes `py_binary` and `py_test` to also return the builtin
PyRuntimeInfo. This
should make the transition from the builtin symbols to the rules_python
symbols a bit
easier.
Fixes https://github.com/bazelbuild/rules_python/issues/1732
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5923c5e..6338e9c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -48,6 +48,11 @@
* (toolchain) Changed the `host_toolchain` to symlink all files to support
Windows host environments without symlink support.
+* (PyRuntimeInfo) Switch back to builtin PyRuntimeInfo for Bazel 6.4 and when
+ pystar is disabled. This fixes an error about `target ... does not have ...
+ PyRuntimeInfo`.
+ ([#1732](https://github.com/bazelbuild/rules_python/issues/1732))
+
### Added
* (py_wheel) Added `requires_file` and `extra_requires_files` attributes.
diff --git a/python/private/common/py_executable.bzl b/python/private/common/py_executable.bzl
index a0720c1..410fb1d 100644
--- a/python/private/common/py_executable.bzl
+++ b/python/private/common/py_executable.bzl
@@ -14,6 +14,7 @@
"""Common functionality between test/binary executables."""
load("@bazel_skylib//lib:dicts.bzl", "dicts")
+load("//python/private:reexports.bzl", "BuiltinPyRuntimeInfo")
load(
":attributes.bzl",
"AGNOSTIC_EXECUTABLE_ATTRS",
@@ -771,7 +772,28 @@
# TODO(b/265840007): Make this non-conditional once Google enables
# --incompatible_use_python_toolchains.
if runtime_details.toolchain_runtime:
- providers.append(runtime_details.toolchain_runtime)
+ py_runtime_info = runtime_details.toolchain_runtime
+ providers.append(py_runtime_info)
+
+ # Re-add the builtin PyRuntimeInfo for compatibility to make
+ # transitioning easier, but only if it isn't already added because
+ # returning the same provider type multiple times is an error.
+ # NOTE: The PyRuntimeInfo from the toolchain could be a rules_python
+ # PyRuntimeInfo or a builtin PyRuntimeInfo -- a user could have used the
+ # builtin py_runtime rule or defined their own. We can't directly detect
+ # the type of the provider object, but the rules_python PyRuntimeInfo
+ # object has an extra attribute that the builtin one doesn't.
+ if hasattr(py_runtime_info, "interpreter_version_info"):
+ providers.append(BuiltinPyRuntimeInfo(
+ interpreter_path = py_runtime_info.interpreter_path,
+ interpreter = py_runtime_info.interpreter,
+ files = py_runtime_info.files,
+ coverage_tool = py_runtime_info.coverage_tool,
+ coverage_files = py_runtime_info.coverage_files,
+ python_version = py_runtime_info.python_version,
+ stub_shebang = py_runtime_info.stub_shebang,
+ bootstrap_template = py_runtime_info.bootstrap_template,
+ ))
# TODO(b/163083591): Remove the PyCcLinkParamsProvider once binaries-in-deps
# are cleaned up.
diff --git a/python/py_runtime_info.bzl b/python/py_runtime_info.bzl
index c0a9288..e88e0c0 100644
--- a/python/py_runtime_info.bzl
+++ b/python/py_runtime_info.bzl
@@ -14,8 +14,8 @@
"""Public entry point for PyRuntimeInfo."""
+load("@rules_python_internal//:rules_python_config.bzl", "config")
load("//python/private:reexports.bzl", "BuiltinPyRuntimeInfo")
-load("//python/private:util.bzl", "IS_BAZEL_6_OR_HIGHER")
load("//python/private/common:providers.bzl", _starlark_PyRuntimeInfo = "PyRuntimeInfo")
-PyRuntimeInfo = _starlark_PyRuntimeInfo if IS_BAZEL_6_OR_HIGHER else BuiltinPyRuntimeInfo
+PyRuntimeInfo = _starlark_PyRuntimeInfo if config.enable_pystar else BuiltinPyRuntimeInfo
diff --git a/tests/base_rules/py_executable_base_tests.bzl b/tests/base_rules/py_executable_base_tests.bzl
index 3960579..ce86eca 100644
--- a/tests/base_rules/py_executable_base_tests.bzl
+++ b/tests/base_rules/py_executable_base_tests.bzl
@@ -13,6 +13,7 @@
# limitations under the License.
"""Tests common to py_binary and py_test (executable rules)."""
+load("@rules_python//python:py_runtime_info.bzl", RulesPythonPyRuntimeInfo = "PyRuntimeInfo")
load("@rules_python_internal//:rules_python_config.bzl", rp_config = "config")
load("@rules_testing//lib:analysis_test.bzl", "analysis_test")
load("@rules_testing//lib:truth.bzl", "matching")
@@ -21,6 +22,8 @@
load("//tests/base_rules:util.bzl", "WINDOWS_ATTR", pt_util = "util")
load("//tests/support:test_platforms.bzl", "WINDOWS")
+_BuiltinPyRuntimeInfo = PyRuntimeInfo
+
_tests = []
def _test_basic_windows(name, config):
@@ -275,6 +278,28 @@
matching.str_matches("name must not end in*.py"),
)
+def _test_py_runtime_info_provided(name, config):
+ rt_util.helper_target(
+ config.rule,
+ name = name + "_subject",
+ srcs = [name + "_subject.py"],
+ )
+ analysis_test(
+ name = name,
+ impl = _test_py_runtime_info_provided_impl,
+ target = name + "_subject",
+ )
+
+def _test_py_runtime_info_provided_impl(env, target):
+ # Make sure that the rules_python loaded symbol is provided.
+ env.expect.that_target(target).has_provider(RulesPythonPyRuntimeInfo)
+
+ # For compatibility during the transition, the builtin PyRuntimeInfo should
+ # also be provided.
+ env.expect.that_target(target).has_provider(_BuiltinPyRuntimeInfo)
+
+_tests.append(_test_py_runtime_info_provided)
+
# Can't test this -- mandatory validation happens before analysis test
# can intercept it
# TODO(#1069): Once re-implemented in Starlark, modify rule logic to make this
