From 4e1151b9fab284a33109b610b562b78183fdb425 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 20 Oct 2026 20:31:21 +0000 Subject: [PATCH] build: Skip pythons excluded by Requires-Python instead of retrying Co-Authored-By: Joshua Li <14209781+joshuarli@users.noreply.github.com> --- build.py | 129 ++++++++++++++++++++++++++++++++++++----- docs/build-pipeline.md | 5 ++ docs/packages-ini.md | 3 + tests/build_test.py | 98 +++++++++++++++++++++++++++++++ tests/validate_test.py | 19 ++++++ validate.py | 28 +++++++++ 6 files changed, 269 insertions(+), 13 deletions(-) diff --git a/build.py b/build.py index f9afef42..d22d2d25 100644 --- a/build.py +++ b/build.py @@ -15,6 +15,7 @@ import tarfile import tempfile import time +import urllib.error import urllib.parse import urllib.request import zipfile @@ -25,6 +26,7 @@ from typing import ContextManager from typing import NamedTuple +from packaging.specifiers import InvalidSpecifier from packaging.specifiers import SpecifierSet from packaging.tags import Tag from packaging.tags import compatible_tags @@ -68,6 +70,25 @@ def version_string(self) -> str: def exe(self) -> str: return "python{}.{}".format(*self.version) + @property + def full_version(self) -> str: + return _python_full_version(self.exe) + + +@functools.cache +def _python_full_version(exe: str) -> str: + cmd = (exe, "-c", "import platform; print(platform.python_version())") + return subprocess.check_output(cmd).decode().strip() + + +class Release(NamedTuple): + requires_python: SpecifierSet + has_sdist: bool + wheel_tags: frozenset[Tag] + + def supports(self, python: Python) -> bool: + return self.requires_python.contains(python.full_version, prereleases=True) + class Package(NamedTuple): name: str @@ -120,6 +141,50 @@ def make(cls, key: str, val: Mapping[str, str]) -> Package: ) +def _public_pypi_retry_delays() -> Generator[float]: + # A release PR can land before public PyPI exposes the new version. + # Yield backoff delays while the publication propagates. + deadline = time.monotonic() + PUBLIC_PYPI_RETRY_TIMEOUT + delay: float = PUBLIC_PYPI_INITIAL_RETRY_DELAY + while (remaining := deadline - time.monotonic()) > 0: + delay = min(delay, remaining) + yield delay + delay *= 2 + + +def _public_release(package: Package) -> Release: + url = f"https://pypi.org/pypi/{package.name}/{package.version}/json" + for delay in _public_pypi_retry_delays(): + try: + resp = urllib.request.urlopen(url) + except urllib.error.HTTPError as e: + if e.code != 404: + raise + print(f"-> not on public PyPI yet; retrying in {delay:g}s") + time.sleep(delay) + else: + break + else: + raise SystemExit(f"{package.name}=={package.version}: not on public PyPI") + + data = json.load(resp) + try: + requires_python = SpecifierSet(data["info"]["requires_python"] or "") + except InvalidSpecifier: + requires_python = SpecifierSet() + + wheel_tags: set[Tag] = set() + for url_info in data["urls"]: + if url_info["packagetype"] == "bdist_wheel": + wheel_tags.update(parse_wheel_filename(url_info["filename"])[3]) + + return Release( + requires_python=requires_python, + has_sdist=any(u["packagetype"] == "sdist" for u in data["urls"]), + wheel_tags=frozenset(wheel_tags), + ) + + def _add_wheel(d: dict[str, list[tuple[Version, frozenset[Tag]]]], f: str) -> None: name, version, _, tags = parse_wheel_filename(f) d.setdefault(name, []).append((version, tags)) @@ -568,9 +633,8 @@ def _download_sdist( *, timeout: int | None = None, ) -> None: - # A release PR can land before PyPI's simple index exposes the new version. - # Retry this lookup while the publication propagates, without retrying the - # actual build, where repeating a genuine build failure would not help. + # Retry only this lookup, not the actual build, where repeating a genuine + # build failure would not help. pip = (python.exe, "-mpip") cmd = ( *pip, @@ -581,24 +645,40 @@ def _download_sdist( f"--no-binary={package.name}", f"{package.name}=={package.version}", ) - deadline = time.monotonic() + PUBLIC_PYPI_RETRY_TIMEOUT - delay: float = PUBLIC_PYPI_INITIAL_RETRY_DELAY - - while True: + for delay in _public_pypi_retry_delays(): try: subprocess.check_call(cmd, timeout=timeout) except subprocess.CalledProcessError: - remaining = deadline - time.monotonic() - if remaining <= 0: - raise - - delay = min(delay, remaining) print(f"-> public PyPI download failed; retrying in {delay:g}s") time.sleep(delay) - delay *= 2 else: return + subprocess.check_call(cmd, timeout=timeout) + + +def _download_wheel_only( + package: Package, + python: Python, + release: Release, + dest: str, +) -> str: + pkg_id = f"{package.name}=={package.version}" + if not release.wheel_tags & python.tags: + raise SystemExit( + f"{pkg_id}: no sdist and no wheel for python {python.version_string}" + ) + + # public PyPI lists a matching wheel but the simple index may still lag + for delay in _public_pypi_retry_delays(): + print(f"-> public PyPI wheel download failed; retrying in {delay:g}s") + time.sleep(delay) + downloaded_wheel = _download(package, python, dest) + if downloaded_wheel is not None: + return downloaded_wheel + + raise SystemExit(f"{pkg_id}: could not download wheel from public PyPI") + def _build( package: Package, @@ -688,6 +768,7 @@ def main() -> int: failed_names: set[str] = set() all_packages = [Package.make(k, cfg[k]) for k in cfg.sections()] + releases: dict[str, Release] = {} if args.upgrade_python: # sort newest versions first within each package name so we can @@ -721,8 +802,30 @@ def main() -> int: print("-> just built!") else: try: + if pkg_id not in releases: + releases[pkg_id] = _public_release(package) + release = releases[pkg_id] + + if not any( + release.supports(p) + for p in pythons + if p.version_string in package.python_versions + ): + raise SystemExit( + f"{pkg_id}: requires-python {release.requires_python} " + f"matches none of the build pythons" + ) + elif not release.supports(python): + print(f"-> skipping: requires-python {release.requires_python}") + continue + print("-> building...") downloaded_wheel = _download(package, python, args.dest) + if downloaded_wheel is None and not release.has_sdist: + downloaded_wheel = _download_wheel_only( + package, python, release, args.dest + ) + if downloaded_wheel is not None: _add_wheel(built, downloaded_wheel) print(f"-> downloaded! {downloaded_wheel}") diff --git a/docs/build-pipeline.md b/docs/build-pipeline.md index 955e550d..2d0f3e67 100644 --- a/docs/build-pipeline.md +++ b/docs/build-pipeline.md @@ -9,6 +9,9 @@ It skips a package version when the index already has a wheel for it that matche It also skips Python versions that the section's `python_versions` excludes. For each remaining package and Python version in `PYTHONS`, `build.py` does this: +1. Reads the release from public PyPI's JSON API, retrying for up to one hour until the release is visible. + It skips Python versions that the release's `Requires-Python` excludes, because pip would never install the package there. + It fails right away when `Requires-Python` excludes every build Python. 1. Downloads a wheel from public PyPI. It discards the wheel when a compiled file in it has the wrong architecture. 1. If no usable wheel exists, downloads the sdist from public PyPI and builds it with `pip wheel`. @@ -22,6 +25,7 @@ The `image` workflow (`.github/workflows/image.yml`) builds that image from `doc So a pull request that changes `docker/` builds packages with the old `:latest` image. The sdist download retries for up to one hour, because a new release can reach PyPI's simple index after its release PR lands. +For a release with no sdist, the wheel download retries instead, but only when public PyPI lists a wheel that matches the Python version. ## validate @@ -29,6 +33,7 @@ The sdist download retries for up to one hour, because a new release can reach P It installs only binary wheels from this index plus the just-built wheels. So every dependency of a new package must also be in `packages.ini`. Then it imports each top-level module of the wheel. +It skips Python versions that the wheel's `Requires-Python` excludes. ## deploy diff --git a/docs/packages-ini.md b/docs/packages-ini.md index 82859d14..ad3d05cf 100644 --- a/docs/packages-ini.md +++ b/docs/packages-ini.md @@ -66,6 +66,9 @@ python_versions = <3.12 `validate.py` also skips the excluded versions. +You do not need `python_versions` to repeat a package's `Requires-Python`. +`build.py` and `validate.py` already skip the Python versions that `Requires-Python` excludes. + ## validation options after building the packages will be checked that they can install and import diff --git a/tests/build_test.py b/tests/build_test.py index c586868b..272909ba 100644 --- a/tests/build_test.py +++ b/tests/build_test.py @@ -1,6 +1,7 @@ from __future__ import annotations import io +import json import os import shutil import subprocess @@ -192,6 +193,103 @@ def sleep(delay: float) -> None: assert sum(slept) == build.PUBLIC_PYPI_RETRY_TIMEOUT +def _pypi_json(requires_python, filenames): + return io.BytesIO( + json.dumps( + { + "info": {"requires_python": requires_python}, + "urls": [ + { + "filename": f, + "packagetype": "sdist" + if f.endswith(".tar.gz") + else "bdist_wheel", + } + for f in filenames + ], + } + ).encode() + ) + + +def test_public_release(): + resp = _pypi_json(">=3.13", ("a-1-py3-none-any.whl", "a-1.tar.gz")) + with mock.patch.object(urllib.request, "urlopen", return_value=resp): + release = build._public_release(Package.make("a==1", {})) + + assert release == build.Release( + requires_python=SpecifierSet(">=3.13"), + has_sdist=True, + wheel_tags=frozenset((Tag("py3", "none", "any"),)), + ) + + +def test_public_release_invalid_requires_python_is_ignored(): + resp = _pypi_json(">=3.6.*", ("a-1-py3-none-any.whl",)) + with mock.patch.object(urllib.request, "urlopen", return_value=resp): + release = build._public_release(Package.make("a==1", {})) + + assert release.requires_python == SpecifierSet() + assert release.has_sdist is False + + +def test_public_release_retries_until_visible(): + not_found = urllib.error.HTTPError("u", 404, "Not Found", {}, None) # type: ignore[arg-type] + resp = _pypi_json(None, ("a-1.tar.gz",)) + with ( + mock.patch.object( + urllib.request, "urlopen", side_effect=[not_found, not_found, resp] + ), + mock.patch.object(build.time, "sleep") as sleep, + ): + release = build._public_release(Package.make("a==1", {})) + + assert sleep.call_args_list == [mock.call(1), mock.call(2)] + assert release.requires_python == SpecifierSet() + + +def test_release_supports(): + python = build.Python((3, 11), frozenset()) + release = build.Release(SpecifierSet(">=3.13"), True, frozenset()) + with mock.patch.object(build, "_python_full_version", return_value="3.11.14"): + assert release.supports(python) is False + with mock.patch.object(build, "_python_full_version", return_value="3.13.12"): + assert release.supports(python._replace(version=(3, 13))) is True + + +def test_download_wheel_only_without_matching_wheel_fails_fast(): + package = Package.make("a==1", {}) + python = build.Python((3, 11), frozenset((Tag("cp311", "cp311", "any"),))) + release = build.Release( + SpecifierSet(), False, frozenset((Tag("cp313", "cp313", "any"),)) + ) + + with mock.patch.object(build, "_download") as download: + with pytest.raises(SystemExit) as excinfo: + build._download_wheel_only(package, python, release, "dest") + + assert excinfo.value.code == "a==1: no sdist and no wheel for python 3.11" + download.assert_not_called() + + +def test_download_wheel_only_retries_until_wheel_visible(): + package = Package.make("a==1", {}) + tag = Tag("py3", "none", "any") + python = build.Python((3, 11), frozenset((tag,))) + release = build.Release(SpecifierSet(), False, frozenset((tag,))) + + with ( + mock.patch.object( + build, "_download", side_effect=[None, "a-1-py3-none-any.whl"] + ), + mock.patch.object(build.time, "sleep") as sleep, + ): + ret = build._download_wheel_only(package, python, release, "dest") + + assert ret == "a-1-py3-none-any.whl" + assert sleep.call_args_list == [mock.call(1), mock.call(2)] + + def test_brew_paths(): out = b"""\ /opt/homebrew/opt/openssl@1.1 diff --git a/tests/validate_test.py b/tests/validate_test.py index 116196cc..fcf27dda 100644 --- a/tests/validate_test.py +++ b/tests/validate_test.py @@ -100,3 +100,22 @@ def test_top_imports_record(tmp_path): expected = ["distlib", "_distlib_backend", "distlib_top"] assert validate._top_imports(str(whl)) == expected + + +def test_requires_python_from_metadata(tmp_path): + whl = tmp_path.joinpath("a-1-py3-none-any.whl") + with zipfile.ZipFile(whl, "w") as zipf: + zipf.writestr( + "a-1.dist-info/METADATA", + "Metadata-Version: 2.1\nName: a\nVersion: 1\nRequires-Python: >=3.13\n", + ) + + assert validate._requires_python(str(whl)) == SpecifierSet(">=3.13") + + +def test_requires_python_missing_allows_everything(tmp_path): + whl = tmp_path.joinpath("a-1-py3-none-any.whl") + with zipfile.ZipFile(whl, "w") as zipf: + zipf.writestr("a-1.dist-info/METADATA", "Metadata-Version: 2.1\nName: a\n") + + assert validate._requires_python(str(whl)) == SpecifierSet() diff --git a/validate.py b/validate.py index 346c83e2..cfc74a63 100644 --- a/validate.py +++ b/validate.py @@ -2,6 +2,8 @@ import argparse import configparser +import email.parser +import functools import os.path import re import subprocess @@ -12,6 +14,7 @@ from typing import NamedTuple import packaging.tags +from packaging.specifiers import InvalidSpecifier from packaging.specifiers import SpecifierSet from packaging.tags import Tag from packaging.utils import parse_wheel_filename @@ -70,6 +73,26 @@ def _pythons_to_check(tags: frozenset[Tag]) -> tuple[str, ...]: return tuple(sorted(ret)) +@functools.cache +def _python_full_version(python: str) -> str: + cmd = (python, "-c", "import platform; print(platform.python_version())") + return subprocess.check_output(cmd).decode().strip() + + +def _requires_python(whl: str) -> SpecifierSet: + with zipfile.ZipFile(whl) as zipf: + for name in zipf.namelist(): + if DIST_INFO_RE.match(name) and name.endswith("/METADATA"): + with zipf.open(name) as f: + metadata = email.parser.BytesHeaderParser().parse(f) + try: + return SpecifierSet(metadata.get("Requires-Python", "")) + except InvalidSpecifier: + return SpecifierSet() + else: + raise NotImplementedError("need METADATA") + + def _top_imports(whl: str) -> list[str]: with zipfile.ZipFile(whl) as zipf: dist_info_names = { @@ -167,11 +190,16 @@ def main() -> int: for filename in sorted(os.listdir(args.dist)): name, version, _, wheel_tags = parse_wheel_filename(filename) info = packages[(name, version)] + requires_python = _requires_python(os.path.join(args.dist, filename)) for python in _pythons_to_check(wheel_tags): # e.g. "python3.11" -> "3.11" py_version = python[len("python") :] if py_version not in info.python_versions: continue + elif not requires_python.contains( + _python_full_version(python), prereleases=True + ): + continue try: _validate( python=python,