Skip to content

fix: correct python_versions() minimum for major-only and multiple lower bounds#1127

Open
Sanjays2402 wants to merge 1 commit into
wntrblm:mainfrom
Sanjays2402:fix/python-versions-min-lower-bound
Open

fix: correct python_versions() minimum for major-only and multiple lower bounds#1127
Sanjays2402 wants to merge 1 commit into
wntrblm:mainfrom
Sanjays2402:fix/python-versions-min-lower-bound

Conversation

@Sanjays2402

Copy link
Copy Markdown

Summary

nox.project.python_versions(pyproject, max_version=...) reads the lower
bound of supported Python versions from requires-python. It looped over the
SpecifierSet and broke on the first lower-bound specifier it encountered,
reading the minor version with spec.version.split(".")[1]. That logic has two
bugs:

1. Crash on a major-only version

A valid requires-python = ">=3" (equivalent to ">=3.0" per PEP 440) has no
minor component, so "3".split(".")[1] raises a cryptic IndexError straight
out of this public helper:

>>> import nox.project
>>> nox.project.python_versions({"project": {"requires-python": ">=3"}}, max_version="3.13")
Traceback (most recent call last):
  ...
IndexError: list index out of range

2. Wrong result with multiple lower bounds

For requires-python = ">=3.8,>=3.10" the effective minimum is 3.10, but
breaking on the first lower-bound specifier returns 3.8, so the result claims
support for 3.8 and 3.9 which the metadata explicitly excludes:

>>> nox.project.python_versions({"project": {"requires-python": ">=3.8,>=3.10"}}, max_version="3.12")
['3.8', '3.9', '3.10', '3.11', '3.12']   # should be ['3.10', '3.11', '3.12']

The output could even flip depending on specifier iteration order.

Root cause

Both symptoms come from the same block, which assumed (a) exactly one relevant
lower bound and (b) that a version string always has a major.minor shape.

Fix

Collect every lower-bound (>, >=, ~=) specifier's minor version,
treat a major-only version as minor 0, and use the maximum as the
effective minimum. When no lower-bound specifier exists, the existing
ValueError is still raised.

requires-python before after
">=3" IndexError ['3.0', …, max]
">=3.8,>=3.10" (max 3.12) ['3.8','3.9','3.10','3.11','3.12'] ['3.10','3.11','3.12']
">=3.10,>=3.8" (max 3.12) order-dependent ['3.10','3.11','3.12']
">=3.10" (max 3.12) ['3.10','3.11','3.12'] unchanged
">3.2.1,<3.3" (max 3.4) ['3.2','3.3','3.4'] unchanged
"==3.3.1" ValueError unchanged

Tests

Added test_python_range_major_only and test_python_range_multiple_lower_bounds
to tests/test_project.py. Verified they fail on main (the first with the
IndexError, the second asserting on the wrong ['3.8', '3.9', …] output) and
pass with the fix.

$ python -m pytest tests/test_project.py -q
9 passed

$ python -m pytest -q      # full suite
720 passed, 27 skipped, 1 xpassed

ruff check and ruff format --check are clean on the changed files, and the
change introduces no new mypy errors (the 3 reported in nox/project.py are
pre-existing and outside this diff).

…wer bounds

nox.project.python_versions(pyproject, max_version=...) parsed the lower
bound from requires-python by looping over the SpecifierSet and breaking on
the first lower-bound specifier it saw, reading its minor version with
`spec.version.split(".")[1]`. This had two bugs:

1. Crash on a major-only version. A valid requires-python of ">=3" (which
   PEP 440 defines as equivalent to ">=3.0") has no minor component, so
   `"3".split(".")[1]` raised a cryptic `IndexError: list index out of range`
   from this public helper.

2. Wrong result with multiple lower bounds. For ">=3.8,>=3.10" the effective
   minimum is 3.10, but breaking on the first lower-bound specifier returned
   3.8 as the minimum, so the result claimed support for 3.8 and 3.9 which the
   metadata explicitly excludes. The output could even depend on specifier
   iteration order.

Fix: collect every lower-bound (">", ">=", "~=") specifier's minor version,
treating a major-only version as minor 0, and use the maximum as the
effective minimum. If there is no lower-bound specifier at all, the existing
ValueError is still raised.

Added regression tests for the major-only and multiple-lower-bound cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant