From 839ec778d4e2b6af2fc5a37c2e35c5588d7d3810 Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Wed, 18 Dec 2024 14:12:31 -0500 Subject: [PATCH 01/18] add 3-field form and tests --- .../python_build_standalone/rules.py | 293 +++++++++++++++--- .../rules_integration_test.py | 22 +- 2 files changed, 258 insertions(+), 57 deletions(-) diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules.py b/src/python/pants/backend/python/providers/python_build_standalone/rules.py index 3143535fc11..e4102c4bdfe 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules.py @@ -4,13 +4,17 @@ import functools import json +import logging import posixpath import re import textwrap import urllib import uuid +from dataclasses import dataclass from pathlib import PurePath -from typing import Iterable, Mapping, TypedDict, cast +from typing import Iterable, Mapping, Sequence, TypedDict, cast + +from packaging.version import InvalidVersion from pants.backend.python.providers.python_build_standalone.constraints import ( ConstraintParseError, @@ -55,6 +59,8 @@ from pants.util.strutil import softwrap from pants.version import Version +logger = logging.getLogger(__name__) + PBS_SANDBOX_NAME = ".python_build_standalone" PBS_NAMED_CACHE_NAME = "python_build_standalone" PBS_APPEND_ONLY_CACHES = FrozenDict({PBS_NAMED_CACHE_NAME: PBS_SANDBOX_NAME}) @@ -69,6 +75,220 @@ class PBSPythonInfo(TypedDict): PBSVersionsT = dict[str, dict[str, dict[str, PBSPythonInfo]]] +@dataclass +class _ParsedPBSPython: + py_version: Version + pbs_release_tag: Version + platform: Platform + url: str + sha256: str + size: int + + +def _parse_py_version_and_pbs_release_tag( + version_and_tag: str, +) -> tuple[Version | None, Version | None]: + version_and_tag = version_and_tag.strip() + if not version_and_tag: + return None, None + + parts = version_and_tag.split("+", 2) + py_version: Version | None = None + pbs_release_tag: Version | None = None + + if len(parts) >= 1: + try: + py_version = Version(parts[0]) + except InvalidVersion: + raise ValueError(f"Version `{parts[0]}` is not a valid Python version.") + + if len(parts) == 2: + try: + pbs_release_tag = Version(parts[1]) + except InvalidVersion: + raise ValueError(f"PBS release tag `{parts[1]}` is not a valid version.") + + return py_version, pbs_release_tag + + +def _parse_pbs_url(url: str) -> tuple[Version, Version, Platform]: + parsed_url = urllib.parse.urlparse(urllib.parse.unquote(url)) + base_path = posixpath.basename(parsed_url.path) + + base_path_no_prefix = base_path.removeprefix("cpython-") + if base_path_no_prefix == base_path: + raise ValueError( + f"Unable to parse the provided URL since it does not have a cpython prefix as per the PBS naming convention: {url}" + ) + + base_path_parts = base_path_no_prefix.split("-", 1) + if len(base_path_parts) != 2: + raise ValueError( + f"Unable to parse the provided URL because it does not follow the PBS naming convention: {url}" + ) + + py_version, pbs_release_tag = _parse_py_version_and_pbs_release_tag(base_path_parts[0]) + if not py_version or not pbs_release_tag: + raise ValueError( + "Unable to parse the Python version and PBS release tag from the provided URL " + f"because it does not follow the PBS naming convention: {url}" + ) + + platform: Platform + match base_path_parts[1].split("-"): + case ["x86_64", "unknown", "linux", "gnu", *_]: + platform = Platform.linux_x86_64 + case ["aarch64", "unknown", "linux", "gnu", *_]: + platform = Platform.linux_arm64 + case ["x86_64", "apple", "darwin", *_]: + platform = Platform.macos_x86_64 + case ["aarch64", "apple", "darwin", *_]: + platform = Platform.macos_arm64 + case _: + raise ValueError( + "Unable to parse the platfornm from the provided URL " + f"because it does not follow the PBS naming convention: {url}" + ) + + # assert platform is not None + + return py_version, pbs_release_tag, platform + + +def _parse_from_three_fields(parts: Sequence[str], orig_value: str) -> _ParsedPBSPython: + url, sha256, size = parts[0:2] + + try: + py_version, pbs_release_tag, platform = _parse_pbs_url(url) + except ValueError as e: + raise ExternalToolError( + f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " + f"the value `{orig_value}` could not be parsed because the URL does not follow the " + f"PBS naming convention: {e}" + ) + + return _ParsedPBSPython( + py_version=py_version, + pbs_release_tag=pbs_release_tag, + platform=platform, + url=url, + sha256=sha256, + size=int(size), + ) + + +def _parse_from_five_fields(parts: Sequence[str], orig_value: str) -> _ParsedPBSPython: + py_version_and_tag_str, platform_str, sha256, filesize_str, url = (x.strip() for x in parts) + + try: + maybe_py_version, maybe_pbs_release_tag = _parse_py_version_and_pbs_release_tag( + py_version_and_tag_str + ) + except ValueError: + raise ExternalToolError( + f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " + f"the value `{orig_value}` declares version `{py_version_and_tag_str}` in the first field, " + "but it could not be parsed as a PBS release version." + ) + + if platform_str not in ( + Platform.linux_x86_64.value, + Platform.linux_arm64.value, + Platform.macos_x86_64.value, + Platform.macos_arm64.value, + ): + raise ExternalToolError( + f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " + f"the value `{orig_value}` declares platforn `{platform_str}` in the second field, " + "but that value is not a known Pants platform. It must be one of " + "`linux_x86_64`, `linux_arm64`, `macos_x86_64`, or `macos_arm64`." + ) + platform: Platform = Platform(platform_str) + + if len(sha256) != 64 or not re.match("^[a-zA-Z0-9]+$", sha256): + raise ExternalToolError( + f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " + f"the value `{orig_value}` declares SHA256 checksum `{sha256}` in the third field, " + "but that value does not parse as a SHA256 checksum." + ) + + try: + filesize: int = int(filesize_str) + except ValueError: + raise ExternalToolError( + f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " + f"the value `{orig_value}` declares file size `{filesize_str}` in the fourth field, " + "but that value does not parse as an integer." + ) + + maybe_inferred_py_version: Version | None = None + maybe_inferred_pbs_release_tag: Version | None = None + maybe_inferred_platform: Platform | None = None + try: + ( + maybe_inferred_py_version, + maybe_inferred_pbs_release_tag, + maybe_inferred_platform, + ) = _parse_pbs_url(url) + except ValueError: + pass + + match maybe_py_version: + case None: + if maybe_inferred_py_version is None: + raise ExternalToolError( + f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " + f"the value `{orig_value}` does not declare a version in the first field, and no version " + "could be inferred from the URL." + ) + + maybe_py_version = maybe_inferred_py_version + case py_version: + if maybe_inferred_py_version is not None and py_version != maybe_inferred_py_version: + logger.warning( + f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " + f"the value `{orig_value}` declares version `{py_version}` in the first field, but Pants inferred " + f"version `{maybe_inferred_py_version}` from the URL." + ) + + match maybe_pbs_release_tag: + case None: + if maybe_inferred_pbs_release_tag is None: + raise ExternalToolError( + f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " + f"the value `{orig_value}` does not declare a PBS release tag in the first field, and no PBS " + "release tag could be inferred from the URL." + ) + + maybe_pbs_release_tag = maybe_inferred_pbs_release_tag + case pbs_release_tag: + if ( + maybe_inferred_pbs_release_tag is not None + and pbs_release_tag != maybe_inferred_pbs_release_tag + ): + logger.warning( + f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " + f"the value `{orig_value}` declares PBS release tag `{pbs_release_tag}` in the first field, but Pants inferred " + f"PBS release tag `{maybe_inferred_pbs_release_tag}` from the URL." + ) + + if maybe_inferred_platform is not None and platform != maybe_inferred_platform: + logger.warning( + f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " + f"the value `{orig_value}` declares platform `{platform}` in the third field, but Pants inferred " + f"platform `{maybe_inferred_platform}` from the URL." + ) + + return _ParsedPBSPython( + py_version=maybe_py_version, + pbs_release_tag=maybe_pbs_release_tag, + platform=platform, + url=url, + sha256=sha256, + size=filesize, + ) + + @functools.cache def load_pbs_pythons() -> PBSVersionsT: versions_info = json.loads(read_sibling_resource(__name__, "versions_info.json")) @@ -166,68 +386,49 @@ def release_constraints(self) -> ConstraintsList: f"The `[{PBSPythonProviderSubsystem.options_scope}].release_constraints option` is not valid: {e}" ) from None - def get_user_supplied_pbs_pythons(self, require_tag: bool) -> PBSVersionsT: - extract_re = re.compile(r"^cpython-([0-9.]+)\+([0-9]+)-.*\.tar\.\w+$") - - def extract_version_and_tag(url: str) -> tuple[str, str] | None: - parsed_url = urllib.parse.urlparse(urllib.parse.unquote(url)) - base_path = posixpath.basename(parsed_url.path) - - nonlocal extract_re - if m := extract_re.fullmatch(base_path): - return (m.group(1), m.group(2)) - - return None - + def get_user_supplied_pbs_pythons(self) -> PBSVersionsT: user_supplied_pythons: dict[str, dict[str, dict[str, PBSPythonInfo]]] = {} for version_info in self.known_python_versions or []: - version_parts = version_info.split("|") - if len(version_parts) != 5: + version_parts = [x.strip() for x in version_info.split("|")] + if len(version_parts) not in (3, 5): raise ExternalToolError( f"Each value for the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option " - "must be five values separated by a `|` character as follows: PYTHON_VERSION|PLATFORM|SHA256|FILE_SIZE|URL " - f"\n\nInstead, the following value was provided: {version_info}" + "must be a set of three or five values separated by `|` characters as follows:\n\n" + "- 3 fields: URL|SHA256|FILE_SIZE\n\n" + "- 5 fields: PYTHON_VERSION+PBS_RELEASE|PLATFORM|SHA256|FILE_SIZE|URL\n\n" + "\n\nIf 3 fields are provided, Pants will attempt to infer values based on the URL which must " + "follow the PBS naming conventions.\n\n" + f"Instead, the following value was provided: {version_info}" ) - py_version, platform, sha256, filesize, url = (x.strip() for x in version_parts) - - tag: str | None = None - maybe_inferred_py_version_and_tag = extract_version_and_tag(url) - if maybe_inferred_py_version_and_tag: - inferred_py_version, inferred_tag = maybe_inferred_py_version_and_tag - if inferred_py_version != py_version: - raise ExternalToolError( - f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " - f"the value `{version_info}` declares Python version `{py_version}` in the first field, but the URL" - f"provided references Python version `{inferred_py_version}`. These must be the same." - ) - tag = inferred_tag - - if tag is None: - raise ExternalToolError( - f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " - f'the PBS release "tag" could not be inferred from the supplied URL: {url}' - "\n\nThis is an error because the option" - f"`[{PBSPythonProviderSubsystem.options_scope}].require_inferrable_release_tag` is set to True." - ) + info = ( + _parse_from_three_fields(version_parts, orig_value=version_info) + if len(version_parts) == 3 + else _parse_from_five_fields(version_parts, orig_value=version_info) + ) + + py_version: str = str(info.py_version) + pbs_release_tag: str = str(info.pbs_release_tag) if py_version not in user_supplied_pythons: user_supplied_pythons[py_version] = {} - if tag not in user_supplied_pythons[py_version]: - user_supplied_pythons[py_version][tag] = {} + if pbs_release_tag not in user_supplied_pythons[py_version]: + user_supplied_pythons[py_version][pbs_release_tag] = {} + + pbs_python_info = PBSPythonInfo(url=info.url, sha256=info.sha256, size=info.size) - pbs_python_info = PBSPythonInfo(url=url, sha256=sha256, size=int(filesize)) - user_supplied_pythons[py_version][tag][platform] = pbs_python_info + user_supplied_pythons[py_version][pbs_release_tag][ + info.platform.value + ] = pbs_python_info return user_supplied_pythons def get_all_pbs_pythons(self) -> PBSVersionsT: all_pythons = load_pbs_pythons().copy() - user_supplied_pythons: PBSVersionsT = self.get_user_supplied_pbs_pythons( - require_tag=self.require_inferrable_release_tag - ) + user_supplied_pythons: PBSVersionsT = self.get_user_supplied_pbs_pythons() + for py_version, release_metadatas_for_py_version in user_supplied_pythons.items(): for ( release_tag, diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules_integration_test.py b/src/python/pants/backend/python/providers/python_build_standalone/rules_integration_test.py index edfe6dd4bd4..87b60f9d3ea 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules_integration_test.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules_integration_test.py @@ -157,14 +157,14 @@ def test_tag_inference_from_url() -> None: subsystem = create_subsystem( pbs.PBSPythonProviderSubsystem, known_python_versions=[ - "3.10.13|linux_arm|abc123|123|https://github.com/indygreg/python-build-standalone/releases/download/20240224/cpython-3.10.13%2B20240224-aarch64-unknown-linux-gnu-install_only.tar.gz", + "3.10.13|linux_arm64|e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855|123|https://github.com/indygreg/python-build-standalone/releases/download/20240224/cpython-3.10.13%2B20240224-aarch64-unknown-linux-gnu-install_only.tar.gz", ], ) - user_supplied_pbs_versions = subsystem.get_user_supplied_pbs_pythons(require_tag=False) - assert user_supplied_pbs_versions["3.10.13"]["20240224"]["linux_arm"] == pbs.PBSPythonInfo( + user_supplied_pbs_versions = subsystem.get_user_supplied_pbs_pythons() + assert user_supplied_pbs_versions["3.10.13"]["20240224"]["linux_arm64"] == pbs.PBSPythonInfo( url="https://github.com/indygreg/python-build-standalone/releases/download/20240224/cpython-3.10.13%2B20240224-aarch64-unknown-linux-gnu-install_only.tar.gz", - sha256="abc123", + sha256="e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", size=123, ) @@ -172,13 +172,13 @@ def test_tag_inference_from_url() -> None: subsystem = create_subsystem( pbs.PBSPythonProviderSubsystem, known_python_versions=[ - "3.10.13|linux_arm|abc123|123|file:///releases/20240224/cpython.tar.gz", + "3.10.13|linux_arm64|e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855|123|file:///releases/20240224/cpython.tar.gz", ], ) with pytest.raises( - ExternalToolError, match='the PBS release "tag" could not be inferred from the supplied URL' + ExternalToolError, match="no PBS release tag could be inferred from the URL" ): - _ = subsystem.get_user_supplied_pbs_pythons(require_tag=True) + _ = subsystem.get_user_supplied_pbs_pythons() def test_venv_pex_reconstruction(rule_runner): @@ -221,22 +221,22 @@ def test_release_constraint_evaluation(rule_runner: RuleRunner) -> None: def make_platform_metadata(): return { "linux_arm64": { - "sha256": "abc123", + "sha256": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", "size": 1, "url": "foobar", }, "linux_x86_64": { - "sha256": "abc123", + "sha256": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", "size": 1, "url": "https://example.com/foo.zip", }, "macos_arm64": { - "sha256": "abc123", + "sha256": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", "size": 1, "url": "https://example.com/foo.zip", }, "macos_x86_64": { - "sha256": "abc123", + "sha256": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", "size": 1, "url": "https://example.com/foo.zip", }, From 4e37d0963901d3642de90559000dc61332abfd9f Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Fri, 20 Dec 2024 17:20:16 +0100 Subject: [PATCH 02/18] add parsing tests --- .../python_build_standalone/rules.py | 2 - .../python_build_standalone/rules_test.py | 47 +++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 src/python/pants/backend/python/providers/python_build_standalone/rules_test.py diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules.py b/src/python/pants/backend/python/providers/python_build_standalone/rules.py index e4102c4bdfe..9b7908a7254 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules.py @@ -150,8 +150,6 @@ def _parse_pbs_url(url: str) -> tuple[Version, Version, Platform]: f"because it does not follow the PBS naming convention: {url}" ) - # assert platform is not None - return py_version, pbs_release_tag, platform diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules_test.py b/src/python/pants/backend/python/providers/python_build_standalone/rules_test.py new file mode 100644 index 00000000000..12e5941734d --- /dev/null +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules_test.py @@ -0,0 +1,47 @@ +# Copyright 2024 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +from __future__ import annotations + +import pytest + +from pants.backend.python.providers.python_build_standalone.rules import ( + _parse_pbs_url, + _parse_py_version_and_pbs_release_tag, +) +from pants.engine.platform import Platform +from pants.version import Version + + +def test_parse_py_version_and_pbs_release_tag() -> None: + result1 = _parse_py_version_and_pbs_release_tag("") + assert result1 == (None, None) + + result2 = _parse_py_version_and_pbs_release_tag("1.2.3") + assert result2 == (Version("1.2.3"), None) + + result3 = _parse_py_version_and_pbs_release_tag("1.2.3+20241201") + assert result3 == (Version("1.2.3"), Version("20241201")) + + with pytest.raises(ValueError): + _parse_py_version_and_pbs_release_tag("xyzzy+20241201") + + with pytest.raises(ValueError): + _parse_py_version_and_pbs_release_tag("1.2.3+xyzzy") + + +def test_parse_pbs_url() -> None: + result1 = _parse_pbs_url( + "https://github.com/indygreg/python-build-standalone/releases/download/20240726/cpython-3.12.4%2B20240726-x86_64-unknown-linux-gnu-install_only_stripped.tar.gz" + ) + assert result1 == (Version("3.12.4"), Version("20240726"), Platform.linux_x86_64) + + with pytest.raises(ValueError, match="Unable to parse the Python version and PBS release tag"): + _parse_pbs_url( + "https://example.com/cpython-3.12.4-x86_64-unknown-linux-gnu-install_only_stripped.tar.gz" + ) + + with pytest.raises(ValueError, match="Unable to parse the platfornm"): + _parse_pbs_url( + "https://example.com/cpython-3.12.4%2B20240205-s390-unknown-linux-gnu-install_only_stripped.tar.gz" + ) From 422e1d84619221c3e690edd7ee805eade8d42d73 Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Fri, 20 Dec 2024 17:56:18 +0100 Subject: [PATCH 03/18] don't use match for platform + tests of five fields parsing --- .../python_build_standalone/rules.py | 43 +++++++++----- .../python_build_standalone/rules_test.py | 57 +++++++++++++++++++ 2 files changed, 85 insertions(+), 15 deletions(-) diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules.py b/src/python/pants/backend/python/providers/python_build_standalone/rules.py index 9b7908a7254..c2f03175804 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules.py @@ -189,19 +189,23 @@ def _parse_from_five_fields(parts: Sequence[str], orig_value: str) -> _ParsedPBS "but it could not be parsed as a PBS release version." ) - if platform_str not in ( + maybe_platform: Platform | None = None + if not platform_str: + pass + elif platform_str in ( Platform.linux_x86_64.value, Platform.linux_arm64.value, Platform.macos_x86_64.value, Platform.macos_arm64.value, ): + maybe_platform = Platform(platform_str) + else: raise ExternalToolError( f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " f"the value `{orig_value}` declares platforn `{platform_str}` in the second field, " "but that value is not a known Pants platform. It must be one of " "`linux_x86_64`, `linux_arm64`, `macos_x86_64`, or `macos_arm64`." ) - platform: Platform = Platform(platform_str) if len(sha256) != 64 or not re.match("^[a-zA-Z0-9]+$", sha256): raise ExternalToolError( @@ -223,13 +227,12 @@ def _parse_from_five_fields(parts: Sequence[str], orig_value: str) -> _ParsedPBS maybe_inferred_pbs_release_tag: Version | None = None maybe_inferred_platform: Platform | None = None try: - ( - maybe_inferred_py_version, - maybe_inferred_pbs_release_tag, - maybe_inferred_platform, - ) = _parse_pbs_url(url) + v1, v2, p = _parse_pbs_url(url) + maybe_inferred_py_version = v1 + maybe_inferred_pbs_release_tag = v2 + maybe_inferred_platform = p except ValueError: - pass + maybe_inferred_platform = None match maybe_py_version: case None: @@ -270,17 +273,27 @@ def _parse_from_five_fields(parts: Sequence[str], orig_value: str) -> _ParsedPBS f"PBS release tag `{maybe_inferred_pbs_release_tag}` from the URL." ) - if maybe_inferred_platform is not None and platform != maybe_inferred_platform: - logger.warning( - f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " - f"the value `{orig_value}` declares platform `{platform}` in the third field, but Pants inferred " - f"platform `{maybe_inferred_platform}` from the URL." - ) + if maybe_platform is None: + if maybe_inferred_platform is None: + raise ExternalToolError( + f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " + f"the value `{orig_value}` does not declare a platform in the second field, and no platform " + "could be inferred from the URL." + ) + + maybe_platform = maybe_inferred_platform + else: + if maybe_inferred_platform is not None and maybe_platform != maybe_inferred_platform: + logger.warning( + f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " + f"the value `{orig_value}` declares platform `{maybe_platform}` in the second field, but Pants inferred " + f"platform `{maybe_inferred_platform}` from the URL." + ) return _ParsedPBSPython( py_version=maybe_py_version, pbs_release_tag=maybe_pbs_release_tag, - platform=platform, + platform=maybe_platform, url=url, sha256=sha256, size=filesize, diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules_test.py b/src/python/pants/backend/python/providers/python_build_standalone/rules_test.py index 12e5941734d..e4cd24f5585 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules_test.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules_test.py @@ -6,9 +6,12 @@ import pytest from pants.backend.python.providers.python_build_standalone.rules import ( + _parse_from_five_fields, _parse_pbs_url, _parse_py_version_and_pbs_release_tag, + _ParsedPBSPython, ) +from pants.core.util_rules.external_tool import ExternalToolError from pants.engine.platform import Platform from pants.version import Version @@ -45,3 +48,57 @@ def test_parse_pbs_url() -> None: _parse_pbs_url( "https://example.com/cpython-3.12.4%2B20240205-s390-unknown-linux-gnu-install_only_stripped.tar.gz" ) + + +def test_parse_from_five_fields() -> None: + def invoke(s: str) -> _ParsedPBSPython: + parts = s.split("|") + return _parse_from_five_fields(parts, orig_value=s) + + result1 = invoke( + "3.9.16|linux_x86_64|f885f3d011ab08e4d9521a7ae2662e9e0073acc0305a1178984b5a1cf057309a|26767987|https://github.com/indygreg/python-build-standalone/releases/download/20221220/cpython-3.9.16%2B20221220-x86_64-unknown-linux-gnu-install_only.tar.gz" + ) + assert result1 == _ParsedPBSPython( + py_version=Version("3.9.16"), + pbs_release_tag=Version("20221220"), + platform=Platform.linux_x86_64, + url="https://github.com/indygreg/python-build-standalone/releases/download/20221220/cpython-3.9.16%2B20221220-x86_64-unknown-linux-gnu-install_only.tar.gz", + sha256="f885f3d011ab08e4d9521a7ae2662e9e0073acc0305a1178984b5a1cf057309a", + size=26767987, + ) + + with pytest.raises( + ExternalToolError, + match="does not declare a version in the first field, and no version could be inferred from the URL", + ): + invoke( + "||e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855|123|https://dl.example.com/cpython.tar.gz" + ) + + with pytest.raises( + ExternalToolError, + match="does not declare a PBS release tag in the first field, and no PBS release tag could be inferred from the URL", + ): + invoke( + "3.10.1||e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855|123|https://dl.example.com/cpython.tar.gz" + ) + + with pytest.raises( + ExternalToolError, + match="does not declare a platform in the second field, and no platform could be inferred from the URL", + ): + invoke( + "3.10.1+20240601||e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855|123|https://dl.example.com/cpython.tar.gz" + ) + + result2 = invoke( + "3.10.1+20240601|linux_x86_64|e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855|123|https://dl.example.com/cpython.tar.gz" + ) + assert result2 == _ParsedPBSPython( + py_version=Version("3.10.1"), + pbs_release_tag=Version("20240601"), + platform=Platform.linux_x86_64, + url="https://dl.example.com/cpython.tar.gz", + sha256="e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + size=123, + ) From 19b425704e2f1ca5e7e78366dba89fd196b4082c Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Fri, 20 Dec 2024 18:03:35 +0100 Subject: [PATCH 04/18] test for parse_from_three_fields (w/ bug fix) --- .../python_build_standalone/rules.py | 4 +++- .../python_build_standalone/rules_test.py | 24 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules.py b/src/python/pants/backend/python/providers/python_build_standalone/rules.py index c2f03175804..be8b9c53440 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules.py @@ -154,7 +154,8 @@ def _parse_pbs_url(url: str) -> tuple[Version, Version, Platform]: def _parse_from_three_fields(parts: Sequence[str], orig_value: str) -> _ParsedPBSPython: - url, sha256, size = parts[0:2] + assert len(parts) == 3 + url, sha256, size = parts try: py_version, pbs_release_tag, platform = _parse_pbs_url(url) @@ -176,6 +177,7 @@ def _parse_from_three_fields(parts: Sequence[str], orig_value: str) -> _ParsedPB def _parse_from_five_fields(parts: Sequence[str], orig_value: str) -> _ParsedPBSPython: + assert len(parts) == 5 py_version_and_tag_str, platform_str, sha256, filesize_str, url = (x.strip() for x in parts) try: diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules_test.py b/src/python/pants/backend/python/providers/python_build_standalone/rules_test.py index e4cd24f5585..b5fe76b2eb8 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules_test.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules_test.py @@ -7,6 +7,7 @@ from pants.backend.python.providers.python_build_standalone.rules import ( _parse_from_five_fields, + _parse_from_three_fields, _parse_pbs_url, _parse_py_version_and_pbs_release_tag, _ParsedPBSPython, @@ -50,6 +51,29 @@ def test_parse_pbs_url() -> None: ) +def test_parse_from_three_fields() -> None: + def invoke(s: str) -> _ParsedPBSPython: + parts = s.split("|") + return _parse_from_three_fields(parts, orig_value=s) + + result1 = invoke( + "https://github.com/indygreg/python-build-standalone/releases/download/20221220/cpython-3.9.16%2B20221220-x86_64-unknown-linux-gnu-install_only.tar.gz|f885f3d011ab08e4d9521a7ae2662e9e0073acc0305a1178984b5a1cf057309a|26767987" + ) + assert result1 == _ParsedPBSPython( + py_version=Version("3.9.16"), + pbs_release_tag=Version("20221220"), + platform=Platform.linux_x86_64, + url="https://github.com/indygreg/python-build-standalone/releases/download/20221220/cpython-3.9.16%2B20221220-x86_64-unknown-linux-gnu-install_only.tar.gz", + sha256="f885f3d011ab08e4d9521a7ae2662e9e0073acc0305a1178984b5a1cf057309a", + size=26767987, + ) + + with pytest.raises(ExternalToolError, match="since it does not have a cpython prefix"): + invoke( + "https://dl.example.com/cpython.tar.gz|f885f3d011ab08e4d9521a7ae2662e9e0073acc0305a1178984b5a1cf057309a|26767987" + ) + + def test_parse_from_five_fields() -> None: def invoke(s: str) -> _ParsedPBSPython: parts = s.split("|") From 6b700a8dd268e8ddbf5dffb80a9863d149b739b4 Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Fri, 20 Dec 2024 18:05:38 +0100 Subject: [PATCH 05/18] use 3-field form in integration test --- .../python_build_standalone/rules_integration_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules_integration_test.py b/src/python/pants/backend/python/providers/python_build_standalone/rules_integration_test.py index 87b60f9d3ea..f0beef6487f 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules_integration_test.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules_integration_test.py @@ -143,8 +143,8 @@ def test_additional_versions(rule_runner, mock_empty_versions_resource): "--python-build-standalone-python-provider-known-python-versions=[" + "'3.9.16|linux_arm64|75f3d10ae8933e17bf27e8572466ff8a1e7792f521d33acba578cc8a25d82e0b|24540128|https://github.com/indygreg/python-build-standalone/releases/download/20221220/cpython-3.9.16%2B20221220-aarch64-unknown-linux-gnu-install_only.tar.gz'," + "'3.9.16|macos_arm64|73bad3a610a0ff14166fbd5045cd186084bd2ce99edd2c6327054509e790b9ab|16765350|https://github.com/indygreg/python-build-standalone/releases/download/20221220/cpython-3.9.16%2B20221220-aarch64-apple-darwin-install_only.tar.gz'," - + "'3.9.16|linux_x86_64|f885f3d011ab08e4d9521a7ae2662e9e0073acc0305a1178984b5a1cf057309a|26767987|https://github.com/indygreg/python-build-standalone/releases/download/20221220/cpython-3.9.16%2B20221220-x86_64-unknown-linux-gnu-install_only.tar.gz'," - + "'3.9.16|macos_x86_64|69331e93656b179fcbfec0d506dfca11d899fe5dced990b28915e41755ce215c|17151321|https://github.com/indygreg/python-build-standalone/releases/download/20221220/cpython-3.9.16%2B20221220-x86_64-apple-darwin-install_only.tar.gz'," + + "'https://github.com/indygreg/python-build-standalone/releases/download/20221220/cpython-3.9.16%2B20221220-x86_64-unknown-linux-gnu-install_only.tar.gz|f885f3d011ab08e4d9521a7ae2662e9e0073acc0305a1178984b5a1cf057309a|26767987'," + + "'https://github.com/indygreg/python-build-standalone/releases/download/20221220/cpython-3.9.16%2B20221220-x86_64-apple-darwin-install_only.tar.gz|69331e93656b179fcbfec0d506dfca11d899fe5dced990b28915e41755ce215c|17151321'," + "]" ], ) From 3314570ece592dd9763664ec33a125f82202b0e2 Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Fri, 20 Dec 2024 18:08:34 +0100 Subject: [PATCH 06/18] remove match statements in favor of if - less right indent --- .../python_build_standalone/rules.py | 68 +++++++++---------- 1 file changed, 33 insertions(+), 35 deletions(-) diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules.py b/src/python/pants/backend/python/providers/python_build_standalone/rules.py index be8b9c53440..8f37c138adb 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules.py @@ -236,44 +236,42 @@ def _parse_from_five_fields(parts: Sequence[str], orig_value: str) -> _ParsedPBS except ValueError: maybe_inferred_platform = None - match maybe_py_version: - case None: - if maybe_inferred_py_version is None: - raise ExternalToolError( - f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " - f"the value `{orig_value}` does not declare a version in the first field, and no version " - "could be inferred from the URL." - ) + if maybe_py_version is None: + if maybe_inferred_py_version is None: + raise ExternalToolError( + f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " + f"the value `{orig_value}` does not declare a version in the first field, and no version " + "could be inferred from the URL." + ) - maybe_py_version = maybe_inferred_py_version - case py_version: - if maybe_inferred_py_version is not None and py_version != maybe_inferred_py_version: - logger.warning( - f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " - f"the value `{orig_value}` declares version `{py_version}` in the first field, but Pants inferred " - f"version `{maybe_inferred_py_version}` from the URL." - ) + maybe_py_version = maybe_inferred_py_version + else: + if maybe_inferred_py_version is not None and maybe_py_version != maybe_inferred_py_version: + logger.warning( + f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " + f"the value `{orig_value}` declares version `{maybe_py_version}` in the first field, but Pants inferred " + f"version `{maybe_inferred_py_version}` from the URL." + ) - match maybe_pbs_release_tag: - case None: - if maybe_inferred_pbs_release_tag is None: - raise ExternalToolError( - f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " - f"the value `{orig_value}` does not declare a PBS release tag in the first field, and no PBS " - "release tag could be inferred from the URL." - ) + if maybe_pbs_release_tag is None: + if maybe_inferred_pbs_release_tag is None: + raise ExternalToolError( + f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " + f"the value `{orig_value}` does not declare a PBS release tag in the first field, and no PBS " + "release tag could be inferred from the URL." + ) - maybe_pbs_release_tag = maybe_inferred_pbs_release_tag - case pbs_release_tag: - if ( - maybe_inferred_pbs_release_tag is not None - and pbs_release_tag != maybe_inferred_pbs_release_tag - ): - logger.warning( - f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " - f"the value `{orig_value}` declares PBS release tag `{pbs_release_tag}` in the first field, but Pants inferred " - f"PBS release tag `{maybe_inferred_pbs_release_tag}` from the URL." - ) + maybe_pbs_release_tag = maybe_inferred_pbs_release_tag + else: + if ( + maybe_inferred_pbs_release_tag is not None + and maybe_pbs_release_tag != maybe_inferred_pbs_release_tag + ): + logger.warning( + f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " + f"the value `{orig_value}` declares PBS release tag `{maybe_pbs_release_tag}` in the first field, but Pants inferred " + f"PBS release tag `{maybe_inferred_pbs_release_tag}` from the URL." + ) if maybe_platform is None: if maybe_inferred_platform is None: From 41e2b3dd3fde92406537b81e220005ff86a792be Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Fri, 20 Dec 2024 23:10:35 +0100 Subject: [PATCH 07/18] release notes --- docs/notes/2.25.x.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/notes/2.25.x.md b/docs/notes/2.25.x.md index 19d43735c2d..4a8177a4457 100644 --- a/docs/notes/2.25.x.md +++ b/docs/notes/2.25.x.md @@ -62,9 +62,13 @@ The `helm_infer.external_docker_images` glob syntax has been generalized. In ad The AWS Lambda backend now provides built-in complete platforms for the Python 3.13 runtime. -The Python Build Standalone backend (`pants.backend.python.providers.experimental.python_build_standalone`) now supports filtering PBS releases via their "release tag" via [the new `--python-build-standalone-release-constraints` option](https://www.pantsbuild.org/2.25/reference/subsystems/python-build-standalone-python-provider#release_constraints). THe PBS "known versions" database now contains metadata on all known PBS versions, and not just the latest PBS release tag per Python patchlevel. +Several improvements to the Python Build Standalone backend (`pants.backend.python.providers.experimental.python_build_standalone`): -Also, the PBS "release tag" will be inferred for PBS releases supplied via the `--python-build-standalone-known-python-versions` option from the given URLs if those URLs conform to the naming convention used by the PBS project. The new advanced option `--python-build-standalone-require-inferrable-release-tag` controls whether Pants requires the tag to be inferrable. This option currently defaults to `False`, but will be migrated to `True` in a future Pants release. (The release tag cannot currently be specified via `--python-build-standalone-known-python-versions` since changing that option would not be a backward compatible change.) +- The backend now supports filtering PBS releases via their "release tag" via [the new `--python-build-standalone-release-constraints` option](https://www.pantsbuild.org/2.25/reference/subsystems/python-build-standalone-python-provider#release_constraints). THe PBS "known versions" database now contains metadata on all known PBS versions, and not just the latest PBS release tag per Python patchlevel. + +- The backend will now infer metadata for a PBS release from a given URL if the URL conforms to the naming convention used by the PBS project. The inferred metadata is Python version, PBS release tag, and platform. + +- The `--python-build-standalone-known-python-versions` option now accepts a three field format where each value is `URL|SHA256|FILE_SIZE`. All of the PBS release metadata will be parsed from the URL (which must use the naming convention used by the PBS project). (The existing five-field format is still accepted and will now allow the version and platform fields to be blank if that data can be inferred from the URL.) The default version of the [Pex](https://docs.pex-tool.org/) tool has been updated from 2.20.3 to [2.27.1](https://github.com/pex-tool/pex/releases/tag/v2.24.3). Among many improvements and bug fixes, this unlocks support for pip [24.3.1](https://pip.pypa.io/en/stable/news/#v24-3-1). From 5b3235e4515a20fa389e87df33e6787610ef2039 Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Fri, 20 Dec 2024 23:15:01 +0100 Subject: [PATCH 08/18] delete require_inferrable_release_tag --- .../providers/python_build_standalone/rules.py | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules.py b/src/python/pants/backend/python/providers/python_build_standalone/rules.py index 8f37c138adb..dd28205f56f 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules.py @@ -49,7 +49,7 @@ from pants.engine.unions import UnionRule from pants.option.errors import OptionsError from pants.option.global_options import NamedCachesDirOption -from pants.option.option_types import BoolOption, StrListOption, StrOption +from pants.option.option_types import StrListOption, StrOption from pants.option.subsystem import Subsystem from pants.util.docutil import bin_name from pants.util.frozendict import FrozenDict @@ -372,18 +372,6 @@ class PBSPythonProviderSubsystem(Subsystem): ), ) - require_inferrable_release_tag = BoolOption( - default=False, - help=textwrap.dedent( - """ - Normally, Pants will try to infer the PBS release "tag" from URLs supplied to the - `--python-build-standalone-known-python-versions` option. If this option is True, - then it is an error if Pants cannot infer the tag from the URL. - """ - ), - advanced=True, - ) - @memoized_property def release_constraints(self) -> ConstraintsList: rcs = self._release_constraints From c5dbf5d530bd9aeed3ab1cde4f658e0ff32c7674 Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Sat, 21 Dec 2024 00:12:48 +0100 Subject: [PATCH 09/18] update option's help text --- .../providers/python_build_standalone/rules.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules.py b/src/python/pants/backend/python/providers/python_build_standalone/rules.py index dd28205f56f..8eb97cb42c2 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules.py @@ -341,9 +341,11 @@ class PBSPythonProviderSubsystem(Subsystem): f""" Known versions to verify downloads against. - Each element is a pipe-separated string of `version|platform|sha256|length|url`, where: + Each element is a pipe-separated string of either `py_version+pbs_release_tag|platform|sha256|length|url` or + `url|sha256|length`, where: - - `version` is the version string + - `py_version` is the Python version string + - `pbs_release_tag` is the PBS release tag (i.e., the PBS-specific version) - `platform` is one of `[{','.join(Platform.__members__.keys())}]` - `sha256` is the 64-character hex representation of the expected sha256 digest of the download file, as emitted by `shasum -a 256` @@ -351,9 +353,13 @@ class PBSPythonProviderSubsystem(Subsystem): `wc -c` - `url` is the download URL to the `.tar.gz` archive - E.g., `3.1.2|macos_x86_64|6d0f18cd84b918c7b3edd0203e75569e0c7caecb1367bbbe409b44e28514f5be|42813|https://`. + E.g., `3.1.2|macos_x86_64|6d0f18cd84b918c7b3edd0203e75569e0c7caecb1367bbbe409b44e28514f5be|42813|https://` + or `https://|6d0f18cd84b918c7b3edd0203e75569e0c7caecb1367bbbe409b44e28514f5be|42813`. - Values are space-stripped, so pipes can be indented for readability if necessary. + Values are space-stripped, so pipes can be indented for readability if necessary. If the three field + format is used, then Pants will infer the `py_version`, `pbs_release_tag`, and `platform` fields from + the URL. With the five field format, one or more of `py_version`, `pbs_release_tag`, and `platform` + may be left blank if Pants can infer the field from the URL. Additionally, any versions you specify here will override the default Pants metadata for that version. From fbc5778032b581aeb7d7c2b3a2ea4d85b84e1311 Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Sat, 21 Dec 2024 00:16:22 +0100 Subject: [PATCH 10/18] 2 fields is 1 split --- .../backend/python/providers/python_build_standalone/rules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules.py b/src/python/pants/backend/python/providers/python_build_standalone/rules.py index 8eb97cb42c2..1310ab6b8b4 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules.py @@ -92,7 +92,7 @@ def _parse_py_version_and_pbs_release_tag( if not version_and_tag: return None, None - parts = version_and_tag.split("+", 2) + parts = version_and_tag.split("+", 1) py_version: Version | None = None pbs_release_tag: Version | None = None From e8cd3e41c6ae5f9ceef1fce974b2a08d97fce202 Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Sat, 21 Dec 2024 03:10:44 +0100 Subject: [PATCH 11/18] simplify code --- .../python/providers/python_build_standalone/rules.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules.py b/src/python/pants/backend/python/providers/python_build_standalone/rules.py index 1310ab6b8b4..a3bc73c8e9d 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules.py @@ -229,12 +229,13 @@ def _parse_from_five_fields(parts: Sequence[str], orig_value: str) -> _ParsedPBS maybe_inferred_pbs_release_tag: Version | None = None maybe_inferred_platform: Platform | None = None try: - v1, v2, p = _parse_pbs_url(url) - maybe_inferred_py_version = v1 - maybe_inferred_pbs_release_tag = v2 - maybe_inferred_platform = p + ( + maybe_inferred_py_version, + maybe_inferred_pbs_release_tag, + maybe_inferred_platform, + ) = _parse_pbs_url(url) except ValueError: - maybe_inferred_platform = None + pass if maybe_py_version is None: if maybe_inferred_py_version is None: From 5ea6aa1e719c0c681b5153f94cb924c0986907a5 Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Tue, 24 Dec 2024 00:03:35 +0100 Subject: [PATCH 12/18] match on musl libc --- .../backend/python/providers/python_build_standalone/rules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules.py b/src/python/pants/backend/python/providers/python_build_standalone/rules.py index a3bc73c8e9d..85c149f8227 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules.py @@ -136,7 +136,7 @@ def _parse_pbs_url(url: str) -> tuple[Version, Version, Platform]: platform: Platform match base_path_parts[1].split("-"): - case ["x86_64", "unknown", "linux", "gnu", *_]: + case ["x86_64", "unknown", "linux", "gnu" | "musl", *_]: platform = Platform.linux_x86_64 case ["aarch64", "unknown", "linux", "gnu", *_]: platform = Platform.linux_arm64 From a8466ca642866b72d62bb561cc6b09c95e36c187 Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Tue, 24 Dec 2024 00:20:23 +0100 Subject: [PATCH 13/18] infer all platform variants for x86_64 --- .../python/providers/python_build_standalone/rules.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules.py b/src/python/pants/backend/python/providers/python_build_standalone/rules.py index 85c149f8227..724924e4922 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules.py @@ -136,7 +136,13 @@ def _parse_pbs_url(url: str) -> tuple[Version, Version, Platform]: platform: Platform match base_path_parts[1].split("-"): - case ["x86_64", "unknown", "linux", "gnu" | "musl", *_]: + case [ + "x86_64" | "x86_64_v2" | "x86_64_v3" | "x86_64_v4", + "unknown", + "linux", + "gnu" | "musl", + *_, + ]: platform = Platform.linux_x86_64 case ["aarch64", "unknown", "linux", "gnu", *_]: platform = Platform.linux_arm64 From 1d62025a0c6481187d880626f75286f3fbc227ed Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Tue, 24 Dec 2024 00:21:48 +0100 Subject: [PATCH 14/18] fix typo --- .../backend/python/providers/python_build_standalone/rules.py | 2 +- .../python/providers/python_build_standalone/rules_test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules.py b/src/python/pants/backend/python/providers/python_build_standalone/rules.py index 724924e4922..cbffe484c4d 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules.py @@ -152,7 +152,7 @@ def _parse_pbs_url(url: str) -> tuple[Version, Version, Platform]: platform = Platform.macos_arm64 case _: raise ValueError( - "Unable to parse the platfornm from the provided URL " + "Unable to parse the platform from the provided URL " f"because it does not follow the PBS naming convention: {url}" ) diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules_test.py b/src/python/pants/backend/python/providers/python_build_standalone/rules_test.py index b5fe76b2eb8..a5aca212a81 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules_test.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules_test.py @@ -45,7 +45,7 @@ def test_parse_pbs_url() -> None: "https://example.com/cpython-3.12.4-x86_64-unknown-linux-gnu-install_only_stripped.tar.gz" ) - with pytest.raises(ValueError, match="Unable to parse the platfornm"): + with pytest.raises(ValueError, match="Unable to parse the platform"): _parse_pbs_url( "https://example.com/cpython-3.12.4%2B20240205-s390-unknown-linux-gnu-install_only_stripped.tar.gz" ) From 290812da139c0dea825a8acd7ae612f13eb11159 Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Tue, 24 Dec 2024 00:44:58 +0100 Subject: [PATCH 15/18] refactor explicit vs inferred validation logic --- .../python_build_standalone/rules.py | 92 +++++++++---------- 1 file changed, 41 insertions(+), 51 deletions(-) diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules.py b/src/python/pants/backend/python/providers/python_build_standalone/rules.py index cbffe484c4d..d7850c03b6f 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules.py @@ -12,7 +12,7 @@ import uuid from dataclasses import dataclass from pathlib import PurePath -from typing import Iterable, Mapping, Sequence, TypedDict, cast +from typing import Iterable, Mapping, Sequence, TypedDict, TypeVar, cast from packaging.version import InvalidVersion @@ -65,6 +65,8 @@ PBS_NAMED_CACHE_NAME = "python_build_standalone" PBS_APPEND_ONLY_CACHES = FrozenDict({PBS_NAMED_CACHE_NAME: PBS_SANDBOX_NAME}) +_T = TypeVar("_T") # Define type variable "T" + class PBSPythonInfo(TypedDict): url: str @@ -243,59 +245,47 @@ def _parse_from_five_fields(parts: Sequence[str], orig_value: str) -> _ParsedPBS except ValueError: pass - if maybe_py_version is None: - if maybe_inferred_py_version is None: - raise ExternalToolError( - f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " - f"the value `{orig_value}` does not declare a version in the first field, and no version " - "could be inferred from the URL." - ) - - maybe_py_version = maybe_inferred_py_version - else: - if maybe_inferred_py_version is not None and maybe_py_version != maybe_inferred_py_version: - logger.warning( - f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " - f"the value `{orig_value}` declares version `{maybe_py_version}` in the first field, but Pants inferred " - f"version `{maybe_inferred_py_version}` from the URL." - ) - - if maybe_pbs_release_tag is None: - if maybe_inferred_pbs_release_tag is None: - raise ExternalToolError( - f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " - f"the value `{orig_value}` does not declare a PBS release tag in the first field, and no PBS " - "release tag could be inferred from the URL." - ) + def _validate_inferred( + *, explicit: _T | None, inferred: _T | None, description: str, field_pos: str + ) -> _T: + if explicit is None: + if inferred is None: + raise ExternalToolError( + f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " + f"the value `{orig_value}` does not declare a {description} in the {field_pos} field, and no {description} " + "could be inferred from the URL." + ) + else: + return inferred + else: + if inferred is not None and explicit != inferred: + logger.warning( + f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " + f"the value `{orig_value}` declares {description} `{explicit}` in the {field_pos} field, but Pants inferred " + f"{description} `{inferred}` from the URL." + ) + return explicit - maybe_pbs_release_tag = maybe_inferred_pbs_release_tag - else: - if ( - maybe_inferred_pbs_release_tag is not None - and maybe_pbs_release_tag != maybe_inferred_pbs_release_tag - ): - logger.warning( - f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " - f"the value `{orig_value}` declares PBS release tag `{maybe_pbs_release_tag}` in the first field, but Pants inferred " - f"PBS release tag `{maybe_inferred_pbs_release_tag}` from the URL." - ) + maybe_py_version = _validate_inferred( + explicit=maybe_py_version, + inferred=maybe_inferred_py_version, + description="version", + field_pos="first", + ) - if maybe_platform is None: - if maybe_inferred_platform is None: - raise ExternalToolError( - f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " - f"the value `{orig_value}` does not declare a platform in the second field, and no platform " - "could be inferred from the URL." - ) + maybe_pbs_release_tag = _validate_inferred( + explicit=maybe_pbs_release_tag, + inferred=maybe_inferred_pbs_release_tag, + description="PBS release tag", + field_pos="first", + ) - maybe_platform = maybe_inferred_platform - else: - if maybe_inferred_platform is not None and maybe_platform != maybe_inferred_platform: - logger.warning( - f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " - f"the value `{orig_value}` declares platform `{maybe_platform}` in the second field, but Pants inferred " - f"platform `{maybe_inferred_platform}` from the URL." - ) + maybe_platform = _validate_inferred( + explicit=maybe_platform, + inferred=maybe_inferred_platform, + description="platform", + field_pos="second", + ) return _ParsedPBSPython( py_version=maybe_py_version, From 053525e3a812271f8c5112f2290c898037d7bdb2 Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Tue, 24 Dec 2024 00:49:10 +0100 Subject: [PATCH 16/18] make error message less redundant --- .../backend/python/providers/python_build_standalone/rules.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules.py b/src/python/pants/backend/python/providers/python_build_standalone/rules.py index d7850c03b6f..31dfa7ab25c 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules.py @@ -170,8 +170,7 @@ def _parse_from_three_fields(parts: Sequence[str], orig_value: str) -> _ParsedPB except ValueError as e: raise ExternalToolError( f"While parsing the `[{PBSPythonProviderSubsystem.options_scope}].known_python_versions` option, " - f"the value `{orig_value}` could not be parsed because the URL does not follow the " - f"PBS naming convention: {e}" + f"the value `{orig_value}` could not be parsed: {e}" ) return _ParsedPBSPython( From b3289493db8d6a974582ec904fb8337359d286b9 Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Tue, 24 Dec 2024 00:54:41 +0100 Subject: [PATCH 17/18] put url last in 3-field form --- .../backend/python/providers/python_build_standalone/rules.py | 4 ++-- .../python_build_standalone/rules_integration_test.py | 4 ++-- .../python/providers/python_build_standalone/rules_test.py | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules.py b/src/python/pants/backend/python/providers/python_build_standalone/rules.py index 31dfa7ab25c..16fa2acec51 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules.py @@ -163,7 +163,7 @@ def _parse_pbs_url(url: str) -> tuple[Version, Version, Platform]: def _parse_from_three_fields(parts: Sequence[str], orig_value: str) -> _ParsedPBSPython: assert len(parts) == 3 - url, sha256, size = parts + sha256, size, url = parts try: py_version, pbs_release_tag, platform = _parse_pbs_url(url) @@ -338,7 +338,7 @@ class PBSPythonProviderSubsystem(Subsystem): Known versions to verify downloads against. Each element is a pipe-separated string of either `py_version+pbs_release_tag|platform|sha256|length|url` or - `url|sha256|length`, where: + `sha256|length|url`, where: - `py_version` is the Python version string - `pbs_release_tag` is the PBS release tag (i.e., the PBS-specific version) diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules_integration_test.py b/src/python/pants/backend/python/providers/python_build_standalone/rules_integration_test.py index 306ca67013a..b5e9231c517 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules_integration_test.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules_integration_test.py @@ -143,8 +143,8 @@ def test_additional_versions(rule_runner, mock_empty_versions_resource): "--python-build-standalone-python-provider-known-python-versions=[" + "'3.9.16|linux_arm64|75f3d10ae8933e17bf27e8572466ff8a1e7792f521d33acba578cc8a25d82e0b|24540128|https://github.com/astral-sh/python-build-standalone/releases/download/20221220/cpython-3.9.16%2B20221220-aarch64-unknown-linux-gnu-install_only.tar.gz'," + "'3.9.16|macos_arm64|73bad3a610a0ff14166fbd5045cd186084bd2ce99edd2c6327054509e790b9ab|16765350|https://github.com/astral-sh/python-build-standalone/releases/download/20221220/cpython-3.9.16%2B20221220-aarch64-apple-darwin-install_only.tar.gz'," - + "'https://github.com/astral-sh/python-build-standalone/releases/download/20221220/cpython-3.9.16%2B20221220-x86_64-unknown-linux-gnu-install_only.tar.gz|f885f3d011ab08e4d9521a7ae2662e9e0073acc0305a1178984b5a1cf057309a|26767987'," - + "'https://github.com/astral-sh/python-build-standalone/releases/download/20221220/cpython-3.9.16%2B20221220-x86_64-apple-darwin-install_only.tar.gz|69331e93656b179fcbfec0d506dfca11d899fe5dced990b28915e41755ce215c|17151321'," + + "'f885f3d011ab08e4d9521a7ae2662e9e0073acc0305a1178984b5a1cf057309a|26767987|https://github.com/astral-sh/python-build-standalone/releases/download/20221220/cpython-3.9.16%2B20221220-x86_64-unknown-linux-gnu-install_only.tar.gz'," + + "'69331e93656b179fcbfec0d506dfca11d899fe5dced990b28915e41755ce215c|17151321|https://github.com/astral-sh/python-build-standalone/releases/download/20221220/cpython-3.9.16%2B20221220-x86_64-apple-darwin-install_only.tar.gz'," + "]" ], ) diff --git a/src/python/pants/backend/python/providers/python_build_standalone/rules_test.py b/src/python/pants/backend/python/providers/python_build_standalone/rules_test.py index a5aca212a81..3406d7a3de7 100644 --- a/src/python/pants/backend/python/providers/python_build_standalone/rules_test.py +++ b/src/python/pants/backend/python/providers/python_build_standalone/rules_test.py @@ -57,7 +57,7 @@ def invoke(s: str) -> _ParsedPBSPython: return _parse_from_three_fields(parts, orig_value=s) result1 = invoke( - "https://github.com/indygreg/python-build-standalone/releases/download/20221220/cpython-3.9.16%2B20221220-x86_64-unknown-linux-gnu-install_only.tar.gz|f885f3d011ab08e4d9521a7ae2662e9e0073acc0305a1178984b5a1cf057309a|26767987" + "f885f3d011ab08e4d9521a7ae2662e9e0073acc0305a1178984b5a1cf057309a|26767987|https://github.com/indygreg/python-build-standalone/releases/download/20221220/cpython-3.9.16%2B20221220-x86_64-unknown-linux-gnu-install_only.tar.gz" ) assert result1 == _ParsedPBSPython( py_version=Version("3.9.16"), @@ -70,7 +70,7 @@ def invoke(s: str) -> _ParsedPBSPython: with pytest.raises(ExternalToolError, match="since it does not have a cpython prefix"): invoke( - "https://dl.example.com/cpython.tar.gz|f885f3d011ab08e4d9521a7ae2662e9e0073acc0305a1178984b5a1cf057309a|26767987" + "f885f3d011ab08e4d9521a7ae2662e9e0073acc0305a1178984b5a1cf057309a|26767987|https://dl.example.com/cpython.tar.gz" ) From 47d31c8eb9b8fe89f35d11dc539c8de80095eefb Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Tue, 24 Dec 2024 00:55:56 +0100 Subject: [PATCH 18/18] release notes update --- docs/notes/2.25.x.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/notes/2.25.x.md b/docs/notes/2.25.x.md index f2875d2fb4a..acff5c6b541 100644 --- a/docs/notes/2.25.x.md +++ b/docs/notes/2.25.x.md @@ -70,7 +70,7 @@ Several improvements to the Python Build Standalone backend (`pants.backend.pyth - The backend will now infer metadata for a PBS release from a given URL if the URL conforms to the naming convention used by the PBS project. The inferred metadata is Python version, PBS release tag, and platform. -- The `--python-build-standalone-known-python-versions` option now accepts a three field format where each value is `URL|SHA256|FILE_SIZE`. All of the PBS release metadata will be parsed from the URL (which must use the naming convention used by the PBS project). (The existing five-field format is still accepted and will now allow the version and platform fields to be blank if that data can be inferred from the URL.) +- The `--python-build-standalone-known-python-versions` option now accepts a three field format where each value is `SHA256|FILE_SIZE|URL`. All of the PBS release metadata will be parsed from the URL (which must use the naming convention used by the PBS project). (The existing five-field format is still accepted and will now allow the version and platform fields to be blank if that data can be inferred from the URL.) Reverence to Python Build Standalone not refer to the [GitHub organization](https://github.com/astral-sh/python-build-standalone) as described in [Transferring Python Build Standalone Stewardship to Astral](https://gregoryszorc.com/blog/2024/12/03/transferring-python-build-standalone-stewardship-to-astral/).