From 0f30e02a9913fb8854bcfce130964572c60b9404 Mon Sep 17 00:00:00 2001 From: aignas <240938+aignas@users.noreply.github.com> Date: Mon, 18 Mar 2024 12:16:23 +0900 Subject: [PATCH] fix(whl_library): correctly parse wheel target platforms The #1693 PR incorrectly assumed that the platform tag will be os-arch specific if and only if the abi tag is of form cpxy. However, there are many wheels that are not like this (e.g. watchdog, tornado, libclang). This fixes the starlark code that is overriding the user platforms with something that only the wheel supports by also taking into account the ABI. Fixes #1810. --- CHANGELOG.md | 4 ++ python/pip_install/pip_repository.bzl | 7 ++- python/private/whl_target_platforms.bzl | 27 +++++++++--- .../whl_target_platforms_tests.bzl | 43 +++++++++++++++---- 4 files changed, 64 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 142df6a208..9b7ab25545 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,10 @@ A brief description of the categories of changes: ### Fixed +* (whl_library): Fix the experimental_target_platforms overriding for platform + specific wheels when the wheels are for any python interpreter version. Fixes + [#1810](https://github.com/bazelbuild/rules_python/issues/1810). + ### Added * New Python versions available: `3.11.8`, `3.12.2` using diff --git a/python/pip_install/pip_repository.bzl b/python/pip_install/pip_repository.bzl index ded7112144..3d5f3c1eb8 100644 --- a/python/pip_install/pip_repository.bzl +++ b/python/pip_install/pip_repository.bzl @@ -801,8 +801,11 @@ def _whl_library_impl(rctx): # NOTE @aignas 2023-12-04: if the wheel is a platform specific # wheel, we only include deps for that target platform target_platforms = [ - "{}_{}_{}".format(parsed_whl.abi_tag, p.os, p.cpu) - for p in whl_target_platforms(parsed_whl.platform_tag) + p.target_platform + for p in whl_target_platforms( + platform_tag = parsed_whl.platform_tag, + abi_tag = parsed_whl.abi_tag, + ) ] repo_utils.execute_checked( diff --git a/python/private/whl_target_platforms.bzl b/python/private/whl_target_platforms.bzl index 2c63efe26f..30e4dd4c7a 100644 --- a/python/private/whl_target_platforms.bzl +++ b/python/private/whl_target_platforms.bzl @@ -40,28 +40,41 @@ _OS_PREFIXES = { "win": "windows", } # buildifier: disable=unsorted-dict-items -def whl_target_platforms(tag): - """Parse the wheel platform tag and return (os, cpu) tuples. +def whl_target_platforms(platform_tag, abi_tag = ""): + """Parse the wheel abi and platform tags and return (os, cpu) tuples. Args: - tag (str): The platform_tag part of the wheel name. See + platform_tag (str): The platform_tag part of the wheel name. See ./parse_whl_name.bzl for more details. + abi_tag (str): The abi tag that should be used for parsing. Returns: A list of structs, with attributes: * os: str, one of the _OS_PREFIXES values * cpu: str, one of the _CPU_PREFIXES values + * abi: str, the ABI that the interpreter should have if it is passed. + * target_platform: str, the target_platform that can be given to the + wheel_installer for parsing whl METADATA. """ - cpus = _cpu_from_tag(tag) + cpus = _cpu_from_tag(platform_tag) + + abi = None + if abi_tag not in ["", "none", "abi3"]: + abi = abi_tag for prefix, os in _OS_PREFIXES.items(): - if tag.startswith(prefix): + if platform_tag.startswith(prefix): return [ - struct(os = os, cpu = cpu) + struct( + os = os, + cpu = cpu, + abi = abi, + target_platform = "_".join([abi, os, cpu] if abi else [os, cpu]), + ) for cpu in cpus ] - fail("unknown tag os: {}".format(tag)) + fail("unknown platform_tag os: {}".format(platform_tag)) def _cpu_from_tag(tag): candidate = [ diff --git a/tests/private/whl_target_platforms/whl_target_platforms_tests.bzl b/tests/private/whl_target_platforms/whl_target_platforms_tests.bzl index 9ccff0e485..f52437fd3c 100644 --- a/tests/private/whl_target_platforms/whl_target_platforms_tests.bzl +++ b/tests/private/whl_target_platforms/whl_target_platforms_tests.bzl @@ -22,29 +22,56 @@ _tests = [] def _test_simple(env): tests = { "macosx_10_9_arm64": [ - struct(os = "osx", cpu = "aarch64"), + struct(os = "osx", cpu = "aarch64", abi = None, target_platform = "osx_aarch64"), ], "macosx_10_9_universal2": [ - struct(os = "osx", cpu = "x86_64"), - struct(os = "osx", cpu = "aarch64"), + struct(os = "osx", cpu = "x86_64", abi = None, target_platform = "osx_x86_64"), + struct(os = "osx", cpu = "aarch64", abi = None, target_platform = "osx_aarch64"), ], "manylinux1_i686.manylinux_2_17_i686": [ - struct(os = "linux", cpu = "x86_32"), + struct(os = "linux", cpu = "x86_32", abi = None, target_platform = "linux_x86_32"), ], "musllinux_1_1_ppc64le": [ - struct(os = "linux", cpu = "ppc"), + struct(os = "linux", cpu = "ppc", abi = None, target_platform = "linux_ppc"), ], "win_amd64": [ - struct(os = "windows", cpu = "x86_64"), + struct(os = "windows", cpu = "x86_64", abi = None, target_platform = "windows_x86_64"), ], } for give, want in tests.items(): - got = whl_target_platforms(give) - env.expect.that_collection(got).contains_exactly(want) + for abi in ["", "abi3", "none"]: + got = whl_target_platforms(give, abi) + env.expect.that_collection(got).contains_exactly(want) _tests.append(_test_simple) +def _test_with_abi(env): + tests = { + "macosx_10_9_arm64": [ + struct(os = "osx", cpu = "aarch64", abi = "cp39", target_platform = "cp39_osx_aarch64"), + ], + "macosx_10_9_universal2": [ + struct(os = "osx", cpu = "x86_64", abi = "cp310", target_platform = "cp310_osx_x86_64"), + struct(os = "osx", cpu = "aarch64", abi = "cp310", target_platform = "cp310_osx_aarch64"), + ], + "manylinux1_i686.manylinux_2_17_i686": [ + struct(os = "linux", cpu = "x86_32", abi = "cp38", target_platform = "cp38_linux_x86_32"), + ], + "musllinux_1_1_ppc64le": [ + struct(os = "linux", cpu = "ppc", abi = "cp311", target_platform = "cp311_linux_ppc"), + ], + "win_amd64": [ + struct(os = "windows", cpu = "x86_64", abi = "cp311", target_platform = "cp311_windows_x86_64"), + ], + } + + for give, want in tests.items(): + got = whl_target_platforms(give, want[0].abi) + env.expect.that_collection(got).contains_exactly(want) + +_tests.append(_test_with_abi) + def whl_target_platforms_test_suite(name): """Create the test suite.