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

Fix pip-sync to check hashes #706

Merged
merged 2 commits into from
Jan 15, 2019
Merged

Fix pip-sync to check hashes #706

merged 2 commits into from
Jan 15, 2019

Conversation

atugushev
Copy link
Member

@atugushev atugushev commented Jan 6, 2019 •
edited
Loading

Fix pip-sync to check hashes when installing packages.
Fixes #619.

Changelog-friendly one-liner: Fix pip-sync to check hashes

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).

jdufresne
jdufresne approved these changes Jan 11, 2019
View reviewed changes
Copy link
Member

@jdufresne jdufresne left a comment

Choose a reason for hiding this comment

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

This is fantastic! Thanks for implementing this. I consider my comments pretty minor and don't necessarily need to hold up merging this PR if they are more trouble than they are worth.

piptools/sync.py Outdated Show resolved Hide resolved
piptools/utils.py Outdated Show resolved Hide resolved
piptools/utils.py Outdated Show resolved Hide resolved
tests/test_sync.py Show resolved Hide resolved
@atugushev
Fix pip-sync to check hashes
57ad548 
Under the hood the pip-sync installs packages by `pip install pkg1 pkg2 ...`,
which causes the issue with checking hashes.

Steps to fix:
- gather all InstallRequirements into a temporary requirement file
- install packages by `pip install -r temp-requirements.txt`
RDIL
RDIL approved these changes Jan 11, 2019
View reviewed changes
Copy link

@RDIL RDIL left a comment

Choose a reason for hiding this comment

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

I’ll merge this pull request tomorrow, any objections please speak up.

@RDIL
Copy link

RDIL commented Jan 14, 2019

Sorry, I’m no longer a part of the organization and am not authorized to review code at this time.

@jdufresne jdufresne merged commit cf2264e into jazzband:master Jan 15, 2019
@jdufresne
Copy link
Member

Thanks @atugushev !

@vphilippon vphilippon added this to the 3.3.0 milestone Jan 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RDIL RDIL RDIL approved these changes

@jdufresne jdufresne jdufresne approved these changes

@vphilippon vphilippon Awaiting requested review from vphilippon

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

Successfully merging this pull request may close these issues.
4 participants
@atugushev @RDIL @jdufresne @vphilippon