Handle PyInfo removal in bazel 9.x #118
Conversation
This optionally fetches PyInfo from bazel_features during the transition.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @keith, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a critical compatibility layer to gracefully handle the upcoming changes in Bazel 9.x regarding the PyInfo provider. By adding a new dependency and implementing a conditional fallback mechanism, the system can adapt to whether the legacy PyInfo is present or if a new PyInfo from bazel_features is available. This ensures that the mypy rules continue to function correctly and resolve Python-related information across different Bazel versions during the transition period.
Highlights
- Dependency Update: Added a new
bazel_depforbazel_featuresat version1.21.0in theMODULE.bazelfile. - PyInfo Compatibility Layer: Introduced a mechanism to handle the potential removal or change of
PyInfoin Bazel 9.x by conditionally fetching it frombazel_features.globals.PyInfoor falling back toRulesPythonPyInfo. - Code Adaptation: Updated
mypy/private/mypy.bzlto utilize the new_LegacyPyInfoalias when checking for and accessingPyInfoattributes within the_importsand_mypy_implfunctions, ensuring forward compatibility.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request updates the Bazel configuration to handle the upcoming removal of the built-in PyInfo provider in Bazel 9.x by using bazel_features for a backward-compatible transition. The changes correctly introduce bazel_features as a dependency and use it to conditionally access the legacy PyInfo. My review includes suggestions to improve the clarity and maintainability of the compatibility logic by making the distinction between the legacy and modern PyInfo providers more explicit, which will prevent redundant checks and make the code easier to understand.
| load("@rules_python//python:py_info.bzl", RulesPythonPyInfo = "PyInfo") | ||
| load(":py_type_library.bzl", "PyTypeLibraryInfo") | ||
|
|
||
| _LegacyPyInfo = bazel_features.globals.PyInfo or RulesPythonPyInfo |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
i considered this but didn't really feel worth complicating the callsites when this will be the low traffic path
| elif _LegacyPyInfo in target: | ||
| return target[_LegacyPyInfo].imports.to_list() |
There was a problem hiding this comment.
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()
| return [] | ||
|
|
||
| if RulesPythonPyInfo not in target and PyInfo not in target: | ||
| if RulesPythonPyInfo not in target and _LegacyPyInfo not in target: |
There was a problem hiding this comment.
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):
1 participant
This optionally fetches PyInfo from bazel_features during the
transition.