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
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6ae6508..0cbf2f9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -41,6 +41,9 @@
`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 @@
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 3d6780d..c167df9 100644
--- a/python/private/pypi/whl_installer/wheel.py
+++ b/python/private/pypi/whl_installer/wheel.py
@@ -342,7 +342,12 @@
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 @@
):
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 9b27205..76bfe72 100644
--- a/tests/pypi/whl_installer/wheel_test.py
+++ b/tests/pypi/whl_installer/wheel_test.py
@@ -220,6 +220,42 @@
@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"
+ )
def test_can_get_version_select(self, mock_host_interpreter_version):
requires_dist = [
"bar",
@@ -227,6 +263,7 @@
"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 @@
"@//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 @@
"posix_dep",
"posix_dep_with_version",
],
+ "linux_x86_64": ["arch_dep", "baz", "posix_dep"],
+ "windows_x86_64": ["arch_dep", "baz"],
},
got.deps_select,
)
