diff --git a/news/3289.bugfix.rst b/news/3289.bugfix.rst new file mode 100644 index 0000000000..ff089313c3 --- /dev/null +++ b/news/3289.bugfix.rst @@ -0,0 +1 @@ +Fixed a race condition in hash resolution for dependencies for certain dependencies with missing cache entries or fresh Pipenv installs. diff --git a/pipenv/utils.py b/pipenv/utils.py index c3173fc235..b1b73316d5 100644 --- a/pipenv/utils.py +++ b/pipenv/utils.py @@ -19,26 +19,13 @@ from first import first from vistir.misc import fs_str -six.add_move(six.MovedAttribute("Mapping", "collections", "collections.abc")) -six.add_move(six.MovedAttribute("Sequence", "collections", "collections.abc")) -from six.moves import Mapping, Sequence +six.add_move(six.MovedAttribute("Mapping", "collections", "collections.abc")) # noqa +six.add_move(six.MovedAttribute("Sequence", "collections", "collections.abc")) # noqa +six.add_move(six.MovedAttribute("Set", "collections", "collections.abc")) # noqa +from six.moves import Mapping, Sequence, Set from vistir.compat import ResourceWarning -try: - from weakref import finalize -except ImportError: - try: - from .vendor.backports.weakref import finalize - except ImportError: - - class finalize(object): - def __init__(self, *args, **kwargs): - logging.warn("weakref.finalize unavailable, not cleaning...") - - def detach(self): - return False - logging.basicConfig(level=logging.ERROR) @@ -411,40 +398,55 @@ def resolve(self): self.resolved_tree.update(results) return self.resolved_tree - def resolve_hashes(self): - def _should_include_hash(ireq): - from pipenv.vendor.vistir.compat import Path, to_native_string - from pipenv.vendor.vistir.path import url_to_path + @staticmethod + def _should_include_hash(ireq): + from pipenv.vendor.vistir.compat import Path, to_native_string + from pipenv.vendor.vistir.path import url_to_path - # We can only hash artifacts. - try: - if not ireq.link.is_artifact: - return False - except AttributeError: + # We can only hash artifacts. + try: + if not ireq.link.is_artifact: return False + except AttributeError: + return False - # But we don't want normal pypi artifcats since the normal resolver - # handles those - if is_pypi_url(ireq.link.url): - return False + # But we don't want normal pypi artifcats since the normal resolver + # handles those + if is_pypi_url(ireq.link.url): + return False - # We also don't want to try to hash directories as this will fail - # as these are editable deps and are not hashable. - if (ireq.link.scheme == "file" and - Path(to_native_string(url_to_path(ireq.link.url))).is_dir()): - return False - return True + # We also don't want to try to hash directories as this will fail + # as these are editable deps and are not hashable. + if (ireq.link.scheme == "file" and + Path(to_native_string(url_to_path(ireq.link.url))).is_dir()): + return False + return True + def resolve_hashes(self): if self.results is not None: resolved_hashes = self.resolver.resolve_hashes(self.results) for ireq, ireq_hashes in resolved_hashes.items(): + # We _ALWAYS MUST PRIORITIZE_ the inclusion of hashes from local sources + # PLEASE *DO NOT MODIFY THIS* TO CHECK WHETHER AN IREQ ALREADY HAS A HASH + # RESOLVED. The resolver will pull hashes from PyPI and only from PyPI. + # The entire purpose of this approach is to include missing hashes. + # This fixes a race condition in resolution for missing dependency caches + # see pypa/pipenv#3289 + if self._should_include_hash(ireq) and ( + not ireq_hashes or ireq.link.scheme == "file" + ): + if not ireq_hashes: + ireq_hashes = set() + new_hashes = self.resolver.repository._hash_cache.get_hash(ireq.link) + add_to_set(ireq_hashes, new_hashes) + else: + ireq_hashes = set(ireq_hashes) + # The _ONLY CASE_ where we flat out set the value is if it isn't present + # It's a set, so otherwise we *always* need to do a union update if ireq not in self.hashes: - if _should_include_hash(ireq): - self.hashes[ireq] = [ - self.resolver.repository._hash_cache.get_hash(ireq.link) - ] - else: - self.hashes[ireq] = ireq_hashes + self.hashes[ireq] = ireq_hashes + else: + self.hashes[ireq] |= ireq_hashes return self.hashes @@ -1485,3 +1487,15 @@ def sys_version(version_tuple): sys.version_info = version_tuple yield sys.version_info = old_version + + +def add_to_set(original_set, element): + """Given a set and some arbitrary element, add the element(s) to the set""" + if not element: + return original_set + if isinstance(element, Set): + original_set |= element + elif isinstance(element, (list, tuple)): + original_set |= set(element) + else: + original_set.add(element) diff --git a/tests/integration/test_lock.py b/tests/integration/test_lock.py index 831b164408..8ab5608159 100644 --- a/tests/integration/test_lock.py +++ b/tests/integration/test_lock.py @@ -1,5 +1,6 @@ import pytest import os +import sys from pipenv.utils import temp_environ @@ -517,3 +518,25 @@ def test_lock_no_warnings(PipenvInstance, pypi): assert "Warning" in c.err assert "Warning" not in c.out assert "hello" in c.out + + +@pytest.mark.lock +@pytest.mark.install +@pytest.mark.skipif(sys.version_info >= (3, 5), reason="scandir doesn't get installed on python 3.5+") +def test_lock_missing_cache_entries_gets_all_hashes(monkeypatch, PipenvInstance, pypi, tmpdir): + """ + Test locking pathlib2 on python2.7 which needs `scandir`, but fails to resolve when + using a fresh dependency cache. + """ + + with monkeypatch.context() as m: + monkeypatch.setattr("pipenv.patched.piptools.locations.CACHE_DIR", tmpdir.strpath) + with PipenvInstance(pypi=pypi, chdir=True) as p: + p._pipfile.add("pathlib2", "*") + assert "pathlib2" in p.pipfile["packages"] + c = p.pipenv("install") + assert c.return_code == 0, c.err + assert "pathlib2" in p.lockfile["default"] + assert "scandir" in p.lockfile["default"] + assert isinstance(p.lockfile["default"]["scandir"]["hashes"], list) + assert len(p.lockfile["default"]["scandir"]["hashes"]) > 1 diff --git a/tests/pypi/pathlib2/pathlib2-2.3.2-py2.py3-none-any.whl b/tests/pypi/pathlib2/pathlib2-2.3.2-py2.py3-none-any.whl new file mode 100644 index 0000000000..0cd52d8517 Binary files /dev/null and b/tests/pypi/pathlib2/pathlib2-2.3.2-py2.py3-none-any.whl differ diff --git a/tests/pypi/pathlib2/pathlib2-2.3.2.tar.gz b/tests/pypi/pathlib2/pathlib2-2.3.2.tar.gz new file mode 100644 index 0000000000..86e784d135 Binary files /dev/null and b/tests/pypi/pathlib2/pathlib2-2.3.2.tar.gz differ diff --git a/tests/pypi/scandir/scandir-1.9.0-cp27-cp27m-win32.whl b/tests/pypi/scandir/scandir-1.9.0-cp27-cp27m-win32.whl new file mode 100644 index 0000000000..fd2f1047cd Binary files /dev/null and b/tests/pypi/scandir/scandir-1.9.0-cp27-cp27m-win32.whl differ diff --git a/tests/pypi/scandir/scandir-1.9.0-cp27-cp27m-win_amd64.whl b/tests/pypi/scandir/scandir-1.9.0-cp27-cp27m-win_amd64.whl new file mode 100644 index 0000000000..7c718c6705 Binary files /dev/null and b/tests/pypi/scandir/scandir-1.9.0-cp27-cp27m-win_amd64.whl differ diff --git a/tests/pypi/scandir/scandir-1.9.0-cp36-cp36m-win32.whl b/tests/pypi/scandir/scandir-1.9.0-cp36-cp36m-win32.whl new file mode 100644 index 0000000000..39bff79940 Binary files /dev/null and b/tests/pypi/scandir/scandir-1.9.0-cp36-cp36m-win32.whl differ diff --git a/tests/pypi/scandir/scandir-1.9.0-cp36-cp36m-win_amd64.whl b/tests/pypi/scandir/scandir-1.9.0-cp36-cp36m-win_amd64.whl new file mode 100644 index 0000000000..10a8d7ef42 Binary files /dev/null and b/tests/pypi/scandir/scandir-1.9.0-cp36-cp36m-win_amd64.whl differ diff --git a/tests/pypi/scandir/scandir-1.9.0.tar.gz b/tests/pypi/scandir/scandir-1.9.0.tar.gz new file mode 100644 index 0000000000..f134aace88 Binary files /dev/null and b/tests/pypi/scandir/scandir-1.9.0.tar.gz differ