From fcfebaf1ccf46824af70e400712c4ce1e6897b11 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Thu, 3 May 2018 21:42:26 -0400 Subject: [PATCH 1/3] Only use `-e` for install when `editable=True` - Fixes #2121 - Add tests for regressions Signed-off-by: Dan Ryan --- pipenv/utils.py | 19 ++++++------------- tests/unit/test_utils.py | 4 ++++ 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/pipenv/utils.py b/pipenv/utils.py index e5a1cb87..54403c60 100644 --- a/pipenv/utils.py +++ b/pipenv/utils.py @@ -657,6 +657,7 @@ def convert_deps_to_pip(deps, project=None, r=True, include_index=False): for dep in deps.keys(): # Default (e.g. '>1.10'). extra = deps[dep] if isinstance(deps[dep], six.string_types) else '' + editable = False extras = '' version = '' index = '' @@ -712,6 +713,8 @@ def convert_deps_to_pip(deps, project=None, r=True, include_index=False): vcs = maybe_vcs[0] if maybe_vcs else None if not any(key in deps[dep] for key in ['path', 'vcs', 'file']): extra += extras + if not isinstance(deps[dep], six.string_types): + editable = str(deps[dep].get('editable', '')).lower() == 'true' # Support for files. if 'file' in deps[dep]: dep_file = deps[dep]['file'] @@ -719,18 +722,12 @@ def convert_deps_to_pip(deps, project=None, r=True, include_index=False): dep_file += '#egg={0}'.format(dep) extra = '{0}{1}'.format(dep_file, extras).strip() # Flag the file as editable if it is a local relative path - if 'editable' in deps[dep]: - dep = '-e ' - else: - dep = '' + dep = '-e ' if editable else '' # Support for paths. elif 'path' in deps[dep]: extra = '{1}{0}'.format(extras, deps[dep]['path']).strip() # Flag the file as editable if it is a local relative path - if 'editable' in deps[dep]: - dep = '-e ' - else: - dep = '' + dep = '-e ' if editable else '' if vcs: extra = '{0}+{1}'.format(vcs, deps[dep][vcs]) # Support for @refs. @@ -741,11 +738,7 @@ def convert_deps_to_pip(deps, project=None, r=True, include_index=False): if 'subdirectory' in deps[dep]: extra += '&subdirectory={0}'.format(deps[dep]['subdirectory']) # Support for editable. - if 'editable' in deps[dep]: - # Support for --egg. - dep = '-e ' - else: - dep = '' + dep = '-e ' if editable else '' s = '{0}{1}{2}{3}{4} {5}'.format( dep, extra, version, specs, hash, index diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 1eedcccc..919e5359 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -56,6 +56,7 @@ DEP_PIP_PAIRS = [ {'requests': { 'git': 'https://github.com/requests/requests.git', 'ref': 'master', 'extras': ['security'], + 'editable': 'false' }}, 'git+https://github.com/requests/requests.git@master#egg=requests[security]', ), @@ -108,6 +109,9 @@ def test_convert_deps_to_pip_unicode(): @pytest.mark.utils @pytest.mark.parametrize('expected, requirement', DEP_PIP_PAIRS) def test_convert_from_pip(expected, requirement): + # We don't build requirements back up with the editable key, so lets drop it out + if 'requests' in expected and expected['requests'].get('editable', '') == 'false': + del expected['requests']['editable'] assert pipenv.utils.convert_deps_from_pip(requirement) == expected From 08f61e7684eb9fe3cfe192f367bbfd8c69179af3 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Fri, 4 May 2018 15:53:56 -0400 Subject: [PATCH 2/3] Switch to checks for Mapping type and proper bool Signed-off-by: Dan Ryan --- pipenv/utils.py | 8 ++++++-- tests/unit/test_utils.py | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/pipenv/utils.py b/pipenv/utils.py index 54403c60..70149ff1 100644 --- a/pipenv/utils.py +++ b/pipenv/utils.py @@ -564,6 +564,10 @@ def multi_split(s, split): def convert_deps_from_pip(dep): """"Converts a pip-formatted dependency to a Pipfile-formatted one.""" + try: + from collections.abc import Mapping + except ImportError: + from collections import Mapping dependency = {} req = get_requirement(dep) extras = {'extras': req.extras} @@ -713,8 +717,8 @@ def convert_deps_to_pip(deps, project=None, r=True, include_index=False): vcs = maybe_vcs[0] if maybe_vcs else None if not any(key in deps[dep] for key in ['path', 'vcs', 'file']): extra += extras - if not isinstance(deps[dep], six.string_types): - editable = str(deps[dep].get('editable', '')).lower() == 'true' + if not isinstance(deps[dep], Mapping): + editable = bool(deps[dep].get('editable', False)) # Support for files. if 'file' in deps[dep]: dep_file = deps[dep]['file'] diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 919e5359..855a78cb 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -56,7 +56,7 @@ DEP_PIP_PAIRS = [ {'requests': { 'git': 'https://github.com/requests/requests.git', 'ref': 'master', 'extras': ['security'], - 'editable': 'false' + 'editable': False }}, 'git+https://github.com/requests/requests.git@master#egg=requests[security]', ), From f6cff554f44e677911e4d2aef0d95db804f29486 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Fri, 4 May 2018 16:01:16 -0400 Subject: [PATCH 3/3] update tests to remove False key during comparison Signed-off-by: Dan Ryan --- pipenv/utils.py | 2 +- tests/unit/test_utils.py | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/pipenv/utils.py b/pipenv/utils.py index 70149ff1..a63089ed 100644 --- a/pipenv/utils.py +++ b/pipenv/utils.py @@ -717,7 +717,7 @@ def convert_deps_to_pip(deps, project=None, r=True, include_index=False): vcs = maybe_vcs[0] if maybe_vcs else None if not any(key in deps[dep] for key in ['path', 'vcs', 'file']): extra += extras - if not isinstance(deps[dep], Mapping): + if isinstance(deps[dep], Mapping): editable = bool(deps[dep].get('editable', False)) # Support for files. if 'file' in deps[dep]: diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 855a78cb..f314b7e9 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -2,7 +2,7 @@ import os import pytest from mock import patch, Mock - +from first import first import pipenv.utils @@ -110,8 +110,9 @@ def test_convert_deps_to_pip_unicode(): @pytest.mark.parametrize('expected, requirement', DEP_PIP_PAIRS) def test_convert_from_pip(expected, requirement): # We don't build requirements back up with the editable key, so lets drop it out - if 'requests' in expected and expected['requests'].get('editable', '') == 'false': - del expected['requests']['editable'] + package = first(expected.keys()) + if hasattr(expected[package], 'keys') and expected[package].get('editable') is False: + del expected[package]['editable'] assert pipenv.utils.convert_deps_from_pip(requirement) == expected