Support multiple `abi` and `platform` values for `pip download`. #8820

katzdm · 2020-08-28T17:04:46Z

As proposed in #6121.

This is useful when fetching wheels for a well-defined interpreter, whose supported ABIs and platforms are known and fixed, different than the one pip is running under.

uranusjr · 2020-08-29T12:38:10Z

Would it be better to make the options action="append", instead of inventing a new comma-separated format? IMO it fits pip’s current argument design to do something like this instead:

pip download ...
    --abi cp36m --abi cp36 --abi abi3 --abi none
    ...

McSinyx · 2020-08-29T13:23:42Z

+1 on making it an append option, as it won't break backward compatibility.

uranusjr · 2020-08-29T13:51:37Z

I don’t think backward compatibility would be an issue either way, since comma is not a valid character in wheel tags anyway 🙂

McSinyx · 2020-08-29T14:17:49Z

I was thinking about the name change (e.g. platform -> platforms) but I guess we can keep an alias if we're to go with the comma-separated list.

katzdm · 2020-08-29T15:42:33Z

Thanks for the guidance! This is my first would-be contribution to pip, so I'm a bit unfamiliar with internal stylistic conventions - Will look at action="append" shortly!

katzdm · 2020-09-03T15:23:40Z

@uranusjr I pushed the action="append" change a few days ago - Would you mind taking another look, when you have a chance?

src/pip/_internal/cli/cmdoptions.py

uranusjr

Sorry for the delay. Just one nitpick on the help message, otherwise this looks good to me. Thanks for the effort!

katzdm · 2020-09-09T15:49:38Z

@uranusjr No problem, and thanks for taking a look! I've addressed your feedback, and have added a couple of elementary functional test-cases. Let me know if you have any other points of concern, whether you'd like to see broader test coverage, or whether things look good to you as-is.

Thanks again for your time!

katzdm · 2020-09-10T18:25:03Z

@uranusjr Thanks for the review and approval! As this is my first time contributing to pip, I'm not familiar with how the review proceeds from here. Are multiple approving reviews typically awaited before merging?

katzdm · 2020-09-17T14:55:00Z

Friendly ping 🙂

katzdm · 2020-09-29T12:48:39Z

@uranusjr @pradyunsg Wanted to ping this again - I had to rebase and resolve some conflicts yesterday, following other changes that were merged. Would be nice to get this in, to avoid doing so repeatedly.

Still a bit unclear as to whether we're waiting on other steps (e.g. additional reviews) before merging. Some clarification on the usual process would be appreciated.

Thanks in advance!

pradyunsg · 2020-09-30T08:53:31Z

Usually, we wait on a couple of reviews or have the primary reviewer be fairly certain that things will work. I'm personally not super comfortable around ABI tags and if/how we should expose them to the end user, so I'm not comfortable reviewing this (and considering the implications of this change) myself. :)

@uranusjr if you reckon this is good to go, I'm happy to merge.

uranusjr · 2020-09-30T09:35:30Z

Wheel tags are difficult, but these flags are designed as explicit overrides and the user has always been responsible for keeping the values straight, so I don’t think this PR has much chance to go wrong, actually. I think it’d be fine including this in 20.3.

xavfernandez · 2020-09-30T20:23:18Z

src/pip/_internal/utils/compatibility_tags.py

+    if platforms and len(platforms) == 1:
+        # Only expand list of platforms if a single platform was provided.
+        # Otherwise, assume that the list provided is comprehensive.
+        platforms = _get_custom_platforms(platforms[0])


Why not call _get_custom_platforms on all provided platforms ?

I'd find it strange that pip download --platform manylinux2010 foobar would successfully find a manylinux1_x86_64 wheel but pip download --platform manylinux2010 --platform manylinux2014 foobar wouldn't.

@xavfernandez I think this is a bit tricky.

I think it's reasonable to assume that anybody providing multiple --platform invocations is a fairly expert user, who is targeting a particular python interpreter different than the one through which pip is running. Therefore, the user should be the authority on what is and is not supported.

There may be edge-cases in which manylinux2014 wheels are acceptable, but manylinux1 wheels are not. For instance, the manylinux2014 and manylinux2010 specifications don't allow for binaries to link against the ncurses library, whereas manylinux1 does allow this library. If a user knows that their target environment does not provide ncurses, then they might write

pip download \ --platform manylinux2014 --platform manylinux2010 --platform any \ --abi cp39m --abi abi3 --abi none \ --python-version 39 \ --no-deps \ hiredis==0.3.1

To avoid downloading wheels built for manylinux1.

This won't be an issue for most users, but it could come up in more complex deployments (e.g. large monorepos), which (I think) is the setting in which this feature is most likely to be useful.

What do you think? We could always add platform-expansion here later - It's probably more difficult to take it back, afterwards.

Another question is one of precedence. Given the following two commands:

1. pip download --platform manylinux2014_x86_64 --platform linux_x86_64 foobar==0.1.0 2. pip download --platform manylinux_2014_x86_64 --platform any foobar==0.1.0

where the available wheels are built for [linux_x86_64, manylinux2010_x86_64, any], what should each command download?

If we perform manylinux-expansion, I would expect for 1 to download linux_86_64 (since I explicitly declared that platform to be acceptable), whereas I would expect for 2 to download manylinux2010_x86_64 (since any should probably be lowest precedence).

On the other hand, one might argue that manylinux2014_x86_64 should expand "in-place" in the precedence-list, to include all three manylinux-platforms. In that case, 1 should download manylinux2010_x86_64.

My intention isn't to argue which approach is more "correct" - Instead, it's to point out that the outcome is non-obvious. If we instead tell pip to just trust what the user provided, then this question melts away.

I would expects the --platform values be simply union-ed, and all expansions done in-place. The wheels downloaded would typically be installed by pip install on a specific platform, which would again filter the wheels. Incompatible wheels downloaded previously wouldn’t really make any practical differences (except some additional local disk space).

Fair enough, I suppose I don't feel strongly in either direction.

@uranusjr @xavfernandez I've pushed a change to implement this - Let me know what you think.

src/pip/_internal/cli/cmdoptions.py

katzdm · 2020-10-27T11:41:09Z

Looks like the MacOS test failures are happening at HEAD, and are likely unrelated.

pradyunsg · 2020-10-27T12:42:21Z

Yup -- #9030.

xavfernandez · 2020-10-27T22:42:56Z

Since #9030 should now be solved, rebasing again (sorry) should make the tests pass.

katzdm · 2020-10-27T23:01:33Z

Since #9030 should now be solved, rebasing again (sorry) should make the tests pass.

@xavfernandez Thanks for the heads up - Done and pushed.

pradyunsg · 2020-10-28T08:21:16Z

Merging given multiple approvals and that @uranusjr wants this in 20.3. :P

pradyunsg added the type: feature request Request for a new feature label Aug 28, 2020

katzdm force-pushed the multi-abis-platforms branch from 9bb8b2e to 1b5aa8b Compare August 28, 2020 17:18

katzdm mentioned this pull request Aug 28, 2020

pip download --platform --python-version does not work for some packages #6121

Closed

uranusjr reviewed Sep 9, 2020

View reviewed changes

src/pip/_internal/cli/cmdoptions.py Outdated Show resolved Hide resolved

uranusjr reviewed Sep 9, 2020

View reviewed changes

katzdm force-pushed the multi-abis-platforms branch 2 times, most recently from 02cf555 to d091260 Compare September 9, 2020 14:57

uranusjr approved these changes Sep 9, 2020

View reviewed changes

katzdm force-pushed the multi-abis-platforms branch 2 times, most recently from 91a0cc5 to c12157d Compare September 28, 2020 15:02

xavfernandez reviewed Sep 30, 2020

View reviewed changes

katzdm force-pushed the multi-abis-platforms branch 3 times, most recently from 12a2530 to 880fe55 Compare October 16, 2020 14:26

uranusjr reviewed Oct 22, 2020

View reviewed changes

src/pip/_internal/cli/cmdoptions.py Outdated Show resolved Hide resolved

katzdm force-pushed the multi-abis-platforms branch 2 times, most recently from 4fb0344 to 0fa1a08 Compare October 27, 2020 01:27

xavfernandez approved these changes Oct 27, 2020

View reviewed changes

katzdm added 7 commits October 27, 2020 19:00

Support multiple abi and platform values for pip download.

cea9f32

Use 'append'-style CLI arguments, rather than ','-separated values.

abf987b

Respond to feedback, and add functional tests.

7237bd3

Expand platform-tags unconditionally.

e08ec35

Rename news-file to use new *.rst convention.

1037227

Fix linter error: Add missing function type declaration.

01a512c

Spell abis and platforms as plural words.

7632c7a

katzdm force-pushed the multi-abis-platforms branch from 0fa1a08 to 7632c7a Compare October 27, 2020 23:00

uranusjr added this to the 20.3 milestone Oct 28, 2020

pradyunsg merged commit c4dbc7d into pypa:master Oct 28, 2020

katzdm deleted the multi-abis-platforms branch October 28, 2020 11:20

github-actions bot locked as resolved and limited conversation to collaborators Oct 6, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support multiple `abi` and `platform` values for `pip download`. #8820

Support multiple `abi` and `platform` values for `pip download`. #8820

katzdm commented Aug 28, 2020 •

edited

Loading

uranusjr commented Aug 29, 2020

McSinyx commented Aug 29, 2020

uranusjr commented Aug 29, 2020

McSinyx commented Aug 29, 2020

katzdm commented Aug 29, 2020

katzdm commented Sep 3, 2020

uranusjr left a comment

katzdm commented Sep 9, 2020

katzdm commented Sep 10, 2020

katzdm commented Sep 17, 2020

katzdm commented Sep 29, 2020

pradyunsg commented Sep 30, 2020

uranusjr commented Sep 30, 2020

xavfernandez Sep 30, 2020 •

edited

Loading

katzdm Oct 15, 2020

katzdm Oct 15, 2020

uranusjr Oct 16, 2020

katzdm Oct 16, 2020

katzdm commented Oct 27, 2020

pradyunsg commented Oct 27, 2020

xavfernandez commented Oct 27, 2020

katzdm commented Oct 27, 2020

pradyunsg commented Oct 28, 2020

Support multiple abi and platform values for pip download. #8820

Support multiple abi and platform values for pip download. #8820

Conversation

katzdm commented Aug 28, 2020 • edited Loading

uranusjr commented Aug 29, 2020

McSinyx commented Aug 29, 2020

uranusjr commented Aug 29, 2020

McSinyx commented Aug 29, 2020

katzdm commented Aug 29, 2020

katzdm commented Sep 3, 2020

uranusjr left a comment

Choose a reason for hiding this comment

katzdm commented Sep 9, 2020

katzdm commented Sep 10, 2020

katzdm commented Sep 17, 2020

katzdm commented Sep 29, 2020

pradyunsg commented Sep 30, 2020

uranusjr commented Sep 30, 2020

xavfernandez Sep 30, 2020 • edited Loading

Choose a reason for hiding this comment

katzdm Oct 15, 2020

Choose a reason for hiding this comment

katzdm Oct 15, 2020

Choose a reason for hiding this comment

uranusjr Oct 16, 2020

Choose a reason for hiding this comment

katzdm Oct 16, 2020

Choose a reason for hiding this comment

katzdm commented Oct 27, 2020

pradyunsg commented Oct 27, 2020

xavfernandez commented Oct 27, 2020

katzdm commented Oct 27, 2020

pradyunsg commented Oct 28, 2020

Support multiple `abi` and `platform` values for `pip download`. #8820

Support multiple `abi` and `platform` values for `pip download`. #8820

katzdm commented Aug 28, 2020 •

edited

Loading

xavfernandez Sep 30, 2020 •

edited

Loading