Skip to content

Vendor pip 9.0.3 - #644

Merged
vphilippon merged 8 commits into
jazzband:masterfrom
vphilippon:vendor-pip-9.0.2
Mar 23, 2018
Merged

vphilippon merged 8 commits into
jazzband:masterfrom
vphilippon:vendor-pip-9.0.2

Conversation

@vphilippon

@vphilippon vphilippon commented Mar 21, 2018 •

Copy link
Copy Markdown
Member

Vendoring pip 9.0.3 inside of pip-tools.
This will ensure the dependency resolution code will work even if users install pip 10.
The install/uninstall operations of pip-sync as still done through shell out, so they will benefit directly from pip 10, as those command will still be available.

Fixes #580
This should be release in a Major 2.0.0 version.

Changelog-friendly one-liner: Vendored pip 9.0.3 to keep compatibility for users with pip 10

Contributor checklist
  • Provided the tests for the changes
  • Requested (or received) a review from another contributor
  • Gave a clear one-line description in the PR (that the maintainers can add to CHANGELOG.md afterwards).

@vphilippon

Copy link
Copy Markdown
Member Author

I didn't change the test matrix to stop testing on multiple pip version right away. I want to make sure that it still works with earlier pip versions (as I took away some conditional code). If it passes, I could clear the matrix to make CI faster.

@vphilippon vphilippon added this to the 2.0.0 milestone Mar 21, 2018
@vphilippon
vphilippon requested a review from davidovich March 21, 2018 22:21
@vphilippon

Copy link
Copy Markdown
Member Author

@pradyunsg FYI

@vphilippon

Copy link
Copy Markdown
Member Author

There seems to be some issue when running through coverage, I'll get digging (likely tomorrow).

@vphilippon

vphilippon commented Mar 22, 2018 •

Copy link
Copy Markdown
Member Author
  • Don't forget to Vendor pip 9.0.3 now that its out, while at it.

@vphilippon vphilippon changed the title Vendor pip 9.0.2 Vendor pip 9.0.9 Mar 22, 2018
@vphilippon vphilippon changed the title Vendor pip 9.0.9 Vendor pip 9.0.3 Mar 22, 2018

@davidovich davidovich left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment thread piptools/utils.py
"""Get an all-lowercase version of the requirement's name."""
if hasattr(req, 'key'):
# pip 8.1.1 or below, using pkg_resources
# from pkg_resources, such as installed dists for pip-sync

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe this will be stable now that we use vendored pip

@vphilippon vphilippon Mar 23, 2018 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It thought so too, but unfortunately there's another condition unrelated to the pip version.
When parsing the installed packages (in pip-sync), the returned object by pip.get_installed_distributions is a DistributionEggInfo or something alike, which comes from pkg_resources, and those object do not have a .name attribute.
The if hasattr(req, 'key') condition masked that case, making us believe it was only a pip difference.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok

Comment thread piptools/utils.py
def name_from_req(req):
"""Get the name of the requirement"""
if hasattr(req, 'project_name'):
# pip 8.1.1 or below, using pkg_resources

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same: I believe this will be stable now that we use vendored pip

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

See comment above: same applies.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok

@vphilippon

Copy link
Copy Markdown
Member Author

For the record: I installed pip 10 (commit https://github.com/pypa/pip/tree/b20235d7c9e6a7ece969567ad0a21b55aea96624), and both pip-compile and pip-sync work fine so far. I went as far as to uninstall pip to test pip-compile, and it keeps working.

Look like we'll survive through this!

@vphilippon
vphilippon merged commit 5c26e9f into jazzband:master Mar 23, 2018
@vphilippon
vphilippon deleted the vendor-pip-9.0.2 branch March 23, 2018 21:30
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.

2 participants