Skip to content

Handle PyInfo removal in bazel 9.x #118

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Handle PyInfo removal in bazel 9.x #118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions MODULE.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ bazel_dep(name = "buildifier_prebuilt", version = "7.3.1")
bazel_dep(name = "platforms", version = "0.0.8")
bazel_dep(name = "rules_python", version = "1.1.0")
bazel_dep(name = "rules_uv", version = "0.21.0")
bazel_dep(name = "bazel_features", version = "1.21.0")

# configuration
PYTHON_VERSION = "3.12"
Expand Down
9 changes: 6 additions & 3 deletions mypy/private/mypy.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@ directories (the results of other mypy builds), the underlying action first atte
directories.
"""

load("@bazel_features//:features.bzl", "bazel_features")
load("@rules_mypy_pip//:requirements.bzl", "requirement")
load("@rules_python//python:py_binary.bzl", "py_binary")
load("@rules_python//python:py_info.bzl", RulesPythonPyInfo = "PyInfo")
load(":py_type_library.bzl", "PyTypeLibraryInfo")

_LegacyPyInfo = bazel_features.globals.PyInfo or RulesPythonPyInfo
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This definition of _LegacyPyInfo can be misleading. On newer Bazel versions where bazel_features.globals.PyInfo is None, this variable will hold RulesPythonPyInfo, which is the modern provider. This makes the name inaccurate and leads to redundant elif checks in _imports and _mypy_impl.

A clearer approach is to have this variable only hold the legacy provider. This makes the name accurate and simplifies the logic at the call sites.

_LegacyPyInfo = bazel_features.globals.PyInfo

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i considered this but didn't really feel worth complicating the callsites when this will be the low traffic path


MypyCacheInfo = provider(
doc = "Output details of the mypy build rule.",
fields = {
Expand All @@ -31,8 +34,8 @@ def _extract_import_dir(import_):
def _imports(target):
if RulesPythonPyInfo in target:
return target[RulesPythonPyInfo].imports.to_list()
elif PyInfo in target:
return target[PyInfo].imports.to_list()
elif _LegacyPyInfo in target:
return target[_LegacyPyInfo].imports.to_list()
Comment on lines +37 to +38
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

With _LegacyPyInfo representing only the legacy provider (as suggested for line 16), this check should ensure the provider is not None before using it. This change avoids a redundant check on newer Bazel versions where the built-in PyInfo is removed.

    elif _LegacyPyInfo and _LegacyPyInfo in target:
        return target[_LegacyPyInfo].imports.to_list()

else:
return []

Expand Down Expand Up @@ -69,7 +72,7 @@ def _mypy_impl(target, ctx):
if target.label.workspace_root != "":
return []

if RulesPythonPyInfo not in target and PyInfo not in target:
if RulesPythonPyInfo not in target and _LegacyPyInfo not in target:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To align with the suggested refactoring on line 16, this check for the legacy provider should also verify that it exists (is not None) before checking for its presence in the target. This improves clarity and robustness during the transition across Bazel versions.

    if RulesPythonPyInfo not in target and not (_LegacyPyInfo and _LegacyPyInfo in target):

return []

# disable if a target is tagged with at least one suppression tag
Expand Down