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.

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

fix(whl_library): correctly parse wheel target platforms #1811

Merged

Conversation

aignas
Copy link
Collaborator

@aignas aignas commented Mar 18, 2024

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.

Copy link
Contributor

@rickeylev rickeylev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I think. CI is reporting a failure, but looks like a simple missing/renamed arg failure

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 force-pushed the fix/experimental-platform-selection-for-wheel branch from 8f95d1c to 0f30e02 Compare March 18, 2024 13:19
@aignas aignas enabled auto-merge March 18, 2024 13:20
@aignas aignas added this pull request to the merge queue Mar 18, 2024
Merged via the queue into bazelbuild:main with commit c11045d Mar 18, 2024
4 checks passed
@aignas aignas deleted the fix/experimental-platform-selection-for-wheel branch March 19, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wheel_installer with --platform fails for non-cp abis
2 participants