Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit a5e17e6

Browse filesBrowse files
rickeylevrickeylev
authored
fix(PyRuntimeInfo): use builtin PyRuntimeInfo unless pystar is enabled. (bazel-contrib#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 bazel-contrib#1732
1 parent 677fb53 commit a5e17e6
Copy full SHA for a5e17e6

File tree

4 files changed

+55
-3
lines changed
Filter options

4 files changed

+55
-3
lines changed

‎CHANGELOG.md

Copy file name to clipboardExpand all lines: CHANGELOG.md
+5Lines 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

Copy file name to clipboardExpand all lines: python/private/common/py_executable.bzl
+23-1Lines 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

Copy file name to clipboardExpand all lines: python/py_runtime_info.bzl
+2-2Lines 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

Copy file name to clipboardExpand all lines: tests/base_rules/py_executable_base_tests.bzl
+25Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.