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

Continuation of #3137 #3231

Merged
merged 42 commits into from
Nov 7, 2015
Merged

Continuation of #3137 #3231

merged 42 commits into from
Nov 7, 2015

Conversation

dstufft
Copy link
Member

@dstufft dstufft commented Nov 6, 2015

Closes #3137

Review on Reviewable

We purposely keep it off the CLI for now. optparse isn't really geared to expose interspersed args and options, so a more heavy-handed approach will be necessary to support things like `pip install SomePackage --sha256=abcdef... OtherPackage --sha256=012345...`.
…f packages. Close pypa#1175.

* Add --require-hashes option. This is handy in deployment scripts to force application authors to hash their requirements. It is also a convenient way to get pip to show computed hashes for a virgin, unhashed requirements file. Eventually, additions to `pip freeze` should fill a superset of this use case.
  * In --require-hashes mode, at least one hash is required to match for each requirement.
  * Option-based requirements (--sha256=...) turn on --require-hashes mode implicitly.
  * Internet-derived URL-based hashes are "necessary but not sufficient": they do not satisfy --require-hashes mode when they match, but they are still used to guard against transmission errors.
  * Other URL-based requirements (#md5=...) are treated just like flag-based ones, except they don't turn on --require-hashes.
* Complain informatively, with the most devastating errors first so you don't chase your tail all day only to run up against a brick wall at the end. This also means we don't complain that a hash is missing, only for the user to find, after fixing it, that we have no idea how to even compute a hash for that type of requirement.
  * Complain about unpinned requirements when hash-checking mode is on, lest they cause the user surprise later.
  * Complain about missing hashes.
  * Complain about requirement types we don't know how to hash (like VCS ones and local dirs).
* Have InstallRequirement keep its original Link around (original_link) so we can differentiate between URL hashes from requirements files and ones downloaded from the (untrustworthy) internet.
* Remove test_download_hashes, which is obsolete. Similar coverage is provided in test_utils.TestHashes and the various hash cases in test_req.py.
Everybody seems to favor this. Spelled -H, it's still pretty short. And it is less unusual programmatically.
dstufft is nervous about blowing a single-char option on something that will usually be copied and pasted anyway. We can always put it back later if it proves to be a pain.
For dependencies that are properly pinned and hashed (not really dependencies at all, if you like, since they're explicit, root-level requirements), we install them as normal. For ones that are not pinned and hashes, we raise the errors typical of any unhashed requirement in --require-hashes mode.

Since the stanza under "if not ignore_dependencies" doesn't actually add anything if it's already in the RequirementSet, not much has to be done in the way of code: the unhashed deps don't have any hashes, so we complain about them as per usual.

Also...
* Revise wording of HashUnpinned errors. They can be raised even if no hash is specified, so the previous wording was misleading.
* Make wording of HashMissing less awkward.
Previously, Hash Verification, Editable Installs, Controlling setup_requires, and Build System Interface were all getting placed under it.
Those commands already checked hashes, since they use RequirementSet, where the hash-checking is done.

Reorder some options so pre, no-clean, and require-hashes are always in the same order.
Standardize on present tense, improve flow, and clarify.
An info-level message for each package might be too intense. And it might give a false sense of security as well: it doesn't confirm that the virtualenv is non-empty; it merely notices when a package we're installing is already there.
This guards against the possibility of a weaker hash being added to hashlib in the future. Also give _good_hashes() a more descriptive name, and describe what we mean by "strong".

We can get away with returning a static list because those algorithms are guaranteed present in hashlib.
We don't need to talk about the network, since HTTPS should ensure transmission integrity. We do need to watch out for the CA chain. Stop mentioning the CDN because it's a deep hole: we might as well mention Rackspace and Amazon and who knows who else.
Renaming "gots" didn't go well. I think the current naming is the most concise way to put it. If we rename it to "got", then the loop iterator can't be called "got", and the simple relationship between the iterator and collection names is lost. "Actual" and "actuals" are the other names that occurred to me, but they look so much like "allowed" that the code becomes harder to read.
erikrose and others added 12 commits October 12, 2015 14:37
This reverts commit 62ac258.

pypa#3176 is about to add the missing piece that makes this code useful (and not dead), so let's not delete it.
… wheel.

This would occur when, for example, installing from a requirements file that references a certain hashed sdist, a common situation.

As of pip 7, pip always tries to build a wheel for each requirement (if one wasn't provided directly) and installs from that. The way this was implemented, InstallRequirement.link pointed to the cached wheel, which obviously had a different hash than the index-sourced archive, so spurious mismatch errors would result.

Now we no longer read from the wheel cache in hash-checking mode.

Make populate_link(), rather than the `link` setter, responsible for mapping InstallRequirement.link to a cached wheel. populate_link() isn't called until until prepare_files(). At that point, when we've examined all InstallRequirements and their potential --hash options, we know whether we should be requiring hashes and thus whether to use the wheel cache at all.

The only place that sets InstallRequirement.link other than InstallRequirement itself is pip.wheel, which does so long after hashes have been checked, when it's unpacking the wheel it just built, so it won't cause spurious hash mismatches.
Removed the mention of "package index options" in the docs, because they don't all fit that category anymore. Not even --no-binary and --only-binary do; they're "install options".
dstufft added a commit that referenced this pull request Nov 7, 2015
@dstufft dstufft merged commit 1da0ea1 into pypa:develop Nov 7, 2015
@dstufft dstufft deleted the hashes2 branch November 7, 2015 00:34
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 3, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants