fix: incorrect abi parsing logic in wheel.py

[cjim8889]

This PR fixes the incorrect abi parsing logic. Currently, the logic would incorrectly assume the first item is not an abi for packages that do not specify an abi compatibility, e.g. watchdog-4.0.0-py3-none-manylinux2014_aarch64.whl. This would render the os incorrectly parsed, causing total build failures later on.

python3 -m python.pip_install.tools.wheel_installer.wheel_installer --requirement "watchdog==4.0.0" --whl-file [REMOVED]/7b77203049f425466cc854899ac39021/external/pip_watchdog/watchdog-4.0.0-py3-none-manylinux2014_aarch64.whl --platform=none_linux_aarch64
<stdout empty>
===== stderr start =====
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "[REMOVED]7b77203049f425466cc854899ac39021/external/rules_python/python/pip_install/tools/wheel_installer/wheel_installer.py", line 205, in <module>
    main()
[REMOVED]
  File "[REMOVED]7b77203049f425466cc854899ac39021/external/rules_python/python/pip_install/tools/wheel_installer/wheel.py", line 205, in from_string
    os=OS[os] if os != "*" else None,
       ~~^^^^
  File
 "[REMOVED]7b77203049f425466cc854899ac39021/external/python311_aarch64-unknown-linux-gnu/lib/python3.11/enum.py", line 790, in __getitem__
    return cls._member_map_[name]
           ~~~~~~~~~~~~~~~~^^^^^^
KeyError: 'none'

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[aignas]

I found this as well, but I think the right fix would be to modify the starlark code that uses this. See #1744 where we pass target platforms to the wheel installer based on the wheel tags.

[cjim8889]

I found this as well, but I think the right fix would be to modify the starlark code that uses this. See #1744 where we pass target platforms to the wheel installer based on the wheel tags.

Interesting, I would imagine none is also a valid abi flag and thus the wheel.py should be able to handle it correctly and thus the fix that I proposed. wdyt?

[cjim8889]

Moreover, I notice there are some packages with names like this: protobuf-4.25.3-cp37-abi3-manylinux2014_aarch64.whl, which means we need to support this as well.

[cjim8889]

I just took a second look, I believe we need to approach this issue differently.

Firstly, it's important to distinguish between the abi_tag and the platform_tag. Modifying this in starlark would be incorrect because the absence of a specified ABI does not necessarily imply that the code is cross-platform. Therefore, we must still verify platform-specific dependencies.

Secondly, I noticed that the abi3 tag poses a similar issue as well. This PR addresses and resolves the issue in a similar manner.

[aignas]

My thinking about the code that we have is that the wheel.py file is about parsing the METADATA with respect to the target platform specified by the list of values that the user requests, whereas the starlark part is to translate the list of values that the user requests to the values that wheel.py understands.

Let's say the user says I want to get dependencies for (cp311, linux_x86_64), (cp311, linux_aarch64), (cp311, osx_x86_64), (cp311, osx_aarch64). When we find a wheel that is py3-none-manylinux_2_17_x86_64, the behaviour should not be to add dependencies for cp39, cp310 and cp311 for linux_x86_64 when parsing the METADATA of that wheel. We should only parse the METADATA by assuming that the target platform is (cp311, linux_x86_64).

The special case where we get a wheel cp311-cp311-manylinux_2_17_x86_64 we are sure that it is a subset of the requested target platforms, because there is no other way to get the requested platforms (i.e. the wheel will not be downloaded). That is why there is that code in the starlark where we are just selecting a particular specific platform. If the wheel is py3-none-manylinux_2_17_x86_64, then we should potentially only forward the linux_x86_64 target platforms, because that is the intersection of the set of the target platforms and the wheel supported platforms.

So in short, the bug that you found is because I assumed that manylinux_x86_64 cannot be together with py3-none and I think that starklark should be modified instead.

[cjim8889]

Interesting, I'll close this then. thanks!