rules_py icon indicating copy to clipboard operation
rules_py copied to clipboard

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

Open dizzy57 opened this issue 1 year ago • 2 comments

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 avatar Jul 04 '24 15:07 dizzy57

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 avatar Jul 04 '24 15:07 thesayyn

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.

https://github.com/aspect-build/rules_py/blob/7a9e4b26beb807e2cb606be170683e22627d6dc7/WORKSPACE#L37-L42

https://github.com/bazelbuild/bazel/issues/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.

thesayyn avatar Sep 05 '24 19:09 thesayyn