Skip to content

Commit

Permalink
Better workaround for cache poisoning pypa#3025
Browse files Browse the repository at this point in the history
Make sure ``pip wheel`` never outputs pure python wheels with a
python implementation tag. Better fix/workaround for
`pypa#3025 <https://github.com/pypa/pip/issues/3025>`_ by
using a per-implementation wheel cache instead of caching pure python
wheels with an implementation tag in their name.

Fixes pypa#7296
  • Loading branch information
sbidoul committed Dec 2, 2019
1 parent b8c16a0 commit 178cd3f
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 13 deletions.
5 changes: 5 additions & 0 deletions news/7296.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Make sure ``pip wheel`` never outputs pure python wheels with a
python implementation tag. Better fix/workaround for
`#3025 <https://github.com/pypa/pip/issues/3025>`_ by
using a per-implementation wheel cache instead of caching pure python
wheels with an implementation tag in their name.
16 changes: 15 additions & 1 deletion src/pip/_internal/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@
import hashlib
import logging
import os
import sys

from pip._vendor.packaging.utils import canonicalize_name

from pip._internal.exceptions import InvalidWheelFilename
from pip._internal.models.link import Link
from pip._internal.models.wheel import Wheel
from pip._internal.utils.compat import expanduser
from pip._internal.utils.misc import interpreter_name
from pip._internal.utils.temp_dir import TempDirectory
from pip._internal.utils.typing import MYPY_CHECK_RUNNING
from pip._internal.utils.urls import path_to_url
Expand Down Expand Up @@ -65,11 +67,23 @@ def _get_cache_path_parts(self, link):
)
key_url = "#".join(key_parts)

# Include interpreter name, major and minor version in cache key
# to cope with ill-behaved sdists that build a different wheel
# depending on the python version their setup.py is being run on,
# and don't encode the difference in compatibility tags.
# https://github.com/pypa/pip/issues/7296
key = "{}-{}.{} {}".format(
interpreter_name(),
sys.version_info[0],
sys.version_info[1],
key_url,
)

# Encode our key url with sha224, we'll use this because it has similar
# security properties to sha256, but with a shorter total output (and
# thus less secure). However the differences don't make a lot of
# difference for our use case here.
hashed = hashlib.sha224(key_url.encode()).hexdigest()
hashed = hashlib.sha224(key.encode()).hexdigest()

# We want to nest the directories some to prevent having a ton of top
# level directories where we might run out of sub directories on some
Expand Down
11 changes: 11 additions & 0 deletions src/pip/_internal/utils/misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import io
import logging
import os
import platform
import posixpath
import shutil
import stat
Expand Down Expand Up @@ -879,3 +880,13 @@ def hash_file(path, blocksize=1 << 20):
length += len(block)
h.update(block)
return (h, length) # type: ignore


def interpreter_name():
# type: () -> str
try:
name = sys.implementation.name # type: ignore
except AttributeError: # pragma: no cover
# Python 2.7 compatibility.
name = platform.python_implementation().lower()
return name
10 changes: 1 addition & 9 deletions src/pip/_internal/wheel_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import re
import shutil

from pip._internal import pep425tags
from pip._internal.models.link import Link
from pip._internal.utils.logging import indent_log
from pip._internal.utils.marker_files import has_delete_marker_file
Expand Down Expand Up @@ -447,10 +446,6 @@ def build(
', '.join([req.name for (req, _) in buildset]),
)

python_tag = None
if should_unpack:
python_tag = pep425tags.implementation_tag

with indent_log():
build_success, build_failure = [], []
for req, output_dir in buildset:
Expand All @@ -464,10 +459,7 @@ def build(
build_failure.append(req)
continue

wheel_file = self._build_one(
req, output_dir,
python_tag=python_tag,
)
wheel_file = self._build_one(req, output_dir)
if wheel_file:
if should_unpack:
# XXX: This is mildly duplicative with prepare_files,
Expand Down
5 changes: 2 additions & 3 deletions tests/functional/test_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
from pip._vendor.six import PY2

from pip import __version__ as pip_current_version
from pip._internal import pep425tags
from pip._internal.cli.status_codes import ERROR, SUCCESS
from pip._internal.models.index import PyPI, TestPyPI
from pip._internal.utils.misc import rmtree
Expand Down Expand Up @@ -1297,9 +1296,9 @@ def test_install_builds_wheels(script, data, with_wheel):
assert "Running setup.py install for requir" not in str(res), str(res)
# wheelbroken has to run install
assert "Running setup.py install for wheelb" in str(res), str(res)
# We want to make sure we used the correct implementation tag
# We want to make sure pure python wheels do not have an implementation tag
assert wheels == [
"Upper-2.0-{}-none-any.whl".format(pep425tags.implementation_tag),
"Upper-2.0-py{}-none-any.whl".format(sys.version_info[0]),
]


Expand Down

0 comments on commit 178cd3f

Please sign in to comment.