diff --git a/scripts/check_pins.py b/scripts/check_pins.py index 79ff56793bb..7964f71b32c 100644 --- a/scripts/check_pins.py +++ b/scripts/check_pins.py @@ -17,11 +17,18 @@ 2. Each of those URLs matches the corresponding ``torch-`` ``[[tool.uv.index]]`` URL in pyproject.toml. 3. pins.json's ``python`` — the version the launcher builds the venv with, - before it installs anything — satisfies ``project.requires-python`` and is - one of the versions pyproject.toml's classifiers claim support for. + before it installs anything — satisfies ``project.requires-python``. Otherwise the launcher creates an interpreter that the package metadata it - is about to install rejects (or that ``uv`` cannot resolve at all), and the - install fails at the last step. + is about to install rejects, and the install fails at the last step. + +It also prints a *warning* when the pin is a version pyproject.toml's classifiers +do not mention. That is advisory only: classifiers are optional in PEP 621 and +informational on PyPI, so they cannot decide whether a pin is installable — +``requires-python`` does. One consequence is deliberate and worth stating: a +version that satisfies an open-ended ``requires-python`` but that no interpreter +has (``>=3.11`` with a ``3.99`` pin) is not caught here. Catching it would mean +gating on non-normative metadata, and a checker that rejects a legal pin is worse +than one that misses an implausible typo. The repo root is derived from this file's own location, so the working directory does not matter — only the path you hand to python does:: @@ -69,7 +76,7 @@ # patch component is inert there in any case. Forbidding the third component also # makes a bogus patch such as "3.12.9999" - which satisfies every specifier but which # `uv python find` cannot resolve - unrepresentable rather than merely unlikely. A -# bogus major.minor is caught instead by requires-python and by the classifier list. +# bogus major.minor is caught instead by requires-python, as far as its bounds reach. # Leading zeros are rejected too, so the pin has exactly one spelling: "03.12" would # otherwise compare equal to "3.12" here while reaching the launcher verbatim. _VERSION_RE = re.compile(r"(?:0|[1-9][0-9]?)\.(?:0|[1-9][0-9]?)\Z") @@ -83,10 +90,12 @@ # "Programming Language :: Python :: 3.12" - a *whole* classifier naming exactly one # major.minor version. The required dot excludes "... :: 3" and "... :: 3 :: Only"; # the anchor excludes "... :: 3.12 :: Only" and "... :: 3.1.4", neither of which -# declares support for the version it appears to name. Digits are bounded like the two -# patterns above so _parse_version's int() cannot hit CPython's str->int digit limit -# and raise out of check_python, which promises never to raise. -_CLASSIFIER_RE = re.compile(r"Programming Language :: Python :: ([0-9]{1,4}\.[0-9]{1,4})\Z") +# declares support for the version it appears to name. Leading zeros are excluded for +# the same reason as in _VERSION_RE: "... :: 3.012" is not a trove classifier, so +# reading it as a declaration of 3.12 support would silence the advisory in exactly the +# case it exists to name. Digits are bounded like the two patterns above, so a 4000-digit +# "version" is not read as one and cannot end up quoted back in the advisory. +_CLASSIFIER_RE = re.compile(r"Programming Language :: Python :: ((?:0|[1-9][0-9]{0,3})\.(?:0|[1-9][0-9]{0,3}))\Z") _SUPPORTED_OPERATORS = "==, !=, >=, <=, > and <" @@ -149,7 +158,9 @@ def _classifier_versions(pyproject: dict) -> list[str]: """The major.minor versions project.classifiers claims support for, in declared order. Tolerates every malformed shape - a missing [project] table, a non-list value, non-string - entries - by returning nothing, so the caller reports it rather than raising. + entries - by returning nothing rather than raising. Nothing is a fine answer: classifiers + are optional, so their absence and their malformation mean the same thing here, which is + that there is no advisory to give. """ project = pyproject.get("project") @@ -158,18 +169,10 @@ def _classifier_versions(pyproject: dict) -> list[str]: return [] matches = (_CLASSIFIER_RE.match(entry) for entry in classifiers if isinstance(entry, str)) # dict.fromkeys de-duplicates without reordering; a repeated classifier is legal but would - # otherwise be listed twice in the error message. + # otherwise be listed twice in the warning. return list(dict.fromkeys(match.group(1) for match in matches if match is not None)) -def _is_dynamic(pyproject: dict, field: str) -> bool: - """Whether project.dynamic defers `field` to the build backend (PEP 621).""" - - project = pyproject.get("project") - dynamic = project.get("dynamic") if isinstance(project, dict) else None - return isinstance(dynamic, list) and field in dynamic - - def check_python(pins: dict, pyproject: dict) -> list[str]: """Check pins.json's `python` against pyproject.toml's `requires-python`. @@ -197,28 +200,6 @@ def check_python(pins: dict, pyproject: dict) -> list[str]: version = _parse_version(pinned) requires_python = ", ".join(clauses) - # requires-python says which versions the metadata *allows*; the classifiers say which ones - # this project actually ships for. Without the second check a pin like '3.99' - inside an - # open-ended requires-python, but not an interpreter uv can find - would still pass. - supported = _classifier_versions(pyproject) - if not supported and _is_dynamic(pyproject, "classifiers"): - errors.append( - "pyproject.toml lists 'classifiers' in project.dynamic, so pins.json's python pin " - "cannot be checked against the versions this project ships for; teach " - "scripts/check_pins.py where the classifiers come from" - ) - elif not supported: - errors.append( - "pyproject.toml declares no 'Programming Language :: Python :: X.Y' classifier, so " - "pins.json's python pin cannot be checked against the versions this project ships for" - ) - elif version not in {_parse_version(entry) for entry in supported}: - errors.append( - f"pins.json python is '{pinned}' but pyproject.toml's classifiers declare support only " - f"for {', '.join(supported)}; add the classifier if that version is really supported, " - "otherwise the launcher builds the venv on an interpreter this project does not ship for" - ) - for clause in clauses: match = _CLAUSE_RE.match(clause) if match is None: @@ -239,6 +220,37 @@ def check_python(pins: dict, pyproject: dict) -> list[str]: return errors +def check_python_classifiers(pins: dict, pyproject: dict) -> list[str]: + """Return advisory notes - never errors - about the python pin vs project.classifiers. + + `requires-python` is what actually gates installation, and it is the only authority this + script fails on. Classifiers are optional in PEP 621 and purely informational on PyPI, so a + pin they don't mention is a documentation gap, not a broken install: `requires-python = + ">=3.11, <3.13"` genuinely permits a 3.11 pin whether or not a 3.11 classifier exists. + Saying so out loud is still useful - the classifiers are what we publish - but it must not + fail the build, and their *absence* must not be treated as a finding at all. + + Never raises, for the same reason check_python doesn't. + """ + + pinned = pins.get("python") if isinstance(pins, dict) else None + if not isinstance(pinned, str) or _VERSION_RE.match(pinned) is None: + return [] # check_python already reports the shape + + # A plain string comparison is enough: both patterns forbid leading zeros, so a version has + # exactly one spelling on either side and there is nothing left to normalize. + supported = _classifier_versions(pyproject) + if not supported or pinned in supported: + return [] + + return [ + f"pins.json python is '{pinned}', which pyproject.toml's classifiers do not mention " + f"(they list {', '.join(supported)}). That is allowed - classifiers are informational, and " + "requires-python is what gates installation - but consider adding " + f"'Programming Language :: Python :: {pinned}' so the published metadata matches." + ] + + def _uv_indexes(pyproject: dict) -> dict[str, str]: """Map [[tool.uv.index]] name -> url, tolerating a malformed or absent table.""" @@ -328,10 +340,18 @@ def main(repo_root: Path = REPO_ROOT) -> int: for error in errors: print(f" - {error}", file=sys.stderr) print( - "\nUpdate pins.json to match the [[tool.uv.index]] URLs, requires-python and python " - "classifiers in pyproject.toml (or vice versa).", + "\nUpdate pins.json to match the [[tool.uv.index]] URLs and requires-python in " + "pyproject.toml (or vice versa).", file=sys.stderr, ) + + # Deliberately after the errors have already been printed: advice is worth less than a real + # problem, so computing it must not be able to come between one and its report. It is still + # printed on a failing run - a wrong pin and an unmentioned pin are usually the same edit. + for warning in check_python_classifiers(pins, pyproject): + print(f"warning: {warning}", file=sys.stderr) + + if errors: return 1 print("pins.json is consistent with pyproject.toml") diff --git a/tests/test_check_pins.py b/tests/test_check_pins.py index 5ad82be81c3..f42f6998111 100644 --- a/tests/test_check_pins.py +++ b/tests/test_check_pins.py @@ -210,8 +210,8 @@ def test_patch_pin_is_rejected_for_being_a_patch_pin(repo_copy: Path, capsys: py assert check_pins.main(repo_copy) == 1 stderr = capsys.readouterr().err assert "a patch component is deliberately not accepted" in stderr - # Not the bare word "classifiers" - that appears in the advice footer printed on every failure. - assert "classifiers declare support only for" not in stderr + # The shape is already wrong, so the classifier advisory has nothing useful to add. + assert "warning" not in stderr def _drop_python_classifiers(repo: Path) -> None: @@ -221,34 +221,87 @@ def _drop_python_classifiers(repo: Path) -> None: path.write_text(updated) -def test_python_not_in_classifiers_fails(repo_copy: Path, capsys: pytest.CaptureFixture[str]): - """requires-python allows 3.11, but the project only ships classifiers for 3.12. The pin - has to name a version this project actually claims to support.""" +def test_pin_allowed_by_requires_python_passes_without_a_classifier( + repo_copy: Path, capsys: pytest.CaptureFixture[str] +): + """requires-python is ">=3.11, <3.13", so 3.11 is a legal pin whether or not a 3.11 + classifier exists. Classifiers are optional and informational, so they must not veto it.""" pins = _read_pins(repo_copy) pins["python"] = "3.11" _write_pins(repo_copy, pins) + assert check_pins.main(repo_copy) == 0 + # Mentioned, but only as advice. + assert "warning: pins.json python is '3.11'" in capsys.readouterr().err + + +def test_unmentioned_pin_is_a_warning_not_an_error(repo_copy: Path, capsys: pytest.CaptureFixture[str]): + """The advisory names the classifier to add and says why it is not fatal.""" + pins = _read_pins(repo_copy) + pins["python"] = "3.11" + _write_pins(repo_copy, pins) + + check_pins.main(repo_copy) + stderr = capsys.readouterr().err + assert "Programming Language :: Python :: 3.11" in stderr + assert "out of sync" not in stderr # the failure banner + + +def test_leading_zero_classifier_does_not_count_as_mentioning_the_pin( + repo_copy: Path, capsys: pytest.CaptureFixture[str] +): + """'Programming Language :: Python :: 3.012' is not a trove classifier, so PyPI shows no + 3.12 support. Normalizing it to (3, 12) would silence the advisory - so 3.11 is declared + alongside it, leaving the advisory something to report against.""" + _drop_python_classifiers(repo_copy) + path = repo_copy / "pyproject.toml" + added = " 'Programming Language :: Python :: 3.012',\n 'Programming Language :: Python :: 3.11',\n" + path.write_text(path.read_text().replace("classifiers = [\n", f"classifiers = [\n{added}", 1)) + + assert check_pins.main(repo_copy) == 0 + # The pin is 3.12, and 3.11 is the only version really declared. + assert "they list 3.11)" in capsys.readouterr().err + + +def test_advisory_is_still_printed_on_a_failing_run(repo_copy: Path, capsys: pytest.CaptureFixture[str]): + """The warning loop runs after the errors are printed, not instead of them: a wrong pin and + an unmentioned pin are usually the same edit, so both belong in the same output.""" + pins = _read_pins(repo_copy) + pins["python"] = "3.11" + pins["torchIndexUrl"]["linux"]["rocm"] = "https://download.pytorch.org/whl/rocm6.3" + _write_pins(repo_copy, pins) + assert check_pins.main(repo_copy) == 1 - assert "classifiers declare support only for" in capsys.readouterr().err + stderr = capsys.readouterr().err + assert "torchIndexUrl.linux.rocm" in stderr + assert "warning: pins.json python is '3.11'" in stderr + + +def test_pin_named_by_the_classifiers_warns_about_nothing(repo_copy: Path, capsys: pytest.CaptureFixture[str]): + """The checked-in state: 3.12 is pinned and 3.12 is classified, so there is nothing to say.""" + assert check_pins.main(repo_copy) == 0 + assert "warning" not in capsys.readouterr().err -def test_unreal_version_inside_an_open_ended_requires_python_fails(repo_copy: Path): - """The hole the classifier check exists to close: with no upper bound in requires-python, - a version that no interpreter has would otherwise satisfy every clause.""" +def test_unreal_version_inside_an_open_ended_requires_python_is_not_caught(repo_copy: Path): + """Documenting the deliberate gap: with no upper bound in requires-python there is no + normative metadata left to reject '3.99' with. Gating on classifiers would catch it, but at + the cost of rejecting legal pins - see test_pin_allowed_by_requires_python_passes_*.""" _set_requires_python(repo_copy, ">=3.11") pins = _read_pins(repo_copy) pins["python"] = "3.99" _write_pins(repo_copy, pins) - assert check_pins.main(repo_copy) == 1 + assert check_pins.main(repo_copy) == 0 -def test_missing_python_classifiers_fails(repo_copy: Path, capsys: pytest.CaptureFixture[str]): - """Deleting the classifiers must not silently downgrade the pin check to requires-python only.""" +def test_missing_python_classifiers_is_not_a_failure(repo_copy: Path, capsys: pytest.CaptureFixture[str]): + """classifiers is optional in PEP 621. Its absence says nothing about the pin, so it is not + a finding - not even a warning.""" _drop_python_classifiers(repo_copy) - assert check_pins.main(repo_copy) == 1 - assert "no 'Programming Language :: Python :: X.Y' classifier" in capsys.readouterr().err + assert check_pins.main(repo_copy) == 0 + assert "warning" not in capsys.readouterr().err @pytest.mark.parametrize( @@ -260,6 +313,10 @@ def test_missing_python_classifiers_fails(repo_copy: Path, capsys: pytest.Captur "Programming Language :: Python :: 3.1.4", # ditto "Programming Language :: Python :: 3.12 ", # trailing space "Programming Language :: Python :: Implementation :: CPython", + # Not trove classifiers. Reading either as a declaration of 3.12 support would silence + # the advisory in exactly the case it exists to name. + "Programming Language :: Python :: 3.012", + "Programming Language :: Python :: 03.12", ], ) def test_classifier_is_not_read_as_a_version(repo_copy: Path, classifier: str): @@ -281,13 +338,13 @@ def test_duplicate_classifiers_are_reported_once(repo_copy: Path): @pytest.mark.parametrize("literal", ["3", "true", '"3.12"', "{}", "[3.12]", "[[]]", "[]"]) def test_malformed_classifiers_do_not_raise(repo_copy: Path, literal: str): - """A non-list, or a list of non-strings, must be reported rather than blowing up - the - same tolerance requires-python already gets.""" + """A non-list, or a list of non-strings, must not blow up the checker. Since classifiers no + longer gate anything, a malformed one costs only the advisory.""" path = repo_copy / "pyproject.toml" path.write_text(re.sub(r"(?ms)^classifiers = \[.*?^\]$", f"classifiers = {literal}", path.read_text())) assert "classifiers = [\n" not in (repo_copy / "pyproject.toml").read_text() - assert check_pins.main(repo_copy) == 1 + assert check_pins.main(repo_copy) == 0 def test_missing_project_table_does_not_raise(repo_copy: Path): @@ -304,29 +361,37 @@ def test_missing_project_table_does_not_raise(repo_copy: Path): def test_absurdly_long_classifier_version_does_not_mask_url_drift(repo_copy: Path, capsys: pytest.CaptureFixture[str]): - """int() refuses to parse a string of more than 4300 digits. An unbounded classifier - pattern would let one reach _parse_version and raise straight out of check_python, - taking the torchIndexUrl checks with it.""" + """A 4400-digit "version" is not a version. Nothing converts classifier text to int any + more, so this can no longer raise - but the classifier code shares main() with the + torchIndexUrl reporting, this run must still name the drift, and an unbounded pattern + would quote all 4400 digits back at the reader in the advisory.""" + digits = "9" * 4400 + absurd = f" 'Programming Language :: Python :: {digits}.0',\n" + # 3.11 gives the advisory something real to list; the pin is 3.12. + real = " 'Programming Language :: Python :: 3.11',\n" + _drop_python_classifiers(repo_copy) path = repo_copy / "pyproject.toml" - absurd = "Programming Language :: Python :: " + "9" * 4400 + ".0" - path.write_text(path.read_text().replace("classifiers = [\n", f"classifiers = [\n '{absurd}',\n", 1)) + path.write_text(path.read_text().replace("classifiers = [\n", f"classifiers = [\n{absurd}{real}", 1)) pins = _read_pins(repo_copy) pins["torchIndexUrl"]["linux"]["rocm"] = "https://download.pytorch.org/whl/rocm6.3" _write_pins(repo_copy, pins) assert check_pins.main(repo_copy) == 1 - assert "torchIndexUrl.linux.rocm" in capsys.readouterr().err + stderr = capsys.readouterr().err + assert "torchIndexUrl.linux.rocm" in stderr + assert "they list 3.11)" in stderr + assert digits not in stderr -def test_dynamic_classifiers_are_reported_as_such(repo_copy: Path, capsys: pytest.CaptureFixture[str]): - """PEP 621 lets the build backend supply classifiers. The checker can't read them, and must - say so rather than claiming none are declared.""" +def test_dynamic_classifiers_are_not_a_failure(repo_copy: Path, capsys: pytest.CaptureFixture[str]): + """PEP 621 lets the build backend supply classifiers. The checker can't read them - which is + fine now that it doesn't gate on them.""" _drop_python_classifiers(repo_copy) path = repo_copy / "pyproject.toml" path.write_text(re.sub(r'(?m)^dynamic = \["version"\]$', 'dynamic = ["version", "classifiers"]', path.read_text())) - assert check_pins.main(repo_copy) == 1 - assert "project.dynamic" in capsys.readouterr().err + assert check_pins.main(repo_copy) == 0 + assert "warning" not in capsys.readouterr().err @pytest.mark.parametrize("value", [None, "3.12", ["3.12"], 5])