Skip to content

Commit a5e17e6

Browse files
authored
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 #1732
1 parent 677fb53 commit a5e17e6

File tree

4 files changed

+55
-3
lines changed

4 files changed

+55
-3
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ A brief description of the categories of changes:
4848
* (toolchain) Changed the `host_toolchain` to symlink all files to support
4949
Windows host environments without symlink support.
5050

51+
* (PyRuntimeInfo) Switch back to builtin PyRuntimeInfo for Bazel 6.4 and when
52+
pystar is disabled. This fixes an error about `target ... does not have ...
53+
PyRuntimeInfo`.
54+
([#1732](https://github.com/bazelbuild/rules_python/issues/1732))
55+
5156
### Added
5257

5358
* (py_wheel) Added `requires_file` and `extra_requires_files` attributes.

python/private/common/py_executable.bzl

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
"""Common functionality between test/binary executables."""
1515

1616
load("@bazel_skylib//lib:dicts.bzl", "dicts")
17+
load("//python/private:reexports.bzl", "BuiltinPyRuntimeInfo")
1718
load(
1819
":attributes.bzl",
1920
"AGNOSTIC_EXECUTABLE_ATTRS",
@@ -771,7 +772,28 @@ def _create_providers(
771772
# TODO(b/265840007): Make this non-conditional once Google enables
772773
# --incompatible_use_python_toolchains.
773774
if runtime_details.toolchain_runtime:
774-
providers.append(runtime_details.toolchain_runtime)
775+
py_runtime_info = runtime_details.toolchain_runtime
776+
providers.append(py_runtime_info)
777+
778+
# Re-add the builtin PyRuntimeInfo for compatibility to make
779+
# transitioning easier, but only if it isn't already added because
780+
# returning the same provider type multiple times is an error.
781+
# NOTE: The PyRuntimeInfo from the toolchain could be a rules_python
782+
# PyRuntimeInfo or a builtin PyRuntimeInfo -- a user could have used the
783+
# builtin py_runtime rule or defined their own. We can't directly detect
784+
# the type of the provider object, but the rules_python PyRuntimeInfo
785+
# object has an extra attribute that the builtin one doesn't.
786+
if hasattr(py_runtime_info, "interpreter_version_info"):
787+
providers.append(BuiltinPyRuntimeInfo(
788+
interpreter_path = py_runtime_info.interpreter_path,
789+
interpreter = py_runtime_info.interpreter,
790+
files = py_runtime_info.files,
791+
coverage_tool = py_runtime_info.coverage_tool,
792+
coverage_files = py_runtime_info.coverage_files,
793+
python_version = py_runtime_info.python_version,
794+
stub_shebang = py_runtime_info.stub_shebang,
795+
bootstrap_template = py_runtime_info.bootstrap_template,
796+
))
775797

776798
# TODO(b/163083591): Remove the PyCcLinkParamsProvider once binaries-in-deps
777799
# are cleaned up.

python/py_runtime_info.bzl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@
1414

1515
"""Public entry point for PyRuntimeInfo."""
1616

17+
load("@rules_python_internal//:rules_python_config.bzl", "config")
1718
load("//python/private:reexports.bzl", "BuiltinPyRuntimeInfo")
18-
load("//python/private:util.bzl", "IS_BAZEL_6_OR_HIGHER")
1919
load("//python/private/common:providers.bzl", _starlark_PyRuntimeInfo = "PyRuntimeInfo")
2020

21-
PyRuntimeInfo = _starlark_PyRuntimeInfo if IS_BAZEL_6_OR_HIGHER else BuiltinPyRuntimeInfo
21+
PyRuntimeInfo = _starlark_PyRuntimeInfo if config.enable_pystar else BuiltinPyRuntimeInfo

tests/base_rules/py_executable_base_tests.bzl

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
# limitations under the License.
1414
"""Tests common to py_binary and py_test (executable rules)."""
1515

16+
load("@rules_python//python:py_runtime_info.bzl", RulesPythonPyRuntimeInfo = "PyRuntimeInfo")
1617
load("@rules_python_internal//:rules_python_config.bzl", rp_config = "config")
1718
load("@rules_testing//lib:analysis_test.bzl", "analysis_test")
1819
load("@rules_testing//lib:truth.bzl", "matching")
@@ -21,6 +22,8 @@ load("//tests/base_rules:base_tests.bzl", "create_base_tests")
2122
load("//tests/base_rules:util.bzl", "WINDOWS_ATTR", pt_util = "util")
2223
load("//tests/support:test_platforms.bzl", "WINDOWS")
2324

25+
_BuiltinPyRuntimeInfo = PyRuntimeInfo
26+
2427
_tests = []
2528

2629
def _test_basic_windows(name, config):
@@ -275,6 +278,28 @@ def _test_name_cannot_end_in_py_impl(env, target):
275278
matching.str_matches("name must not end in*.py"),
276279
)
277280

281+
def _test_py_runtime_info_provided(name, config):
282+
rt_util.helper_target(
283+
config.rule,
284+
name = name + "_subject",
285+
srcs = [name + "_subject.py"],
286+
)
287+
analysis_test(
288+
name = name,
289+
impl = _test_py_runtime_info_provided_impl,
290+
target = name + "_subject",
291+
)
292+
293+
def _test_py_runtime_info_provided_impl(env, target):
294+
# Make sure that the rules_python loaded symbol is provided.
295+
env.expect.that_target(target).has_provider(RulesPythonPyRuntimeInfo)
296+
297+
# For compatibility during the transition, the builtin PyRuntimeInfo should
298+
# also be provided.
299+
env.expect.that_target(target).has_provider(_BuiltinPyRuntimeInfo)
300+
301+
_tests.append(_test_py_runtime_info_provided)
302+
278303
# Can't test this -- mandatory validation happens before analysis test
279304
# can intercept it
280305
# TODO(#1069): Once re-implemented in Starlark, modify rule logic to make this

0 commit comments

Comments
 (0)