fix(whl_library): correctly handle arch-specific deps in wheels (#2007)

It seems that there was a single-line bug that did not allow us to
handle arch-specific deps and since the percentage of such wheels is
extremely low, it went under the radar for quite some time.

I am going to not implement any explicit passing of the default python
version to `whl_library` as it is a much bigger change. Just doing this
could be sufficient for the time being.

Fixes #1996

Author: aignas

CHANGELOG.md

@@ -41,6 +41,9 @@ A brief description of the categories of changes:
   `interpreter_version_info` arg.
 * (bzlmod) Correctly pass `isolated`, `quiet` and `timeout` values to `whl_library`
   and drop the defaults from the lock file.
+* (whl_library) Correctly handle arch-specific dependencies when we encounter a
+  platform specific wheel and use `experimental_target_platforms`.
+  Fixes [#1996](https://github.com/bazelbuild/rules_python/issues/1996).
 * (rules) The first element of the default outputs is now the executable again.
 
 ### Removed
@@ -55,7 +58,7 @@ A brief description of the categories of changes:
   To enable it, set {obj}`--//python/config_settings:exec_tools_toolchain=enabled`.
   This toolchain must be enabled for precompilation to work. This toolchain will
   be enabled by default in a future release.
-  Fixes [1967](https://github.com/bazelbuild/rules_python/issues/1967).
+  Fixes [#1967](https://github.com/bazelbuild/rules_python/issues/1967).
 
 ## [0.33.1] - 2024-06-13
 

python/private/pypi/whl_installer/wheel.py

@@ -342,7 +342,12 @@ 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._add_version_select = platforms and len(self._target_versions) > 2
+        self._default_minor_version = None
+        if platforms and len(self._target_versions) > 2:
+            # 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()
+
         if None in self._target_versions and len(self._target_versions) > 2:
             raise ValueError(
                 f"all python versions need to be specified explicitly, got: {platforms}"
@@ -540,21 +545,21 @@ def _add_req(self, req: Requirement, extras: Set[str]) -> None:
             ):
                 continue
 
-            if match_arch and self._add_version_select:
+            if match_arch and self._default_minor_version:
                 self._add(req.name, plat)
-                if plat.minor_version == host_interpreter_minor_version():
+                if plat.minor_version == self._default_minor_version:
                     self._add(req.name, Platform(plat.os, plat.arch))
             elif match_arch:
-                self._add(req.name, plat)
-            elif match_os and self._add_version_select:
+                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 == host_interpreter_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._add_version_select:
+            elif match_version and self._default_minor_version:
                 self._add(req.name, Platform(minor_version=plat.minor_version))
-                if plat.minor_version == host_interpreter_minor_version():
+                if plat.minor_version == self._default_minor_version:
                     self._add(req.name, Platform())
             elif match_version:
                 self._add(req.name, None)

tests/pypi/whl_installer/wheel_test.py

@@ -217,6 +217,42 @@ def test_can_get_deps_based_on_specific_python_version(self):
         self.assertEqual(["bar"], py38_deps.deps)
         self.assertEqual({"@platforms//os:linux": ["posix_dep"]}, py38_deps.deps_select)
 
+    @mock.patch(
+        "python.private.pypi.whl_installer.wheel.host_interpreter_minor_version"
+    )
+    def test_no_version_select_when_single_version(self, mock_host_interpreter_version):
+        requires_dist = [
+            "bar",
+            "baz; python_version >= '3.8'",
+            "posix_dep; os_name=='posix'",
+            "posix_dep_with_version; os_name=='posix' and python_version >= '3.8'",
+            "arch_dep; platform_machine=='x86_64' and python_version >= '3.8'",
+        ]
+        mock_host_interpreter_version.return_value = 7
+
+        self.maxDiff = None
+
+        deps = wheel.Deps(
+            "foo",
+            requires_dist=requires_dist,
+            platforms=[
+                wheel.Platform(os=os, arch=wheel.Arch.x86_64, minor_version=minor)
+                for minor in [8]
+                for os in [wheel.OS.linux, wheel.OS.windows]
+            ],
+        )
+        got = deps.build()
+
+        self.assertEqual(["bar", "baz"], got.deps)
+        self.assertEqual(
+            {
+                "@platforms//os:linux": ["posix_dep", "posix_dep_with_version"],
+                "linux_x86_64": ["arch_dep", "posix_dep", "posix_dep_with_version"],
+                "windows_x86_64": ["arch_dep"],
+            },
+            got.deps_select,
+        )
+
     @mock.patch(
         "python.private.pypi.whl_installer.wheel.host_interpreter_minor_version"
     )
@@ -227,6 +263,7 @@ def test_can_get_version_select(self, mock_host_interpreter_version):
             "baz_new; python_version >= '3.8'",
             "posix_dep; os_name=='posix'",
             "posix_dep_with_version; os_name=='posix' and python_version >= '3.8'",
+            "arch_dep; platform_machine=='x86_64' and python_version < '3.8'",
         ]
         mock_host_interpreter_version.return_value = 7
 
@@ -251,6 +288,8 @@ def test_can_get_version_select(self, mock_host_interpreter_version):
                 "@//python/config_settings:is_python_3.8": ["baz_new"],
                 "@//python/config_settings:is_python_3.9": ["baz_new"],
                 "@platforms//os:linux": ["baz", "posix_dep"],
+                "cp37_linux_x86_64": ["arch_dep", "baz", "posix_dep"],
+                "cp37_windows_x86_64": ["arch_dep", "baz"],
                 "cp37_linux_anyarch": ["baz", "posix_dep"],
                 "cp38_linux_anyarch": [
                     "baz_new",
@@ -262,6 +301,8 @@ def test_can_get_version_select(self, mock_host_interpreter_version):
                     "posix_dep",
                     "posix_dep_with_version",
                 ],
+                "linux_x86_64": ["arch_dep", "baz", "posix_dep"],
+                "windows_x86_64": ["arch_dep", "baz"],
             },
             got.deps_select,
         )