fix(pypi) backport python_full_version fix to Python

This has been fixed in the Starlark implementation in bazel-contrib#2793 and in this
PR I am backporting the changes to handle the full python version
target platform strings so that we can have the same behaviour for now.

At the same time I have simplified and got rid of the specialization
handling in the Python algorithm just like I did in the starlark, which
simplifies the tests and makes the algorithm more correct.

Work towards bazel-contrib#2830

Author: aignas

python/private/pypi/whl_installer/platform.py

@@ -18,7 +18,7 @@
 import sys
 from dataclasses import dataclass
 from enum import Enum
-from typing import Any, Dict, Iterator, List, Optional, Union
+from typing import Any, Dict, Iterator, List, Optional, Tuple, Union
 
 
 class OS(Enum):
@@ -77,25 +77,32 @@ def _as_int(value: Optional[Union[OS, Arch]]) -> int:
     return int(value.value)
 
 
-def host_interpreter_minor_version() -> int:
-    return sys.version_info.minor
+def host_interpreter_version() -> Tuple[int, int]:
+    return (sys.version_info.minor, sys.version_info.micro)
 
 
 @dataclass(frozen=True)
 class Platform:
     os: Optional[OS] = None
     arch: Optional[Arch] = None
     minor_version: Optional[int] = None
