ci(pins): make the python-classifier check advisory, not fatal

scripts/check_pins.py

@@ -17,11 +17,18 @@
   2. Each of those URLs matches the corresponding ``torch-<backend>``
      ``[[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")

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])