Skip to content

Commit

Permalink
Merge pull request #2480 from pypa/bugfix/2435-pkg-resources-typeerror
Browse files Browse the repository at this point in the history
Update piptools to avoid reusing InstallRequirement
  • Loading branch information
techalchemy authored Jul 1, 2018
2 parents ca54b6d + 7dd8579 commit 74b7a09
Show file tree
Hide file tree
Showing 9 changed files with 153 additions and 81 deletions.
1 change: 1 addition & 0 deletions news/2480.bugfix
Original file line number Diff line number Diff line change
@@ -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``.
1 change: 1 addition & 0 deletions news/2480.vendor
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Unraveled a lot of old, unnecessary patches to ``pip-tools`` which were causing non-deterministic resolution errors.
47 changes: 15 additions & 32 deletions pipenv/patched/piptools/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))

Expand All @@ -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):
Expand Down
47 changes: 46 additions & 1 deletion pipenv/patched/piptools/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 = []
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pipenv/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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):
Expand Down
2 changes: 2 additions & 0 deletions pipenv/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}

Expand Down
File renamed without changes.
124 changes: 80 additions & 44 deletions tasks/vendoring/patches/patched/piptools.patch
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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')
Expand Down Expand Up @@ -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
Expand All @@ -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):
Expand All @@ -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 = []
Expand All @@ -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')


Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 74b7a09

Please sign in to comment.