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

[Bug]: python_version silently fall backs to a default toolchain in case it is unable to find specified version #363

Open
dizzy57 opened this issue Jul 4, 2024 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@dizzy57
Copy link

dizzy57 commented Jul 4, 2024

What happened?

I expect to get an error when rules_py can't find configured toolchain for the specified python version.
I also expect examples to pin minor version (e.g. "3.10") instead of full version("3.10.13") to accommodate for future patch version upgrades by rules_python. (e.g. it can silently bump patch versions)

Version

Development (host) and target OS/architectures:

Output of bazel --version: 7.0.2

Version of the Aspect rules, or other relevant rules from your
WORKSPACE or MODULE.bazel file: HEAD (v0.7.3+)

Language(s) and/or frameworks involved:

How to reproduce

Check out https://github.com/dizzy57/rules_py/tree/wrong_python_version_defaults_to_default
Run `bazel test //examples/multi_version:py_version_test`.

Expected: `//examples/multi_version:py_version_test` fails with `AssertionError: 10 == 8` since `python_version = "3.10.13"` is specified in `examples/multi_version/BUILD.bazel`.

Actual: `//examples/multi_version:py_version_test fails with `AssertionError: 9 == 8` since it selects the default toolchain because `3.10` toolchain was never configured.

Any other information?

Please note that silently switching to the default toolchain and specifying full version ("3.10.13") will result in poor user experience after rules_python bumps the default version for 3.10 interpreter in MINOR_MAPPING

@dizzy57 dizzy57 added the bug Something isn't working label Jul 4, 2024
@thesayyn
Copy link
Member

thesayyn commented Jul 4, 2024

We should never silently fallback to the default interpreter if there no matching python toolchain registered for the given python_version.

We should also make sure that specifying python_version without the patch should 3.10 work.

@thesayyn
Copy link
Member

thesayyn commented Sep 5, 2024

Okay more information on this; Reason is that when python_version is set to a version that is not registered, it will fallback to a toolchain that has no target_settings set.

Which precisely what's happening here.

rules_py/WORKSPACE

Lines 37 to 42 in 7a9e4b2

# It is important to register the default toolchain at last as it will be selected for any
# py_test/py_binary target even if it has python_version attribute set.
python_register_toolchains(
name = "python_toolchain",
python_version = "3.9",
)

bazelbuild/bazel#16976 touches this are a little bit.

IN rules_sol i solved this issue by always setting a constraint and setting the default value of the flag to LATEST version.

https://github.com/aspect-build/rules_sol/blob/23831f4cd5f1d35357b46d43d65880d32bcc0a39/sol/private/BUILD.bazel#L6-L10

Something like that could be done here, it needs go into rules_python rather than rules_py.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants