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

Add pip>=20.3 support #1216

Merged
merged 8 commits into from
Nov 20, 2020
Merged

Add pip>=20.3 support #1216

merged 8 commits into from
Nov 20, 2020

Conversation

atugushev
Copy link
Member

@atugushev atugushev commented Oct 15, 2020

  1. Support pip=>20.3
  2. Introduce new tox env: tox -e pipprevious
  3. Update CI to run latest and previous versions of pip

Changelog-friendly one-liner: Add pip>=20.3 support.

Contributor checklist
  • Provided the tests for the changes.
  • Gave a clear one-line description in the PR (that the maintainers can add to CHANGELOG.md on release).
  • Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

@atugushev atugushev added this to the 5.4.0 milestone Oct 15, 2020
@atugushev atugushev added the enhancement Improvements to functionality label Oct 15, 2020
@atugushev atugushev changed the title Add pip==20.3 support Add pip>=20.3 support Oct 15, 2020
@codecov
Copy link

codecov bot commented Oct 15, 2020

Codecov Report

Merging #1216 (3b79e9d) into master (05c9ec0) will decrease coverage by 0.06%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1216      +/-   ##
==========================================
- Coverage   99.50%   99.44%   -0.07%     
==========================================
  Files          36       36              
  Lines        2855     2863       +8     
  Branches      332      336       +4     
==========================================
+ Hits         2841     2847       +6     
- Misses          8        9       +1     
- Partials        6        7       +1     
Impacted Files Coverage Δ
piptools/repositories/pypi.py 94.86% <100.00%> (-0.65%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 05c9ec0...3b79e9d. Read the comment docs.

@atugushev atugushev marked this pull request as draft October 15, 2020 02:25
@AndydeCleyre
Copy link
Contributor

AndydeCleyre commented Oct 28, 2020

@atugushev Looks like we've got to add compatibility for the new merging of download_dir and wheel_download_dir. Please see my added and reverted commit, which I think would be fine if we were only supporting >=20.3.

I'll try to get back to this very soon to make it compatible with older releases as well.

EDIT: Please see the new commit.

@AndydeCleyre
Copy link
Contributor

Ideally we can push a release out before the pip 20.3 release, which is imminent.

@groodt
Copy link

groodt commented Nov 2, 2020

The 2.3 beta 1 is now on pypi https://pypi.org/project/pip/20.3b1/

@AndydeCleyre
Copy link
Contributor

OK, we've got at least one more recent change in pip that require attention here:

@AndydeCleyre
Copy link
Contributor

Notes on the _resolve_one issue:

  • that method remains in the legacy resolver, which may be used by users of pip>=20.3, so a version check isn't going to cut it.
  • I don't know how our support for choosing the resolver will really be, but my priority is keeping this usable at all with the new pip and new resolver.

@AndydeCleyre
Copy link
Contributor

I'm trying to understand the use and purpose of _resolve_one, by pip-tools. It was introduced in 1db139c, which says:

Incorporate changes from #666 (comment)

I welcome any extra notes from @techalchemy

Link to that comment

I don't understand yet. Obviously it resolves dependencies. It partially may be about filtering direct requirements from the results. It also looks like it's used to "prepare" the reqs.

For now I'm trying using pip's resolver's resolve method instead while working it out.

I also now realize that the pip version checks we already have in this PR are not going to cut it when the user has newer pip, but passes --pip-args --use-deprecated=legacy-resolver. I don't know a good way to detect which resolver is active -- some isinstance check, or attribute check, or something? For now I'm catching AttributeErrors to guess, but hopefully we can work out something better.

We may want to modify most or all of the compile cli tests to run again with that option, if the pip version is new enough...

@groodt
Copy link

groodt commented Nov 19, 2020

Looks like it's going GA very soon: pypa/pip#8936 (comment)

@pradyunsg
Copy link
Contributor

Ask me anything you want, regarding the new resolver in pip and the internal model/design of the various components (from pip's PoV)! :)

@atugushev
Copy link
Member Author

@AndydeCleyre thank you very much for helping me with pip-20.3 🙏

I don't know a good way to detect which resolver is active -- some isinstance check, or attribute check, or something? For now I'm catching AttributeErrors to guess, but hopefully we can work out something better.

It seems for now we should pass --use-deprecated=legacy-resolver option to the InstallComamnd to force pip to use legacy resolver until pip-tools would support new resolver. See a1e34a7.


@pradyunsg thank you! Your assistance would help a lot when we start to work on the new resolver!

@AndydeCleyre
Copy link
Contributor

I'm sorry I haven't been more helpful on this.

@atugushev atugushev marked this pull request as ready for review November 20, 2020 07:42
@atugushev
Copy link
Member Author

It's ready for review, and yeah it's better to merge and release this before pip-20.3 being released (which is today 😰)

Copy link
Member

@vphilippon vphilippon left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@atugushev
Copy link
Member Author

@vphilippon thanks for reviewing this! I'll merge this then eagerly if nobody minds.

@atugushev
Copy link
Member Author

Ah, no I can't. Travis is still pending since yesterday :(

@vphilippon
Copy link
Member

@atugushev Yeah I saw that. I cancelled and retried the build in hope to get it unstuck, but to no avail.
I was going to ask if that was something you knew about.

@atugushev
Copy link
Member Author

@vphilippon not that I'm aware of. Maybe we have reached free credit for free usage? See also jazzband/help#206 (comment)

@pradyunsg
Copy link
Contributor

I'd suggest turning Travis CI off, because they have an open source plan "in progress" and not actually rolled out to any project AFAIK. The one-time credit limit they announced is like 16 hours of CI time, which just isn't enough.

@pradyunsg
Copy link
Contributor

#1229 is the relevant PR. :)

@hugovk
Copy link
Member

hugovk commented Nov 20, 2020

I'd suggest turning Travis CI off, because they have an open source plan "in progress" and not actually rolled out to any project AFAIK. The one-time credit limit they announced is like 16 hours of CI time, which just isn't enough.

Raku is the only project I've seen which has been granted something:

I've applied but not got anything yet. But, yeah, #1229 :)

@groodt
Copy link

groodt commented Nov 21, 2020

Thanks for the quick turnaround all!

This was referenced Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants