Hash packages without unpacking (Fixes #512 and #544) #557
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some distributions have incompatible type of files with same name on the root directory. For example, matplotlib-2.0.2.tar.gz has a directory named "LICENSE" on the root, which may conflict many other distributions, causing errors like: IOError: [Errno 20] Not a directory: u'/tmp/tmpz/LICENSE/LICENSE_STIX' So we need to isolate unpacking directory of each distributions.
suutari
force-pushed
the
dont-unpack-for-hashing
branch
from
1b9fec8 to
e264bd3
Compare
The PyPIRepository._get_file_hash used to call unpack_url, when generating the hash. It only needed the side effect of the downloaded package being left in the download directory and the unpacking part was actually unnecessary. Change it to just open the (local or remote) package as a file object and hash the contents without unpacking. This makes it faster and lighter, since unpacking consumes CPU cycles and disk space, and more importantly, avoids problems which happen when some distribution has a file with the same name as a directory in another. Unpacking both to packages to the same directory will then fail. E.g. matplotlib-2.0.2.tar.gz has a directory named LICENSE, but many other packages have a file named LICENSE. Fixes jazzband#512, jazzband#544
suutari
force-pushed
the
dont-unpack-for-hashing
branch
from
e264bd3 to
627dbaf
Compare
3 tasks
suutari
changed the title
|
Can this be merged, please? |
|
@suutari it would be even better if you pulled the SHA256 hashes from PyPI directly. |
|
Fixes #544 |
|
IMHO Pulling the hashesh from Warehouse is a different issue. This fixes two existing bugs. That would be a new feature, so I'd rather create a new PR of it. |
|
This would also help solve sdispater/poet#21 - which is basically an extension of #512. It would be great to see this merged. |
|
@suutari @suutari-ai Could you rebase on master, I'll be ready to get this in right away. Thanks! |
|
Thanks for the PR! |
This was referenced Sep 27, 2017
This was referenced Sep 27, 2017
This was referenced Nov 19, 2017
This was referenced Nov 22, 2017
This was referenced Nov 23, 2017
This was referenced Nov 30, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The PyPIRepository._get_file_hash used to call unpack_url, when
generating the hash. It only needed the side effect of the downloaded
package being left in the download directory and the unpacking part was
actually unnecessary. Change it to just open the (local or remote)
package as a file object and hash the contents without unpacking.
This makes it faster and lighter, since unpacking consumes CPU cycles
and disk space, and more importantly, avoids problems which happen when
some distribution has a file with the same name as a directory in
another. Unpacking both to packages to the same directory will then
fail. E.g. matplotlib-2.0.2.tar.gz has a directory named LICENSE, but
many other packages have a file named LICENSE.
Fixes #512, #544
Contributor checklist