Parse and include extras from vcs and non-vcs urls

- Include extras when rebuilding urls from pipfiles - Fixes #1997, #2128 Signed-off-by: Dan Ryan <[email protected]>
pypa · May 4, 2018 · f0c3336 · f0c3336
1 parent 69526d0
commit f0c3336
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 19 deletions.
diff --git 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 ''
+        extras = ''
         version = ''
         index = ''
         # Get rid of '*'.
@@ -675,7 +676,7 @@ def convert_deps_to_pip(deps, project=None, r=True, include_index=False):
             )
         # Support for extras (e.g. requests[socks])
         if 'extras' in deps[dep]:
-            extra = '[{0}]'.format(','.join(deps[dep]['extras']))
+            extras = '[{0}]'.format(','.join(deps[dep]['extras']))
         if 'version' in deps[dep]:
             if not is_star(deps[dep]['version']):
                 version = deps[dep]['version']
@@ -709,17 +710,22 @@ def convert_deps_to_pip(deps, project=None, r=True, include_index=False):
         # Support for version control
         maybe_vcs = [vcs for vcs in VCS_LIST if vcs in deps[dep]]
         vcs = maybe_vcs[0] if maybe_vcs else None
+        if not any(key in deps[dep] for key in ['path', 'vcs', 'file']):
+            extra += extras
         # Support for files.
         if 'file' in deps[dep]:
-            extra = '{1}{0}'.format(extra, deps[dep]['file']).strip()
+            dep_file = deps[dep]['file']
+            if is_valid_url(dep_file) and dep_file.startswith('http'):
+                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 = ''
         # Support for paths.
         elif 'path' in deps[dep]:
-            extra = '{1}{0}'.format(extra, deps[dep]['path']).strip()
+            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 '
@@ -730,7 +736,7 @@ def convert_deps_to_pip(deps, project=None, r=True, include_index=False):
             # Support for @refs.
             if 'ref' in deps[dep]:
                 extra += '@{0}'.format(deps[dep]['ref'])
-            extra += '#egg={0}'.format(dep)
+            extra += '#egg={0}{1}'.format(dep, extras)
             # Support for subdirectory
             if 'subdirectory' in deps[dep]:
                 extra += '&subdirectory={0}'.format(deps[dep]['subdirectory'])
@@ -740,6 +746,7 @@ def convert_deps_to_pip(deps, project=None, r=True, include_index=False):
                 dep = '-e '
             else:
                 dep = ''
+
         s = '{0}{1}{2}{3}{4} {5}'.format(
             dep, extra, version, specs, hash, index
         ).strip()

diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py
@@ -44,7 +44,21 @@
         }},
         '-e svn+svn://svn.myproject.org/svn/MyProject#egg=MyProject',
     ),
-
+    (
+        # Extras in url
+        {'discord.py': {
+                'file': 'https://github.com/Rapptz/discord.py/archive/rewrite.zip',
+                'extras': ['voice']
+        }},
+        'https://github.com/Rapptz/discord.py/archive/rewrite.zip#egg=discord.py[voice]',
+    ),
+    (
+        {'requests': {
+            'git': 'https://github.com/requests/requests.git',
+            'ref': 'master', 'extras': ['security'],
+        }},
+        'git+https://github.com/requests/requests.git@master#egg=requests[security]',
+    ),
 ]
 
 
@@ -97,20 +111,6 @@ def test_convert_from_pip(expected, requirement):
     assert pipenv.utils.convert_deps_from_pip(requirement) == expected
 
 
-@pytest.mark.utils
-@pytest.mark.parametrize('expected, requirement', [
-    (   # XXX: This should work the other way around as well, but does not atm.
-        {'requests': {
-            'git': 'https://github.com/requests/requests.git',
-            'ref': 'master', 'extras': ['security'],
-        }},
-        'git+https://github.com/requests/requests.git@master#egg=requests[security]',
-    ),
-])
-def test_convert_from_pip_vcs_with_extra(expected, requirement):
-    assert pipenv.utils.convert_deps_from_pip(requirement) == expected
-
-
 @pytest.mark.utils
 def test_convert_from_pip_fail_if_no_egg():
     """Parsing should fail without `#egg=`.