diff --git a/news/2384.bugfix b/news/2384.bugfix index 0261fd31b8..e6e0c01fda 100644 --- a/news/2384.bugfix +++ b/news/2384.bugfix @@ -1 +1 @@ -Dependencies with markers that don't match the current environment will now be skipped during ``pipenv lock``. +Resolved a bug in our patched resolvers which could cause nondeterministic resolution failures in certain conditions. diff --git a/news/2384.feature b/news/2384.feature new file mode 100644 index 0000000000..799c0c1d12 --- /dev/null +++ b/news/2384.feature @@ -0,0 +1 @@ +Optimized hashing speed. diff --git a/news/2384.trivial b/news/2384.trivial new file mode 100644 index 0000000000..21695a9c92 --- /dev/null +++ b/news/2384.trivial @@ -0,0 +1 @@ +Added pytz 2018.4 wheel for testing -- needed for dependency resolution. diff --git a/pipenv/core.py b/pipenv/core.py index 3bc5e6afb9..c365f7ed8e 100644 --- a/pipenv/core.py +++ b/pipenv/core.py @@ -47,13 +47,11 @@ is_star, rmtree, split_argument, - extract_uri_from_vcs_dep, fs_str, clean_resolved_dep, ) from ._compat import ( TemporaryDirectory, - vcs, Path ) from .import pep508checker, progress @@ -103,7 +101,7 @@ ): INSTALL_LABEL = '🎅 ' else: - INSTALL_LABEL = '🐍 ' + INSTALL_LABEL = '🝝 ' INSTALL_LABEL2 = crayons.normal('☤ ', bold=True) STARTING_LABEL = ' ' else: @@ -1006,7 +1004,7 @@ def do_lock( pre=False, keep_outdated=False, write=True, - pypi_mirror = None, + pypi_mirror=None, ): """Executes the freeze functionality.""" from .utils import get_vcs_deps @@ -1382,7 +1380,7 @@ def pip_install( selective_upgrade=False, requirements_dir=None, extra_indexes=None, - pypi_mirror = None, + pypi_mirror=None, ): from notpip._internal import logger as piplogger from notpip._vendor.pyparsing import ParseException diff --git a/pipenv/patched/piptools/repositories/local.py b/pipenv/patched/piptools/repositories/local.py index 08dabe12f6..480ad1ed2f 100644 --- a/pipenv/patched/piptools/repositories/local.py +++ b/pipenv/patched/piptools/repositories/local.py @@ -56,7 +56,7 @@ def find_best_match(self, ireq, prereleases=None): if existing_pin and ireq_satisfied_by_existing_pin(ireq, existing_pin): project, version, _ = as_tuple(existing_pin) return make_install_requirement( - project, version, ireq.extras, constraint=ireq.constraint + project, version, ireq.extras, constraint=ireq.constraint, markers=ireq.markers ) else: return self.repository.find_best_match(ireq, prereleases) diff --git a/pipenv/patched/piptools/repositories/pypi.py b/pipenv/patched/piptools/repositories/pypi.py index 1c0db57170..5759ad07ad 100644 --- a/pipenv/patched/piptools/repositories/pypi.py +++ b/pipenv/patched/piptools/repositories/pypi.py @@ -24,14 +24,14 @@ from pipenv.patched.notpip._vendor.packaging.requirements import InvalidRequirement, Requirement from pipenv.patched.notpip._vendor.packaging.version import Version, InvalidVersion, parse as parse_version -from pipenv.patched.notpip._vendor.packaging.specifiers import SpecifierSet, InvalidSpecifier +from pipenv.patched.notpip._vendor.packaging.specifiers import SpecifierSet, InvalidSpecifier, Specifier from pipenv.patched.notpip._vendor.pyparsing import ParseException from ..cache import CACHE_DIR from pipenv.environments import PIPENV_CACHE_DIR from ..exceptions import NoCandidateFound from ..utils import (fs_str, is_pinned_requirement, lookup_table, as_tuple, key_from_req, - make_install_requirement, format_requirement, dedup) + make_install_requirement, format_requirement, dedup, clean_requires_python) from .base import BaseRepository @@ -165,21 +165,7 @@ def find_best_match(self, ireq, prereleases=None): return ireq # return itself as the best match py_version = parse_version(os.environ.get('PIP_PYTHON_VERSION', str(sys.version_info[:3]))) - all_candidates = [] - for c in self.find_all_candidates(ireq.name): - if c.requires_python: - # Old specifications had people setting this to single digits - # which is effectively the same as '>=digit,=2.6"')` + first_spec, marker_str = specifierset[0]._spec + if len(specifierset) > 1: + marker_str = [marker_str,] + for spec in specifierset[1:]: + marker_str.append(str(spec)) + marker_str = ','.join(marker_str) + # join the leading specifier operator and the rest of the specifiers + marker_str = '{0}"{1}"'.format(first_spec, marker_str) + else: + marker_str = '=="{0}"'.format(requires_python.replace(' ', '')) + # The best way to add markers to a requirement is to make a separate requirement + # with only markers on it, and then to transfer the object istelf + marker_to_add = Requirement('fakepkg; python_version{0}'.format(marker_str)).marker + result.remove(ireq) + ireq.req.marker = marker_to_add + result.add(ireq) self._dependencies_cache[ireq] = result reqset.cleanup_files() + return set(self._dependencies_cache[ireq]) def get_hashes(self, ireq): @@ -399,11 +431,16 @@ def get_hashes(self, ireq): # We need to get all of the candidates that match our current version # pin, these will represent all of the files that could possibly # satisfy this constraint. - all_candidates = self.find_all_candidates(ireq.name) - candidates_by_version = lookup_table(all_candidates, key=lambda c: c.version) - matching_versions = list( - ireq.specifier.filter((candidate.version for candidate in all_candidates))) - matching_candidates = candidates_by_version[matching_versions[0]] + ### Modification -- this is much more efficient.... + ### modification again -- still more efficient + matching_candidates = ( + c for c in clean_requires_python(self.find_all_candidates(ireq.name)) + if c.version in ireq.specifier + ) + # candidates_by_version = lookup_table(all_candidates, key=lambda c: c.version) + # matching_versions = list( + # ireq.specifier.filter((candidate.version for candidate in all_candidates))) + # matching_candidates = candidates_by_version[matching_versions[0]] return { self._hash_cache.get_hash(candidate.location) diff --git a/pipenv/patched/piptools/utils.py b/pipenv/patched/piptools/utils.py index 1d732bf90e..5827a55545 100644 --- a/pipenv/patched/piptools/utils.py +++ b/pipenv/patched/piptools/utils.py @@ -11,13 +11,30 @@ from ._compat import InstallRequirement from first import first - +from pip._vendor.packaging.specifiers import SpecifierSet, InvalidSpecifier from .click import style UNSAFE_PACKAGES = {'setuptools', 'distribute', 'pip'} +def clean_requires_python(candidates): + """Get a cleaned list of all the candidates with valid specifiers in the `requires_python` attributes.""" + all_candidates = [] + for c in candidates: + if c.requires_python: + # Old specifications had people setting this to single digits + # which is effectively the same as '>=digit,=digit,= 10 (new resolver!) -@@ -190,14 +322,64 @@ class PyPIRepository(BaseRepository): +@@ -188,17 +320,99 @@ class PyPIRepository(BaseRepository): + finder=self.finder, + session=self.session, upgrade_strategy="to-satisfy-only", - force_reinstall=False, +- force_reinstall=False, ++ force_reinstall=True, ignore_dependencies=False, - ignore_requires_python=False, + ignore_requires_python=True, @@ -268,33 +278,37 @@ index 1c4b943..7c6521d 100644 ) self.resolver.resolve(reqset) - self._dependencies_cache[ireq] = reqset.requirements.values() -+ result = reqset.requirements.values() ++ result = set(reqset.requirements.values()) + -+ # Collect setup_requires info from local eggs. -+ # Do this after we call the preparer on these reqs to make sure their -+ # egg info has been created -+ setup_requires = {} -+ if ireq.editable: ++ # HACK: Sometimes the InstallRequirement doesn't properly get ++ # these values set on it during the resolution process. It's ++ # difficult to pin down what is going wrong. This fixes things. ++ if not getattr(ireq, 'version', None): + try: -+ dist = ireq.get_dist() -+ if dist.has_metadata('requires.txt'): -+ setup_requires = self.finder.get_extras_links( -+ dist.get_metadata_lines('requires.txt') -+ ) -+ # HACK: Sometimes the InstallRequirement doesn't properly get -+ # these values set on it during the resolution process. It's -+ # difficult to pin down what is going wrong. This fixes things. -+ ireq.version = dist.version -+ ireq.project_name = dist.project_name -+ ireq.req = dist.as_requirement() -+ except (TypeError, ValueError): ++ dist = ireq.get_dist() if not dist else None ++ ireq.version = ireq.get_dist().version ++ except (ValueError, OSError, TypeError) as e: + pass ++ if not getattr(ireq, 'project_name', None): ++ try: ++ ireq.project_name = dist.project_name if dist else None ++ except (ValueError, TypeError) as e: ++ pass ++ if not getattr(ireq, 'req', None): ++ try: ++ ireq.req = dist.as_requirement() if dist else None ++ except (ValueError, TypeError) as e: ++ pass ++ + # Convert setup_requires dict into a somewhat usable form. + if setup_requires: + for section in setup_requires: + python_version = section + not_python = not (section.startswith('[') and ':' in section) + ++ # This is for cleaning up :extras: formatted markers ++ # by adding them to the results of the resolver ++ # since any such extra would have been returned as a result anyway + for value in setup_requires[section]: + # This is a marker. + if value.startswith('[') and ':' in value: @@ -308,21 +322,67 @@ index 1c4b943..7c6521d 100644 + try: + if not not_python: + result = result + [InstallRequirement.from_line("{0}{1}".format(value, python_version).replace(':', ';'))] -+ # Anything could go wrong here — can't be too careful. ++ # Anything could go wrong here -- can't be too careful. + except Exception: + pass ++ ++ # this section properly creates 'python_version' markers for cross-python ++ # virtualenv creation and for multi-python compatibility. + requires_python = reqset.requires_python if hasattr(reqset, 'requires_python') else self.resolver.requires_python + if requires_python: -+ marker = 'python_version=="{0}"'.format(requires_python.replace(' ', '')) -+ new_req = InstallRequirement.from_line('{0}; {1}'.format(str(ireq.req), marker)) -+ result = [new_req] ++ marker_str = '' ++ # This corrects a logic error from the previous code which said that if ++ # we Encountered any 'requires_python' attributes, basically only create a ++ # single result no matter how many we resolved. This should fix ++ # a majority of the remaining non-deterministic resolution issues. ++ if any(requires_python.startswith(op) for op in Specifier._operators.keys()): ++ # We are checking first if we have leading specifier operator ++ # if not, we can assume we should be doing a == comparison ++ specifierset = list(SpecifierSet(requires_python)) ++ # for multiple specifiers, the correct way to represent that in ++ # a specifierset is `Requirement('fakepkg; python_version<"3.0,>=2.6"')` ++ first_spec, marker_str = specifierset[0]._spec ++ if len(specifierset) > 1: ++ marker_str = [marker_str,] ++ for spec in specifierset[1:]: ++ marker_str.append(str(spec)) ++ marker_str = ','.join(marker_str) ++ # join the leading specifier operator and the rest of the specifiers ++ marker_str = '{0}"{1}"'.format(first_spec, marker_str) ++ else: ++ marker_str = '=="{0}"'.format(requires_python.replace(' ', '')) ++ # The best way to add markers to a requirement is to make a separate requirement ++ # with only markers on it, and then to transfer the object istelf ++ marker_to_add = Requirement('fakepkg; python_version{0}'.format(marker_str)).marker ++ result.remove(ireq) ++ ireq.req.marker = marker_to_add ++ result.add(ireq) + + self._dependencies_cache[ireq] = result reqset.cleanup_files() ++ return set(self._dependencies_cache[ireq]) -@@ -224,17 +406,10 @@ class PyPIRepository(BaseRepository): - matching_candidates = candidates_by_version[matching_versions[0]] + def get_hashes(self, ireq): +@@ -217,24 +431,22 @@ class PyPIRepository(BaseRepository): + # We need to get all of the candidates that match our current version + # pin, these will represent all of the files that could possibly + # satisfy this constraint. +- all_candidates = self.find_all_candidates(ireq.name) +- candidates_by_version = lookup_table(all_candidates, key=lambda c: c.version) +- matching_versions = list( +- ireq.specifier.filter((candidate.version for candidate in all_candidates))) +- matching_candidates = candidates_by_version[matching_versions[0]] ++ ### Modification -- this is much more efficient.... ++ ### modification again -- still more efficient ++ matching_candidates = ( ++ c for c in clean_requires_python(self.find_all_candidates(ireq.name)) ++ if c.version in ireq.specifier ++ ) ++ # candidates_by_version = lookup_table(all_candidates, key=lambda c: c.version) ++ # matching_versions = list( ++ # ireq.specifier.filter((candidate.version for candidate in all_candidates))) ++ # matching_candidates = candidates_by_version[matching_versions[0]] return { - self._get_file_hash(candidate.location) @@ -462,11 +522,56 @@ index 05ec8fd..c5eb728 100644 def reverse_dependencies(self, ireqs): non_editable = [ireq for ireq in ireqs if not ireq.editable] +diff --git a/pipenv/patched/piptools/repositories/local.py b/pipenv/patched/piptools/repositories/local.py +index 08dabe1..480ad1e 100644 +--- a/pipenv/patched/piptools/repositories/local.py ++++ b/pipenv/patched/piptools/repositories/local.py +@@ -56,7 +56,7 @@ class LocalRequirementsRepository(BaseRepository): + if existing_pin and ireq_satisfied_by_existing_pin(ireq, existing_pin): + project, version, _ = as_tuple(existing_pin) + return make_install_requirement( +- project, version, ireq.extras, constraint=ireq.constraint ++ project, version, ireq.extras, constraint=ireq.constraint, markers=ireq.markers + ) + else: + return self.repository.find_best_match(ireq, prereleases) diff --git a/pipenv/patched/piptools/utils.py b/pipenv/patched/piptools/utils.py -index fde5816..1d732bf 100644 +index fde5816..5827a55 100644 --- a/pipenv/patched/piptools/utils.py +++ b/pipenv/patched/piptools/utils.py -@@ -43,16 +43,51 @@ def comment(text): +@@ -11,13 +11,30 @@ from contextlib import contextmanager + from ._compat import InstallRequirement + + from first import first +- ++from pip._vendor.packaging.specifiers import SpecifierSet, InvalidSpecifier + from .click import style + + + UNSAFE_PACKAGES = {'setuptools', 'distribute', 'pip'} + + ++def clean_requires_python(candidates): ++ """Get a cleaned list of all the candidates with valid specifiers in the `requires_python` attributes.""" ++ all_candidates = [] ++ for c in candidates: ++ if c.requires_python: ++ # Old specifications had people setting this to single digits ++ # which is effectively the same as '>=digit,