Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

wheel_installer with --platform fails for non-cp abis #1810

Closed
Illedran opened this issue Mar 17, 2024 · 1 comment · Fixed by #1811
Closed

wheel_installer with --platform fails for non-cp abis #1810

Illedran opened this issue Mar 17, 2024 · 1 comment · Fixed by #1811
Labels
P3 type: pip

Comments

@Illedran
Copy link

🐞 bug report

Affected Rule

Users of the wheel_installer tool - e.g. like the generated repos from pip_parse when used with experimental_target_platforms.

Is this a regression?

Yes - works on 0.29.0 (likely due to #1693 cc: @aignas).

Description

The wheel_installer tool is defensive when parsing the --platform flag and checks whether the abi starts with cp - this is a problem when wheels do not use that abi (but are nevertheless compatible, see below) as it then tries to find the OS to target using the abi name instead.

For example, in the linked reproduction below, pip ends up downloading:

  • tornado-6.4-cp38-abi3-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_17_x86_64.manylinux2014_x86_64.whl - abi_tag == "abi3" (link on PyPI)
  • libclang-17.0.6-py2.py3-none-manylinux2010_x86_64.whl - abi_tag == "none" (link on PyPI)

Relevant PEPs:

🔬 Minimal Reproduction

See https://github.com/Illedran/wheel_installer_tool_abi_bug.

🔥 Exception or Error

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/home/illedran/.cache/bazel/_bazel_illedran/527668b0d651ad7c51390aa36afa823c/external/rules_python/python/pip_install/tools/wheel_installer/wheel_installer.py", line 205, in <module>
    main()
  File "/home/illedran/.cache/bazel/_bazel_illedran/527668b0d651ad7c51390aa36afa823c/external/rules_python/python/pip_install/tools/wheel_installer/wheel_installer.py", line 149, in main
    args = arguments.parser(description=__doc__).parse_args()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/illedran/.cache/bazel/_bazel_illedran/527668b0d651ad7c51390aa36afa823c/external/python_3_12_x86_64-unknown-linux-gnu/lib/python3.12/argparse.py", line 1891, in parse_args
    args, argv = self.parse_known_args(args, namespace)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/illedran/.cache/bazel/_bazel_illedran/527668b0d651ad7c51390aa36afa823c/external/python_3_12_x86_64-unknown-linux-gnu/lib/python3.12/argparse.py", line 1924, in parse_known_args
    namespace, args = self._parse_known_args(args, namespace)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/illedran/.cache/bazel/_bazel_illedran/527668b0d651ad7c51390aa36afa823c/external/python_3_12_x86_64-unknown-linux-gnu/lib/python3.12/argparse.py", line 2136, in _parse_known_args
    start_index = consume_optional(start_index)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/illedran/.cache/bazel/_bazel_illedran/527668b0d651ad7c51390aa36afa823c/external/python_3_12_x86_64-unknown-linux-gnu/lib/python3.12/argparse.py", line 2076, in consume_optional
    take_action(action, args, option_string)
  File "/home/illedran/.cache/bazel/_bazel_illedran/527668b0d651ad7c51390aa36afa823c/external/python_3_12_x86_64-unknown-linux-gnu/lib/python3.12/argparse.py", line 1984, in take_action
    argument_values = self._get_values(action, argument_strings)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/illedran/.cache/bazel/_bazel_illedran/527668b0d651ad7c51390aa36afa823c/external/python_3_12_x86_64-unknown-linux-gnu/lib/python3.12/argparse.py", line 2522, in _get_values
    value = self._get_value(action, arg_string)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/illedran/.cache/bazel/_bazel_illedran/527668b0d651ad7c51390aa36afa823c/external/python_3_12_x86_64-unknown-linux-gnu/lib/python3.12/argparse.py", line 2555, in _get_value
    result = type_func(arg_string)
             ^^^^^^^^^^^^^^^^^^^^^
  File "/home/illedran/.cache/bazel/_bazel_illedran/527668b0d651ad7c51390aa36afa823c/external/rules_python/python/pip_install/tools/wheel_installer/wheel.py", line 205, in from_string
    os=OS[os] if os != "*" else None,
       ~~^^^^
  File "/home/illedran/.cache/bazel/_bazel_illedran/527668b0d651ad7c51390aa36afa823c/external/python_3_12_x86_64-unknown-linux-gnu/lib/python3.12/enum.py", line 801, in __getitem__
    return cls._member_map_[name]
           ~~~~~~~~~~~~~~~~^^^^^^
KeyError: 'abi3'

See the README.md for the full error log (incl. Bazel) and another example.

🌍 Your Environment

Operating System:

Linux 5.15.146.1-microsoft-standard-WSL2 x86_64

Output of bazel version:

Bazelisk version: v1.18.0
Build label: 7.0.2
Build target: @@//src/main/java/com/google/devtools/build/lib/bazel:BazelServer
Build time: Thu Jan 25 16:13:35 2024 (1706199215)
Build timestamp: 1706199215
Build timestamp as int: 1706199215

Rules_python version:

0.31.0
@aignas
Copy link
Collaborator

aignas commented Mar 18, 2024 •
edited
Loading

FYI this is related to #1790, more specifically: #1790 (comment)

aignas added a commit to aignas/rules_python that referenced this issue Mar 18, 2024
@aignas
fix(whl_library): correctly parse wheel target platforms
8f95d1c 
The bazelbuild#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 bazelbuild#1810.
@aignas aignas mentioned this issue Mar 18, 2024
Merged
aignas added a commit to aignas/rules_python that referenced this issue Mar 18, 2024
@aignas
fix(whl_library): correctly parse wheel target platforms
0f30e02 
The bazelbuild#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 bazelbuild#1810.
github-merge-queue bot pushed a commit that referenced this issue Mar 18, 2024
@aignas
fix(whl_library): correctly parse wheel target platforms (#1811)
c11045d 
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.
@wingsofovnia wingsofovnia mentioned this issue May 3, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
P3 type: pip
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(whl_library): correctly parse wheel target platforms aignas/rules_python
2 participants
@aignas @Illedran