diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ae6508f21..0cbf2f91ce 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/python/private/pypi/whl_installer/wheel.py b/python/private/pypi/whl_installer/wheel.py index 3d6780de9a..c167df9193 100644 --- a/python/private/pypi/whl_installer/wheel.py +++ b/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) diff --git a/tests/pypi/whl_installer/wheel_test.py b/tests/pypi/whl_installer/wheel_test.py index 9b27205ac9..76bfe720bc 100644 --- a/tests/pypi/whl_installer/wheel_test.py +++ b/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, )