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

Upgrade to pip 19 #833

Merged
merged 4 commits into from
Feb 19, 2020
Merged

Upgrade to pip 19 #833

merged 4 commits into from
Feb 19, 2020

Conversation

cjolowicz
Copy link
Contributor

@cjolowicz cjolowicz commented Aug 24, 2019 •
edited
Loading

This PR upgrades pip to version 19.x. This upgrade is the first step to provide support for pyproject.toml-based projects (PEP 517), such as those managed by poetry and flit.

  • Upgrade to pip 19.2.3, for projects using Python 2.7 and >= 3.5.
  • Upgrade to pip 19.1.1, for Python 3.4 projects.

Note that pip 19.2 dropped Python 3.4 support, hence the version difference.

Patch the vendorized pip-pop package, which is used by the pip-uninstall, gdal, and pylibmc steps. It relies on internal pip modules changed by the update.

Closes #740
See #804

Follow-ups:

CI is broken for PRs from another repository, see this comment. Please check the corresponding PR in my fork to verify CI:

@cjolowicz cjolowicz requested a review from a team as a code owner August 24, 2019 11:47
@cjolowicz cjolowicz mentioned this pull request Aug 24, 2019
Open
@cjolowicz
Copy link
Contributor Author

pip just released 19.2.3.
https://pypi.org/project/pip/#history

@cjolowicz cjolowicz mentioned this pull request Aug 26, 2019
Open
@cjolowicz cjolowicz mentioned this pull request Aug 28, 2019
Open
@cjolowicz cjolowicz mentioned this pull request Aug 29, 2019
Open
@David-OConnor
Copy link

Limitations on merging this?

@cjolowicz
Copy link
Contributor Author

Rebased onto master (v157).

@cjolowicz cjolowicz mentioned this pull request Sep 26, 2019
Open
@CaseyFaist
Copy link
Contributor

Thanks for the PR series @cjolowicz!

This looks good, but updating pip for pipenv users or requiring them to update without a heads up won't be a good experience (our version is old enough that they'll need to uninstall and reinstall pipenv locally to successfully update). If you can refactor this to stay pinned to current version for pipenv users only, I should be able to accept this (and the related project updates) 👍

CaseyFaist
CaseyFaist suggested changes Oct 3, 2019
View reviewed changes
Copy link
Contributor

@CaseyFaist CaseyFaist left a comment

Choose a reason for hiding this comment

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

See comment for details - but for now, pipenv users should still get the old version of pip

@CaseyFaist
Copy link
Contributor

also relevant: I'm going to extract the package manager detection parts from #816 as well, contributor willing. I can make sure it all plays nicely together after the fact, but all I'm looking for here is a separate pipenv-pip-version variable that I can pass to the pipenv step, and confirmation that the pip-diff updates should still work with the old version 👍

cjolowicz added a commit to cjolowicz/heroku-buildpack-python that referenced this pull request Oct 3, 2019
@cjolowicz
Pin to pip 9.0.2 for pipenv users only
346a67d 
This addresses an issue raised by @CaseyFeist during code review:

  Updating pip for pipenv users or requiring them to update without a
  heads up won't be a good experience (our version is old enough that
  they'll need to uninstall and reinstall pipenv locally to successfully
  update). If you can refactor this to stay pinned to current version for
  pipenv users only, I should be able to accept this (and the related
  project updates).

  heroku#833 (comment)
cjolowicz added a commit to cjolowicz/heroku-buildpack-python that referenced this pull request Oct 3, 2019
@cjolowicz
Pin to pip 9.0.2 for pipenv users only
8408e34 
This addresses an issue raised by @CaseyFeist during code review:

  Updating pip for pipenv users or requiring them to update without a
  heads up won't be a good experience (our version is old enough that
  they'll need to uninstall and reinstall pipenv locally to successfully
  update). If you can refactor this to stay pinned to current version for
  pipenv users only, I should be able to accept this (and the related
  project updates).

  heroku#833 (comment)
@cjolowicz
Copy link
Contributor Author

@cjolowicz
Copy link
Contributor Author

Hey @CaseyFaist, thanks for your feedback!

Here's a new commit to keep pip pinned to the current version for pipenv users: 8408e34. Does this address your concerns?

I like the approach of #816 to modularize the buildpack by package manager, so great!

Here's a comment explaining the changes to pip-diff and pip-grep. Please let me know if you need more detail. Basically these changes make sure that pip-diff and friends work both with the old and the new version of pip.

The branch was also rebased to the tip of master.

@CaseyFaist
Copy link
Contributor

@cjolowicz thanks for your work! Looking over it now, I'll add comments as I have them ❤️

@helioseven
Copy link

What is the status of this PR? I see this as a high-priority fix, given that manylinux2010 wheels do not work with pip 9.0.2 (as was mentioned in #804 ).

Currently, only option for building apps with such wheels seems to be forking this repo, modifying PIP_UPDATE in /bin/compile, and then setting buildpack for app to forked repo. Less than ideal, obviously.

@cjolowicz
Copy link
Contributor Author

cjolowicz commented Nov 6, 2019 •
edited
Loading

Current pip release is now 19.3.1, released on October 17, 2019.

@cjolowicz
Upgrade to pip 19.2.3
53c4674 
Bump PIP_UPDATE from 9.0.2 to 19.2.3. This variable is used in bin/steps/python
to determine which pip version to install or upgrade to.
@cjolowicz
Upgrade to pip 19.1.1 for Python 3.4 projects
515a222 
Python 3.4 support was dropped in pip >= 19.2. For projects still on
this Python version, use pip 19.1.1 instead of pip 19.2.1.
@cjolowicz
Adapt pip-pop to changes in pip >= 10
468d27a 
The pip-diff and pip-grep tools from the vendorized `pip-pop` package
import internal modules from pip. In pip >= 10, internal modules were
moved under `pip._internal`, breaking the imports. Use `try...except
ImportError` to handle both import paths.

Also, the interface of the `PackageFinder` class from one of these
modules changed. Provide a wrapper function to allow creating objects of
this type using the old interface.
@cjolowicz
Pin to pip 9.0.2 for pipenv users only
8eb2954 
This addresses an issue raised by @CaseyFeist during code review:

  Updating pip for pipenv users or requiring them to update without a
  heads up won't be a good experience (our version is old enough that
  they'll need to uninstall and reinstall pipenv locally to successfully
  update). If you can refactor this to stay pinned to current version for
  pipenv users only, I should be able to accept this (and the related
  project updates).

  heroku#833 (comment)
@cjolowicz
Copy link
Contributor Author

cjolowicz commented Nov 24, 2019 •
edited
Loading

  • Rebased onto master (93a620e)
  • Upgrade to pip 19.3.1 (40ec196d34a0774d6dc4519759407bcd2540cf4d)

Update: Retracted the upgrade to pip 19.3.1. This is now merely a rebase onto master, without additional changes.

Background: The upgrade to pip 19.3.1 caused testSmartRequirements to fail. I presume this happens because the internal interfaces used by pip-pop changed again. So I'm keeping this at 19.2.3 for now.

@cjolowicz
Copy link
Contributor Author

@CaseyFaist : This PR is marked "changes requested", but I'm not sure which changes are requested.

cjolowicz added a commit to cjolowicz/heroku-buildpack-python that referenced this pull request Nov 24, 2019
@cjolowicz
Pin to pip 9.0.2 for pipenv users only
83ffd2a 
This addresses an issue raised by @CaseyFeist during code review:

  Updating pip for pipenv users or requiring them to update without a
  heads up won't be a good experience (our version is old enough that
  they'll need to uninstall and reinstall pipenv locally to successfully
  update). If you can refactor this to stay pinned to current version for
  pipenv users only, I should be able to accept this (and the related
  project updates).

  heroku#833 (comment)
@helioseven
Copy link

Did it fail checks just because changelog.md wasn't modified? Do we just need to add an entry to said changelog for this PR?

@cjolowicz
Copy link
Contributor Author

cjolowicz commented Nov 26, 2019 •
edited
Loading

@helioseven, the checks here do not work for external contributions. See the mirror PR in my fork:

Tests over there pass, with the exception of the changelog.

@CaseyFaist CaseyFaist merged commit 8eb2954 into heroku:master Feb 19, 2020
@cjolowicz cjolowicz deleted the upgrade-to-pip-19 branch February 19, 2020 20:39
@edmorley edmorley mentioned this pull request Nov 11, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CaseyFaist CaseyFaist CaseyFaist requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support pip10
4 participants
@cjolowicz @David-OConnor @CaseyFaist @helioseven