diff --git a/news/2480.bugfix b/news/2480.bugfix new file mode 100644 index 0000000000..9e60e9b2ad --- /dev/null +++ b/news/2480.bugfix @@ -0,0 +1 @@ +Resolved a long-standing issue with re-using previously generated ``InstallRequirement`` objects for resolution which could cause ``PKG-INFO`` file information to be deleted, raising a ``TypeError``. diff --git a/news/2480.vendor b/news/2480.vendor new file mode 100644 index 0000000000..f96f2b3081 --- /dev/null +++ b/news/2480.vendor @@ -0,0 +1 @@ +Unraveled a lot of old, unnecessary patches to ``pip-tools`` which were causing non-deterministic resolution errors. diff --git a/pipenv/patched/piptools/resolver.py b/pipenv/patched/piptools/resolver.py index 2ba85f4bf4..807cf518b0 100644 --- a/pipenv/patched/piptools/resolver.py +++ b/pipenv/patched/piptools/resolver.py @@ -15,7 +15,7 @@ from .cache import DependencyCache from .exceptions import UnsupportedConstraint from .logging import log -from .utils import (format_requirement, format_specifier, full_groupby, dedup, +from .utils import (format_requirement, format_specifier, full_groupby, dedup, simplify_markers, is_pinned_requirement, key_from_ireq, key_from_req, UNSAFE_PACKAGES) green = partial(click.style, fg='green') @@ -160,7 +160,12 @@ def _group_constraints(self, constraints): if ireq.req.specifier._specs and not combined_ireq.req.specifier._specs: combined_ireq.req.specifier._specs = ireq.req.specifier._specs combined_ireq.constraint &= ireq.constraint - combined_ireq.markers = ireq.markers + if not combined_ireq.markers: + combined_ireq.markers = ireq.markers + else: + _markers = combined_ireq.markers._markers + if not isinstance(_markers[0], (tuple, list)): + combined_ireq.markers._markers = [_markers, 'and', ireq.markers._markers] # Return a sorted, de-duped tuple of extras combined_ireq.extras = tuple(sorted(set(tuple(combined_ireq.extras) + tuple(ireq.extras)))) yield combined_ireq @@ -278,25 +283,14 @@ def _iter_dependencies(self, ireq): for dependency in self.repository.get_dependencies(ireq): yield dependency return - elif ireq.markers: - for dependency in self.repository.get_dependencies(ireq): - dependency.prepared = False - yield dependency - return - elif ireq.extras: - valid_markers = default_environment().keys() - for dependency in self.repository.get_dependencies(ireq): - dependency.prepared = False - if dependency.markers and not any(dependency.markers._markers[0][0].value.startswith(k) for k in valid_markers): - dependency.markers = None - if hasattr(ireq, 'extra'): - if ireq.extras: - ireq.extras.extend(ireq.extra) - else: - ireq.extras = ireq.extra - yield dependency - return + # fix our malformed extras + if ireq.extras: + if hasattr(ireq, 'extra'): + if ireq.extras: + ireq.extras.extend(ireq.extra) + else: + ireq.extras = ireq.extra elif not is_pinned_requirement(ireq): raise TypeError('Expected pinned or editable requirement, got {}'.format(ireq)) @@ -307,24 +301,13 @@ def _iter_dependencies(self, ireq): if ireq not in self.dependency_cache: log.debug(' {} not in cache, need to check index'.format(format_requirement(ireq)), fg='yellow') dependencies = self.repository.get_dependencies(ireq) - import sys - self.dependency_cache[ireq] = sorted(format_requirement(ireq) for ireq in dependencies) + self.dependency_cache[ireq] = sorted(format_requirement(_ireq) for _ireq in dependencies) # Example: ['Werkzeug>=0.9', 'Jinja2>=2.4'] dependency_strings = self.dependency_cache[ireq] log.debug(' {:25} requires {}'.format(format_requirement(ireq), ', '.join(sorted(dependency_strings, key=lambda s: s.lower())) or '-')) - from pipenv.patched.notpip._vendor.packaging.markers import InvalidMarker for dependency_string in dependency_strings: - try: - _dependency_string = dependency_string - if ';' in dependency_string: - # split off markers and remove any duplicates by comparing against deps - _dependencies = [dep.strip() for dep in dependency_string.split(';')] - _dependency_string = '; '.join([dep for dep in dedup(_dependencies)]) - - yield InstallRequirement.from_line(_dependency_string, constraint=ireq.constraint) - except InvalidMarker: yield InstallRequirement.from_line(dependency_string, constraint=ireq.constraint) def reverse_dependencies(self, ireqs): diff --git a/pipenv/patched/piptools/utils.py b/pipenv/patched/piptools/utils.py index 0ee22a56ce..2f389eecaf 100644 --- a/pipenv/patched/piptools/utils.py +++ b/pipenv/patched/piptools/utils.py @@ -2,6 +2,7 @@ from __future__ import (absolute_import, division, print_function, unicode_literals) +import six import os import sys from itertools import chain, groupby @@ -13,12 +14,56 @@ from first import first from pipenv.patched.notpip._vendor.packaging.specifiers import SpecifierSet, InvalidSpecifier from pipenv.patched.notpip._vendor.packaging.version import Version, InvalidVersion, parse as parse_version +from pipenv.patched.notpip._vendor.packaging.markers import Marker, Op, Value, Variable from .click import style UNSAFE_PACKAGES = {'setuptools', 'distribute', 'pip'} +def simplify_markers(ireq): + """simplify_markers "This code cleans up markers for a specific :class:`~InstallRequirement`" + + Clean and deduplicate markers. + + :param ireq: An InstallRequirement to clean + :type ireq: :class:`~pip._internal.req.req_install.InstallRequirement` + :return: An InstallRequirement with cleaned Markers + :rtype: :class:`~pip._internal.req.req_install.InstallRequirement` + """ + + if not getattr(ireq, 'markers', None): + return ireq + markers = ireq.markers + marker_list = [] + if isinstance(markers, six.string_types): + if ';' in markers: + markers = [Marker(m_str.strip()) for m_str in markers.split(';')] + else: + markers = Marker(markers) + for m in markers._markers: + _single_marker = [] + if isinstance(m[0], six.string_types): + continue + if not isinstance(m[0], (list, tuple)): + marker_list.append(''.join([_piece.serialize() for _piece in m])) + continue + for _marker_part in m: + if isinstance(_marker_part, six.string_types): + _single_marker.append(_marker_part) + continue + _single_marker.append(''.join([_piece.serialize() for _piece in _marker_part])) + _single_marker = [_m.strip() for _m in _single_marker] + marker_list.append(tuple(_single_marker,)) + marker_str = ' and '.join(list(dedup(tuple(marker_list,)))) if marker_list else '' + new_markers = Marker(marker_str) + ireq.markers = new_markers + new_ireq = InstallRequirement.from_line(format_requirement(ireq)) + if ireq.constraint: + new_ireq.constraint = ireq.constraint + return new_ireq + + def clean_requires_python(candidates): """Get a cleaned list of all the candidates with valid specifiers in the `requires_python` attributes.""" all_candidates = [] @@ -122,7 +167,7 @@ def format_requirement(ireq, marker=None): else: line = _requirement_to_str_lowercase_name(ireq.req) - if marker: + if marker and ';' not in line: line = '{}; {}'.format(line, marker) return line diff --git a/pipenv/resolver.py b/pipenv/resolver.py index 91259bd20c..9f06caa3b4 100644 --- a/pipenv/resolver.py +++ b/pipenv/resolver.py @@ -34,7 +34,6 @@ def main(): new_sys_argv.append(v) sys.argv = new_sys_argv - from pipenv.utils import create_mirror_source, resolve_deps, replace_pypi_sources os.environ['PIP_PYTHON_VERSION'] = '.'.join([str(s) for s in sys.version_info[:3]]) os.environ['PIP_PYTHON_PATH'] = sys.executable if is_verbose: @@ -49,6 +48,7 @@ def main(): for i, package in enumerate(packages): if package.startswith('--'): del packages[i] + from pipenv.utils import create_mirror_source, resolve_deps, replace_pypi_sources pypi_mirror_source = create_mirror_source(os.environ['PIPENV_PYPI_MIRROR']) if 'PIPENV_PYPI_MIRROR' in os.environ else None def resolve(packages, pre, project, sources, verbose, clear, system): diff --git a/pipenv/utils.py b/pipenv/utils.py index 4079988191..d468afcb03 100644 --- a/pipenv/utils.py +++ b/pipenv/utils.py @@ -922,9 +922,11 @@ def is_valid_url(url): def is_pypi_url(url): return bool(re.match(r'^http[s]?:\/\/pypi(?:\.python)?\.org\/simple[\/]?$', url)) + def replace_pypi_sources(sources, pypi_replacement_source): return [pypi_replacement_source] + [source for source in sources if not is_pypi_url(source['url'])] + def create_mirror_source(url): return {'url': url, 'verify_ssl': url.startswith('https://'), 'name': urlparse(url).hostname} diff --git a/pipenv/vendor/click-didyoumean.LICENSE b/pipenv/vendor/click_didyoumean/LICENSE similarity index 100% rename from pipenv/vendor/click-didyoumean.LICENSE rename to pipenv/vendor/click_didyoumean/LICENSE diff --git a/tasks/vendoring/patches/patched/piptools.patch b/tasks/vendoring/patches/patched/piptools.patch index b1ba98a576..f5fb822f4a 100644 --- a/tasks/vendoring/patches/patched/piptools.patch +++ b/tasks/vendoring/patches/patched/piptools.patch @@ -406,7 +406,7 @@ index 1c4b943..c922be1 100644 def allow_all_wheels(self): """ diff --git a/pipenv/patched/piptools/resolver.py b/pipenv/patched/piptools/resolver.py -index 05ec8fd..c5eb728 100644 +index 05ec8fd..2f94f6b 100644 --- a/pipenv/patched/piptools/resolver.py +++ b/pipenv/patched/piptools/resolver.py @@ -8,13 +8,14 @@ from itertools import chain, count @@ -421,7 +421,7 @@ index 05ec8fd..c5eb728 100644 from .exceptions import UnsupportedConstraint from .logging import log -from .utils import (format_requirement, format_specifier, full_groupby, -+from .utils import (format_requirement, format_specifier, full_groupby, dedup, ++from .utils import (format_requirement, format_specifier, full_groupby, dedup, simplify_markers, is_pinned_requirement, key_from_ireq, key_from_req, UNSAFE_PACKAGES) green = partial(click.style, fg='green') @@ -451,7 +451,7 @@ index 05ec8fd..c5eb728 100644 msg = ('pip-compile does not support URLs as packages, unless they are editable. ' 'Perhaps add -e option?') raise UnsupportedConstraint(msg, constraint) -@@ -147,15 +149,18 @@ class Resolver(object): +@@ -147,15 +149,23 @@ class Resolver(object): if editable_ireq: yield editable_ireq # ignore all the other specs: the editable one is the one that counts continue @@ -469,60 +469,43 @@ index 05ec8fd..c5eb728 100644 + if ireq.req.specifier._specs and not combined_ireq.req.specifier._specs: + combined_ireq.req.specifier._specs = ireq.req.specifier._specs combined_ireq.constraint &= ireq.constraint -+ combined_ireq.markers = ireq.markers ++ if not combined_ireq.markers: ++ combined_ireq.markers = ireq.markers ++ else: ++ _markers = combined_ireq.markers._markers ++ if not isinstance(_markers[0], (tuple, list)): ++ combined_ireq.markers._markers = [_markers, 'and', ireq.markers._markers] # Return a sorted, de-duped tuple of extras combined_ireq.extras = tuple(sorted(set(tuple(combined_ireq.extras) + tuple(ireq.extras)))) yield combined_ireq -@@ -271,6 +276,25 @@ class Resolver(object): - """ - if ireq.editable: +@@ -273,6 +283,14 @@ class Resolver(object): for dependency in self.repository.get_dependencies(ireq): -+ yield dependency -+ return -+ elif ireq.markers: -+ for dependency in self.repository.get_dependencies(ireq): -+ dependency.prepared = False -+ yield dependency -+ return -+ elif ireq.extras: -+ valid_markers = default_environment().keys() -+ for dependency in self.repository.get_dependencies(ireq): -+ dependency.prepared = False -+ if dependency.markers and not any(dependency.markers._markers[0][0].value.startswith(k) for k in valid_markers): -+ dependency.markers = None -+ if hasattr(ireq, 'extra'): -+ if ireq.extras: -+ ireq.extras.extend(ireq.extra) -+ else: -+ ireq.extras = ireq.extra -+ yield dependency return ++ ++ # fix our malformed extras ++ if ireq.extras: ++ if hasattr(ireq, 'extra'): ++ if ireq.extras: ++ ireq.extras.extend(ireq.extra) ++ else: ++ ireq.extras = ireq.extra elif not is_pinned_requirement(ireq): -@@ -283,14 +307,25 @@ class Resolver(object): + raise TypeError('Expected pinned or editable requirement, got {}'.format(ireq)) + +@@ -283,14 +301,14 @@ class Resolver(object): if ireq not in self.dependency_cache: log.debug(' {} not in cache, need to check index'.format(format_requirement(ireq)), fg='yellow') dependencies = self.repository.get_dependencies(ireq) - self.dependency_cache[ireq] = sorted(str(ireq.req) for ireq in dependencies) -+ import sys -+ self.dependency_cache[ireq] = sorted(format_requirement(ireq) for ireq in dependencies) ++ self.dependency_cache[ireq] = sorted(format_requirement(_ireq) for _ireq in dependencies) # Example: ['Werkzeug>=0.9', 'Jinja2>=2.4'] dependency_strings = self.dependency_cache[ireq] log.debug(' {:25} requires {}'.format(format_requirement(ireq), ', '.join(sorted(dependency_strings, key=lambda s: s.lower())) or '-')) -+ from pip._vendor.packaging.markers import InvalidMarker for dependency_string in dependency_strings: - yield InstallRequirement.from_line(dependency_string, constraint=ireq.constraint) -+ try: -+ _dependency_string = dependency_string -+ if ';' in dependency_string: -+ # split off markers and remove any duplicates by comparing against deps -+ _dependencies = [dep.strip() for dep in dependency_string.split(';')] -+ _dependency_string = '; '.join([dep for dep in dedup(_dependencies)]) -+ -+ yield InstallRequirement.from_line(_dependency_string, constraint=ireq.constraint) -+ except InvalidMarker: + yield InstallRequirement.from_line(dependency_string, constraint=ireq.constraint) def reverse_dependencies(self, ireqs): @@ -541,22 +524,74 @@ index 08dabe1..480ad1e 100644 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..fb71882 100644 +index fde5816..23a05f2 100644 --- a/pipenv/patched/piptools/utils.py +++ b/pipenv/patched/piptools/utils.py -@@ -11,13 +11,35 @@ from contextlib import contextmanager +@@ -2,6 +2,7 @@ + from __future__ import (absolute_import, division, print_function, + unicode_literals) + ++import six + import os + import sys + from itertools import chain, groupby +@@ -11,13 +12,79 @@ from contextlib import contextmanager from ._compat import InstallRequirement from first import first - +from pip._vendor.packaging.specifiers import SpecifierSet, InvalidSpecifier +from pip._vendor.packaging.version import Version, InvalidVersion, parse as parse_version ++from pip._vendor.packaging.markers import Marker, Op, Value, Variable from .click import style UNSAFE_PACKAGES = {'setuptools', 'distribute', 'pip'} ++def simplify_markers(ireq): ++ """simplify_markers "This code cleans up markers for a specific :class:`~InstallRequirement`" ++ ++ Clean and deduplicate markers. ++ ++ :param ireq: An InstallRequirement to clean ++ :type ireq: :class:`~pip._internal.req.req_install.InstallRequirement` ++ :return: An InstallRequirement with cleaned Markers ++ :rtype: :class:`~pip._internal.req.req_install.InstallRequirement` ++ """ ++ ++ if not getattr(ireq, 'markers', None): ++ return ireq ++ markers = ireq.markers ++ marker_list = [] ++ if isinstance(markers, six.string_types): ++ if ';' in markers: ++ markers = [Marker(m_str.strip()) for m_str in markers.split(';')] ++ else: ++ markers = Marker(markers) ++ for m in markers._markers: ++ _single_marker = [] ++ if isinstance(m[0], six.string_types): ++ continue ++ if not isinstance(m[0], (list, tuple)): ++ marker_list.append(''.join([_piece.serialize() for _piece in m])) ++ continue ++ for _marker_part in m: ++ if isinstance(_marker_part, six.string_types): ++ _single_marker.append(_marker_part) ++ continue ++ _single_marker.append(''.join([_piece.serialize() for _piece in _marker_part])) ++ _single_marker = [_m.strip() for _m in _single_marker] ++ marker_list.append(tuple(_single_marker,)) ++ marker_str = ' and '.join(list(dedup(tuple(marker_list,)))) if marker_list else '' ++ new_markers = Marker(marker_str) ++ ireq.markers = new_markers ++ new_ireq = InstallRequirement.from_line(format_requirement(ireq)) ++ if ireq.constraint: ++ new_ireq.constraint = ireq.constraint ++ return new_ireq ++ ++ +def clean_requires_python(candidates): + """Get a cleaned list of all the candidates with valid specifiers in the `requires_python` attributes.""" + all_candidates = [] @@ -581,7 +616,7 @@ index fde5816..fb71882 100644 def key_from_ireq(ireq): """Get a standardized key for an InstallRequirement.""" if ireq.req is None and ireq.link is not None: -@@ -43,16 +65,51 @@ def comment(text): +@@ -43,16 +110,51 @@ def comment(text): return style(text, fg='green') @@ -637,15 +672,16 @@ index fde5816..fb71882 100644 def format_requirement(ireq, marker=None): -@@ -63,10 +120,10 @@ def format_requirement(ireq, marker=None): +@@ -63,10 +165,10 @@ def format_requirement(ireq, marker=None): if ireq.editable: line = '-e {}'.format(ireq.link) else: - line = str(ireq.req).lower() + line = _requirement_to_str_lowercase_name(ireq.req) - if marker: +- if marker: - line = '{} ; {}'.format(line, marker) ++ if marker and ';' not in line: + line = '{}; {}'.format(line, marker) return line diff --git a/tests/integration/test_install_markers.py b/tests/integration/test_install_markers.py index ca6b70fe99..42401c159b 100644 --- a/tests/integration/test_install_markers.py +++ b/tests/integration/test_install_markers.py @@ -37,7 +37,8 @@ def test_package_environment_markers(PipenvInstance, pypi): @pytest.mark.markers @flaky def test_platform_python_implementation_marker(PipenvInstance, pypi): - """Markers should be converted during locking to help users who input this incorrectly + """Markers should be converted during locking to help users who input this + incorrectly. """ with PipenvInstance(pypi=pypi) as p: with open(p.pipfile_path, 'w') as f: @@ -50,9 +51,12 @@ def test_platform_python_implementation_marker(PipenvInstance, pypi): c = p.pipenv('install') assert c.return_code == 0 - # depends-on-marked-package has an install_requires of 'pytz; platform_python_implementation=="CPython"' + # depends-on-marked-package has an install_requires of + # 'pytz; platform_python_implementation=="CPython"' # Verify that that marker shows up in our lockfile unaltered. - assert p.lockfile['default']['pytz']['markers'] == "platform_python_implementation == 'CPython'" + assert 'pytz' in p.lockfile['default'] + assert p.lockfile['default']['pytz'].get('markers') == \ + "platform_python_implementation == 'CPython'" @pytest.mark.run