+    micro_version: Optional[int] = None
 
     @classmethod
     def all(
         cls,
         want_os: Optional[OS] = None,
         minor_version: Optional[int] = None,
+        micro_version: Optional[int] = None,
     ) -> List["Platform"]:
         return sorted(
             [
-                cls(os=os, arch=arch, minor_version=minor_version)
+                cls(
+                    os=os,
+                    arch=arch,
+                    minor_version=minor_version,
+                    micro_version=micro_version,
+                )
                 for os in OS
                 for arch in Arch
                 if not want_os or want_os == os
@@ -112,32 +119,16 @@ def host(cls) -> List["Platform"]:
             A list of parsed values which makes the signature the same as
             `Platform.all` and `Platform.from_string`.
         """
+        minor, micro = host_interpreter_version()
         return [
             Platform(
                 os=OS.interpreter(),
                 arch=Arch.interpreter(),
-                minor_version=host_interpreter_minor_version(),
+                minor_version=minor,
+                micro_version=micro,
             )
         ]
 
-    def all_specializations(self) -> Iterator["Platform"]:
-        """Return the platform itself and all its unambiguous specializations.
-
-        For more info about specializations see
-        https://bazel.build/docs/configurable-attributes
-        """
-        yield self
-        if self.arch is None:
-            for arch in Arch:
-                yield Platform(os=self.os, arch=arch, minor_version=self.minor_version)
-        if self.os is None:
-            for os in OS:
-                yield Platform(os=os, arch=self.arch, minor_version=self.minor_version)
-        if self.arch is None and self.os is None:
-            for os in OS:
-                for arch in Arch:
-                    yield Platform(os=os, arch=arch, minor_version=self.minor_version)
-
     def __lt__(self, other: Any) -> bool:
         """Add a comparison method, so that `sorted` returns the most specialized platforms first."""
         if not isinstance(other, Platform) or other is None:
@@ -153,24 +144,15 @@ def __lt__(self, other: Any) -> bool:
 
     def __str__(self) -> str:
         if self.minor_version is None:
-            if self.os is None and self.arch is None:
-                return "//conditions:default"
-
-            if self.arch is None:
-                return f"@platforms//os:{self.os}"
-            else:
-                return f"{self.os}_{self.arch}"
-
-        if self.arch is None and self.os is None:
-            return f"@//python/config_settings:is_python_3.{self.minor_version}"
+            return f"{self.os}_{self.arch}"
 
-        if self.arch is None:
-            return f"cp3{self.minor_version}_{self.os}_anyarch"
+        minor_version = self.minor_version
+        micro_version = self.micro_version
 
-        if self.os is None:
-            return f"cp3{self.minor_version}_anyos_{self.arch}"
-
-        return f"cp3{self.minor_version}_{self.os}_{self.arch}"
+        if micro_version is None:
+            return f"cp3{minor_version}_{self.os}_{self.arch}"
+        else:
+            return f"cp3{minor_version}.{micro_version}_{self.os}_{self.arch}"
 
     @classmethod
     def from_string(cls, platform: Union[str, List[str]]) -> List["Platform"]:
@@ -190,14 +172,25 @@ def from_string(cls, platform: Union[str, List[str]]) -> List["Platform"]:
             os, _, arch = tail.partition("_")
             arch = arch or "*"
 
-            minor_version = int(abi[len("cp3") :]) if abi else None
+            if abi:
+                tail = abi[len("cp3") :]
+                minor_version, _, micro_version = tail.partition(".")
+                minor_version = int(minor_version)
+                if micro_version == "":
+                    micro_version = None
+                else:
+                    micro_version = int(micro_version)
+            else:
+                minor_version = None
+                micro_version = None
 
             if arch != "*":
                 ret.add(
                     cls(
                         os=OS[os] if os != "*" else None,
                         arch=Arch[arch],
                         minor_version=minor_version,
+                        micro_version=micro_version,
                     )
                 )
 
@@ -206,6 +199,7 @@ def from_string(cls, platform: Union[str, List[str]]) -> List["Platform"]:
                     cls.all(
                         want_os=OS[os] if os != "*" else None,
                         minor_version=minor_version,
+                        micro_version=micro_version,
                     )
                 )
 
@@ -282,7 +276,12 @@ def platform_machine(self) -> str:
 
     def env_markers(self, extra: str) -> Dict[str, str]:
         # If it is None, use the host version
-        minor_version = self.minor_version or host_interpreter_minor_version()
+        if self.minor_version is None:
+            minor, micro = host_interpreter_version()
+        else:
+            minor, micro = self.minor_version, self.micro_version
+
+        micro = micro or 0
 
         return {
             "extra": extra,
@@ -292,12 +291,12 @@ def env_markers(self, extra: str) -> Dict[str, str]:
             "platform_system": self.platform_system,
             "platform_release": "",  # unset
             "platform_version": "",  # unset
-            "python_version": f"3.{minor_version}",
+            "python_version": f"3.{minor}",
             # FIXME @aignas 2024-01-14: is putting zero last a good idea? Maybe we should
             # use `20` or something else to avoid having weird issues where the full version is used for
             # matching and the author decides to only support 3.y.5 upwards.
-            "implementation_version": f"3.{minor_version}.0",
-            "python_full_version": f"3.{minor_version}.0",
+            "implementation_version": f"3.{minor}.{micro}",
+            "python_full_version": f"3.{minor}.{micro}",
             # we assume that the following are the same as the interpreter used to setup the deps:
             # "implementation_name": "cpython"
             # "platform_python_implementation: "CPython",

python/private/pypi/whl_installer/wheel.py

@@ -27,7 +27,7 @@
 
 from python.private.pypi.whl_installer.platform import (
     Platform,
-    host_interpreter_minor_version,
+    host_interpreter_version,
 )
 
 
@@ -62,12 +62,13 @@ def __init__(
         """
         self.name: str = Deps._normalize(name)
         self._platforms: Set[Platform] = platforms or set()
-        self._target_versions = {p.minor_version for p in platforms or {}}
-        self._default_minor_version = None
-        if platforms and len(self._target_versions) > 2:
+        self._target_versions = {(p.minor_version, p.micro_version) for p in platforms or {}}
+        if platforms and len(self._target_versions) > 1:
             # TODO @aignas 2024-06-23: enable this to be set via a CLI arg
             # for being more explicit.
-            self._default_minor_version = host_interpreter_minor_version()
+            self._default_minor_version, _ = host_interpreter_version()
+        else:
+            self._default_minor_version = None
 
         if None in self._target_versions and len(self._target_versions) > 2:
             raise ValueError(
@@ -88,8 +89,13 @@ def __init__(
         # Then add all of the requirements in order
         self._deps: Set[str] = set()
         self._select: Dict[Platform, Set[str]] = defaultdict(set)
+
+        reqs_by_name = {}
         for req in reqs:
-            self._add_req(req, want_extras)
+            reqs_by_name.setdefault(req.name, []).append(req)
+
+        for reqs in reqs_by_name.values():
+            self._add_req(reqs, want_extras)
 
     def _add(self, dep: str, platform: Optional[Platform]):
         dep = Deps._normalize(dep)
@@ -123,50 +129,6 @@ def _add(self, dep: str, platform: Optional[Platform]):
         # Add the platform-specific dep
         self._select[platform].add(dep)
 
-        # Add the dep to specializations of the given platform if they
-        # exist in the select statement.
-        for p in platform.all_specializations():
-            if p not in self._select:
-                continue
-
-            self._select[p].add(dep)
-
-        if len(self._select[platform]) == 1:
-            # We are adding a new item to the select and we need to ensure that
-            # existing dependencies from less specialized platforms are propagated
-            # to the newly added dependency set.
-            for p, deps in self._select.items():
-                # Check if the existing platform overlaps with the given platform
-                if p == platform or platform not in p.all_specializations():
-                    continue
-
-                self._select[platform].update(self._select[p])
-
-    def _maybe_add_common_dep(self, dep):
-        if len(self._target_versions) < 2:
-            return
-
-        platforms = [Platform()] + [
-            Platform(minor_version=v) for v in self._target_versions
-        ]
-
-        # If the dep is targeting all target python versions, lets add it to
-        # the common dependency list to simplify the select statements.
-        for p in platforms:
-            if p not in self._select:
-                return
-
-            if dep not in self._select[p]:
-                return
-
-        # All of the python version-specific branches have the dep, so lets add
-        # it to the common deps.
-        self._deps.add(dep)
-        for p in platforms:
-            self._select[p].remove(dep)
-            if not self._select[p]:
-                self._select.pop(p)
-
     @staticmethod
     def _normalize(name: str) -> str:
         return re.sub(r"[-_.]+", "_", name).lower()
@@ -227,66 +189,40 @@ def _resolve_extras(
 
         return extras
 
-    def _add_req(self, req: Requirement, extras: Set[str]) -> None:
-        if req.marker is None:
-            self._add(req.name, None)
-            return
+    def _add_req(self, reqs: List[Requirement], extras: Set[str]) -> None:
+        platforms_to_add = set()
+        for req in reqs:
+            if req.marker is None:
+                self._add(req.name, None)
+                return
 
-        marker_str = str(req.marker)
+            for plat in self._platforms:
+                if plat in platforms_to_add:
+                    # marker evaluation is more expensive than this check
+                    continue
 
-        if not self._platforms:
-            if any(req.marker.evaluate({"extra": extra}) for extra in extras):
-                self._add(req.name, None)
-            return
+                added = False
+                for extra in extras:
+                    if added:
+                        break
 
-        # NOTE @aignas 2023-12-08: in order to have reasonable select statements
-        # we do have to have some parsing of the markers, so it begs the question
-        # if packaging should be reimplemented in Starlark to have the best solution
-        # for now we will implement it in Python and see what the best parsing result
-        # can be before making this decision.
-        match_os = any(
-            tag in marker_str
-            for tag in [
-                "os_name",
-                "sys_platform",
-                "platform_system",
-            ]
-        )
-        match_arch = "platform_machine" in marker_str
-        match_version = "version" in marker_str
+                    if req.marker.evaluate(plat.env_markers(extra)):
+                        platforms_to_add.add(plat)
+                        added = True
+                        break
 
-        if not (match_os or match_arch or match_version):
-            if any(req.marker.evaluate({"extra": extra}) for extra in extras):
-                self._add(req.name, None)
+        if len(platforms_to_add) == len(self._platforms):
+            # the dep is in all target platforms, let's just add it to the regular
+            # list
+            self._add(req.name, None)
             return
 
-        for plat in self._platforms:
-            if not any(
-                req.marker.evaluate(plat.env_markers(extra)) for extra in extras
-            ):
-                continue
-
-            if match_arch and self._default_minor_version:
+        for plat in platforms_to_add:
+            if self._default_minor_version is not None:
                 self._add(req.name, plat)
-                if plat.minor_version == self._default_minor_version:
-                    self._add(req.name, Platform(plat.os, plat.arch))
-            elif match_arch:
-                self._add(req.name, Platform(plat.os, plat.arch))
-            elif match_os and self._default_minor_version:
-                self._add(req.name, Platform(plat.os, minor_version=plat.minor_version))
-                if plat.minor_version == self._default_minor_version:
-                    self._add(req.name, Platform(plat.os))
-            elif match_os:
-                self._add(req.name, Platform(plat.os))
-            elif match_version and self._default_minor_version:
-                self._add(req.name, Platform(minor_version=plat.minor_version))
-                if plat.minor_version == self._default_minor_version:
-                    self._add(req.name, Platform())
-            elif match_version:
-                self._add(req.name, None)
 
-        # Merge to common if possible after processing all platforms
-        self._maybe_add_common_dep(req.name)
+            if self._default_minor_version is None or plat.minor_version == self._default_minor_version:
+                self._add(req.name, Platform(os = plat.os, arch = plat.arch))
 
     def build(self) -> FrozenDeps:
         return FrozenDeps(