Skip to content
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.

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

chore: improve error message for py_runtime missing version info #301

Merged
merged 1 commit into from
Mar 18, 2024

Conversation

wade-arista
Copy link
Contributor

When a python toolchain is added directly using py_runtime, etc, the interpreter_version_info attribute (a dict) is required.

Prior to this change, it would fail in e.g. _py_binary_rule_impl with an error:

Error: 'dict' value has no field or method 'major'

Check this in earlier in _resolve_toolchain where we find the "interpreter_version_info" attribute, ensuring that it has all of the required dict keys.


Test plan

Manual testing.

Error in fail: 
ERROR: In Bazel 7.x and later, the python toolchain py_runtime interpreter_version_info must be set to a dict with keys "major", "minor", and "micro".

`PyRuntimeInfo` requires that this field contains the static version information for the given
interpreter. This can be set via `py_runtime` when registering an interpreter toolchain, and will
done automatically for the builtin interpreter versions registered via `python_register_toolchains`.
Note that this only available on the Starlark implementation of the provider.

For example:

    py_runtime(
        name = "system_runtime",
        interpreter_path = "/usr/bin/python",
        interpreter_version_info = {
            "major": "3",
            "minor": "11",
            "micro": "6",
        },
        python_version = "PY3",
    )

From this toolchain definition:

load("@rules_python//python:defs.bzl", "py_runtime", "py_runtime_pair")

py_runtime(
    name = "container_py3_runtime",
    interpreter_path = "/usr/bin/python",
    # interpreter_version_info = {
    #     "major": "3",
    #     "minor": "11",
    #     "micro": "6",
    # },
    python_version = "PY3",
)

py_runtime_pair(
    name = "container_py_runtime_pair",
    py2_runtime = None,
    py3_runtime = ":container_py3_runtime",
)

toolchain(
    name = "container_py_toolchain",
    toolchain = ":container_py_runtime_pair",
    toolchain_type = "@bazel_tools//tools/python:toolchain_type",
)

When a python toolchain is added directly using `py_runtime`, etc, the
interpreter_version_info attribute (a dict) is required.

Prior to this change, it would fail in e.g. _py_binary_rule_impl with
an error:

    Error: 'dict' value has no field or method 'major'

Check this in earlier in _resolve_toolchain where we find the
"interpreter_version_info" attribute, ensuring that it has all of the
required dict keys.
Copy link
Member

@mattem mattem left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

@mattem mattem merged commit 404cae5 into aspect-build:main Mar 18, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants