Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated piptools to only lock compatible packages #2430

Merged
merged 13 commits into from
Jun 29, 2018
1 change: 1 addition & 0 deletions news/1901.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed an ongoing bug which sometimes resolved incompatible versions into lockfiles.
9 changes: 7 additions & 2 deletions pipenv/patched/piptools/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
from ._compat import InstallRequirement

from first import first
from pip._vendor.packaging.specifiers import SpecifierSet, InvalidSpecifier
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 .click import style


Expand All @@ -21,16 +22,20 @@
def clean_requires_python(candidates):
"""Get a cleaned list of all the candidates with valid specifiers in the `requires_python` attributes."""
all_candidates = []
py_version = parse_version(os.environ.get('PIP_PYTHON_VERSION', '.'.join(map(str, sys.version_info[:3]))))
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+1'
if c.requires_python.isdigit():
c.requires_python = '>={0},<{1}'.format(c.requires_python, int(c.requires_python) + 1)
try:
SpecifierSet(c.requires_python)
specifierset = SpecifierSet(c.requires_python)
except InvalidSpecifier:
continue
else:
if not specifierset.contains(py_version):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will always be true, because os.environ.get('PIP_PYTHON_VERSION') = 'False'

Please merge #2437 and the CI will pass

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still failing :(

continue
all_candidates.append(c)
return all_candidates

Expand Down
12 changes: 9 additions & 3 deletions pipenv/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,12 +180,18 @@ def __init__(self, python_version, python_path):
self.python_path = python_path

def __enter__(self):
os.environ['PIP_PYTHON_VERSION'] = str(self.python_version)
os.environ['PIP_PYTHON_PATH'] = str(self.python_path)
# Only inject when the value is valid
if self.python_version:
os.environ['PIP_PYTHON_VERSION'] = str(self.python_version)
if self.python_path:
os.environ['PIP_PYTHON_PATH'] = str(self.python_path)

def __exit__(self, *args):
# Restore original Python version information.
del os.environ['PIP_PYTHON_VERSION']
try:
del os.environ['PIP_PYTHON_VERSION']
except KeyError:
pass


def prepare_pip_source_args(sources, pip_args=None):
Expand Down
15 changes: 10 additions & 5 deletions tasks/vendoring/patches/patched/piptools.patch
Original file line number Diff line number Diff line change
Expand Up @@ -541,15 +541,16 @@ 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..5827a55 100644
index fde5816..fb71882 100644
--- a/pipenv/patched/piptools/utils.py
+++ b/pipenv/patched/piptools/utils.py
@@ -11,13 +11,30 @@ from contextlib import contextmanager
@@ -11,13 +11,35 @@ 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 .click import style


Expand All @@ -559,24 +560,28 @@ index fde5816..5827a55 100644
+def clean_requires_python(candidates):
+ """Get a cleaned list of all the candidates with valid specifiers in the `requires_python` attributes."""
+ all_candidates = []
+ py_version = parse_version(os.environ.get('PIP_PYTHON_VERSION', '.'.join(map(str, sys.version_info[:3]))))
+ 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+1'
+ if c.requires_python.isdigit():
+ c.requires_python = '>={0},<{1}'.format(c.requires_python, int(c.requires_python) + 1)
+ try:
+ SpecifierSet(c.requires_python)
+ specifierset = SpecifierSet(c.requires_python)
+ except InvalidSpecifier:
+ continue
+ else:
+ if not specifierset.contains(py_version):
+ continue
+ all_candidates.append(c)
+ return all_candidates
+
+
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 +60,51 @@ def comment(text):
@@ -43,16 +65,51 @@ def comment(text):
return style(text, fg='green')


Expand Down Expand Up @@ -632,7 +637,7 @@ index fde5816..5827a55 100644


def format_requirement(ireq, marker=None):
@@ -63,10 +115,10 @@ def format_requirement(ireq, marker=None):
@@ -63,10 +120,10 @@ def format_requirement(ireq, marker=None):
if ireq.editable:
line = '-e {}'.format(ireq.link)
else:
Expand Down
17 changes: 16 additions & 1 deletion tests/integration/test_lock.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import pytest
import os
import six

from pipenv.utils import temp_environ

Expand Down Expand Up @@ -331,7 +332,7 @@ def test_lock_updated_source(PipenvInstance, pypi):
@pytest.mark.lock
@pytest.mark.vcs
@pytest.mark.needs_internet
def lock_editable_vcs_without_install(PipenvInstance, pypi):
def test_lock_editable_vcs_without_install(PipenvInstance, pypi):
with PipenvInstance(pypi=pypi, chdir=True) as p:
with open(p.pipfile_path, 'w') as f:
f.write("""
Expand All @@ -345,3 +346,17 @@ def lock_editable_vcs_without_install(PipenvInstance, pypi):
assert 'chardet' in p.lockfile['default']
c = p.pipenv('install')
assert c.return_code == 0


@pytest.mark.lock
def test_lock_respecting_python_version(PipenvInstance):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pytest.mark.needs_internet

with PipenvInstance() as p:
with open(p.pipfile_path, 'w') as f:
f.write("""
[packages]
django = "*"
""".strip())
django_version = '==2.0.6' if six.PY3 else '==1.11.10'
c = p.pipenv('lock')
assert c.return_code == 0
assert p.lockfile['default']['django']['version'] == django_version
Copy link
Contributor

@frostming frostming Jun 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For internet PyPI, should update this to test if django_version[:3]=='==2'(PY3) and django_version[:3]=='==1'(PY2)

Binary file added tests/pypi/django/Django-2.0.6.tar.gz
Binary file not shown.