From bbfb496eec4c8e2b3150cbd9e600a07758df1ef1 Mon Sep 17 00:00:00 2001 From: mayeut Date: Sun, 29 Sep 2024 13:03:28 +0200 Subject: [PATCH] Add FlexibleVersion per review comment --- cibuildwheel/oci_container.py | 23 ++++++---------- cibuildwheel/util.py | 50 ++++++++++++++++++++++++++++++++++- unit_test/utils_test.py | 15 +++++++++++ 3 files changed, 72 insertions(+), 16 deletions(-) diff --git a/cibuildwheel/oci_container.py b/cibuildwheel/oci_container.py index 74b3237a5..bdd663b30 100644 --- a/cibuildwheel/oci_container.py +++ b/cibuildwheel/oci_container.py @@ -18,14 +18,13 @@ from types import TracebackType from typing import IO, Dict, Literal -from packaging.version import InvalidVersion, Version - from ._compat.typing import Self, assert_never from .errors import OCIEngineTooOldError from .logger import log from .typing import PathOrStr, PopenBytes from .util import ( CIProvider, + FlexibleVersion, call, detect_ci_provider, parse_key_value_string, @@ -104,11 +103,11 @@ def _check_engine_version(engine: OCIContainerEngineConfig) -> None: version_string = call(engine.name, "version", "-f", "{{json .}}", capture_stdout=True) version_info = json.loads(version_string.strip()) if engine.name == "docker": - client_api_version = Version(version_info["Client"]["ApiVersion"]) - server_api_version = Version(version_info["Server"]["ApiVersion"]) + client_api_version = FlexibleVersion(version_info["Client"]["ApiVersion"]) + server_api_version = FlexibleVersion(version_info["Server"]["ApiVersion"]) # --platform support was introduced in 1.32 as experimental, 1.41 removed the experimental flag version = min(client_api_version, server_api_version) - minimum_version = Version("1.41") + minimum_version = FlexibleVersion("1.41") minimum_version_str = "20.10.0" # docker version error_msg = textwrap.dedent( f""" @@ -120,20 +119,14 @@ def _check_engine_version(engine: OCIContainerEngineConfig) -> None: ) elif engine.name == "podman": # podman uses the same version string for "Version" & "ApiVersion" - # the version string is not PEP440 compliant here - def _version(version_string: str) -> Version: - for sep in ("-", "~", "^", "+"): - version_string = version_string.split(sep, maxsplit=1)[0] - return Version(version_string) - - client_version = _version(version_info["Client"]["Version"]) + client_version = FlexibleVersion(version_info["Client"]["Version"]) if "Server" in version_info: - server_version = _version(version_info["Server"]["Version"]) + server_version = FlexibleVersion(version_info["Server"]["Version"]) else: server_version = client_version # --platform support was introduced in v3 version = min(client_version, server_version) - minimum_version = Version("3") + minimum_version = FlexibleVersion("3") error_msg = textwrap.dedent( f""" Build failed because {engine.name} is too old. @@ -146,7 +139,7 @@ def _version(version_string: str) -> Version: assert_never(engine.name) if version < minimum_version: raise OCIEngineTooOldError(error_msg) from None - except (subprocess.CalledProcessError, KeyError, InvalidVersion) as e: + except (subprocess.CalledProcessError, KeyError, ValueError) as e: msg = f"Build failed because {engine.name} is too old or is not working properly." raise OCIEngineTooOldError(msg) from e diff --git a/cibuildwheel/util.py b/cibuildwheel/util.py index 31b3eaf86..b9db4268b 100644 --- a/cibuildwheel/util.py +++ b/cibuildwheel/util.py @@ -19,7 +19,7 @@ from collections.abc import Generator, Iterable, Mapping, MutableMapping, Sequence from dataclasses import dataclass from enum import Enum -from functools import lru_cache +from functools import lru_cache, total_ordering from pathlib import Path, PurePath from tempfile import TemporaryDirectory from time import sleep @@ -899,3 +899,51 @@ def combine_constraints( env["UV_CONSTRAINT"] = env["PIP_CONSTRAINT"] = " ".join( c for c in [our_constraints, user_constraints] if c ) + + +@total_ordering +class FlexibleVersion: + version_str: str + version_parts: tuple[int, ...] + suffix: str + + def __init__(self, version_str: str) -> None: + self.version_str = version_str + + # Split into numeric parts and the optional suffix + match = re.match(r"^[v]?(\d+(\.\d+)*)(.*)$", version_str) + if not match: + msg = f"Invalid version string: {version_str}" + raise ValueError(msg) + + version_part, _, suffix = match.groups() + + # Convert numeric version part into a tuple of integers + self.version_parts = tuple(map(int, version_part.split("."))) + self.suffix = suffix.strip() if suffix else "" + + # Normalize by removing trailing zeros + self.version_parts = self._remove_trailing_zeros(self.version_parts) + + def _remove_trailing_zeros(self, parts: tuple[int, ...]) -> tuple[int, ...]: + # Remove trailing zeros for accurate comparisons + # without this, "3.0" would be considered greater than "3" + while parts and parts[-1] == 0: + parts = parts[:-1] + return parts + + def __eq__(self, other: object) -> bool: + if not isinstance(other, FlexibleVersion): + raise NotImplementedError() + return (self.version_parts, self.suffix) == (other.version_parts, other.suffix) + + def __lt__(self, other: object) -> bool: + if not isinstance(other, FlexibleVersion): + raise NotImplementedError() + return (self.version_parts, self.suffix) < (other.version_parts, other.suffix) + + def __repr__(self) -> str: + return f"FlexibleVersion('{self.version_str}')" + + def __str__(self) -> str: + return self.version_str diff --git a/unit_test/utils_test.py b/unit_test/utils_test.py index 4725163ff..b433800a5 100644 --- a/unit_test/utils_test.py +++ b/unit_test/utils_test.py @@ -6,6 +6,7 @@ import pytest from cibuildwheel.util import ( + FlexibleVersion, find_compatible_wheel, fix_ansi_codes_for_github_actions, format_safe, @@ -206,3 +207,17 @@ def test_parse_key_value_string(): "name": ["docker"], "create_args": [], } + + +def test_flexible_version_comparisons(): + assert FlexibleVersion("2.0") == FlexibleVersion("2") + assert FlexibleVersion("2.0") < FlexibleVersion("2.1") + assert FlexibleVersion("2.1") > FlexibleVersion("2") + assert FlexibleVersion("1.9.9") < FlexibleVersion("2.0") + assert FlexibleVersion("1.10") > FlexibleVersion("1.9.9") + assert FlexibleVersion("3.0.1") > FlexibleVersion("3.0") + assert FlexibleVersion("3.0") < FlexibleVersion("3.0.1") + # Suffix should not affect comparisons + assert FlexibleVersion("1.0.1-rhel") > FlexibleVersion("1.0") + assert FlexibleVersion("1.0.1-rhel") < FlexibleVersion("1.1") + assert FlexibleVersion("1.0.1") == FlexibleVersion("v1.0.1")