Skip to content

toolchain resolution may be wrong when python_version does not include a patch version #2685

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

Jump to bottom
Closed
aignas opened this issue Mar 21, 2025 · 0 comments · Fixed by #2735
Closed

toolchain resolution may be wrong when python_version does not include a patch version #2685

aignas opened this issue Mar 21, 2025 · 0 comments · Fixed by #2735

Comments

@aignas
Copy link
Collaborator

aignas commented Mar 21, 2025

Whilst writing code for #2657 I noticed that the toolchain matching is acting weirdly when using X.Y format in the python_version field. I noticed that when I write 3.12, the first toolchain gets matched instead of the entry in MINOR_MAPPING.

The fix would be to change the ordering in the toolchain repo to first list the versions in the MINOR_MAPPING before listing the rest of the toolchains.
aignas added a commit to aignas/rules_python that referenced this issue Apr 4, 2025
@aignas
fix(python): correctly order the toolchains
a570e1a 
Since toolchain matching is done by matching the first target that
matches target settings, the `minor_mapping` config setting is special,
because e.g. all `3.11.X` toolchains match the `python_version = "3.11"`
setting.

This just reshuffles the list so that we have toolchains that are in the
`minor_mapping` before the rest.

Fixes bazel-contrib#2685
aignas added a commit to aignas/rules_python that referenced this issue Apr 4, 2025
@aignas
fix(python): correctly order the toolchains
0b9e65d 
Since toolchain matching is done by matching the first target that
matches target settings, the `minor_mapping` config setting is special,
because e.g. all `3.11.X` toolchains match the `python_version = "3.11"`
setting.

This just reshuffles the list so that we have toolchains that are in the
`minor_mapping` before the rest.

Fixes bazel-contrib#2685
@aignas aignas mentioned this issue Apr 4, 2025
Merged
github-merge-queue bot pushed a commit that referenced this issue Apr 5, 2025
@aignas
fix(toolchains): correctly order the toolchains (#2735)
3d98aee 
Since toolchain matching is done by matching the first target that
matches target settings, the `minor_mapping` config setting is special,
because e.g. all `3.11.X` toolchains match the `python_version = "3.11"`
setting.

This just reshuffles the list so that we have toolchains that are in the
`minor_mapping` before the rest.

At the same time remove the workaround from the `lock.bzl` where the bug
was initially discovered.

Fixes #2685
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(toolchains): correctly order the toolchains aignas/rules_python
1 participant
@aignas