Repository: kennethreitz/pipenv (python)

Issue:
easy-install.pth contains incomplete number of paths for editable = true packages.
##### Describe your environment

1. OS Type: Debian Stable 9.2
1. Python version: Python 2.7.13
1. Pipenv version: pipenv, version 8.3.2

##### Expected result

When running pipenv install with a Pipfile containing 15 local editable packages:

```
pyramid-mustache = {path = "vendor/pyramid-mustache/", editable = true}
Dalton = {path = "vendor/Dalton/", editable = true}
pyelasticsearch = {path = "vendor/pyelasticsearch/", editable = true}
deus_lib = {path = "deus_lib/", editable = true}
lexdb = {path = "lexdb/", editable = true}
bias = {path = "bias/", editable = true}
winslow = {path = "winslow/", editable = true}
usertool = {path = "usertool/", editable = true}
analytics_services = {path = "analytics_services/", editable = true}
citadel = {path = "citadel/", editable = true}
lex-nlp-scripts = {path = "lexnlp-product/core/scripts/", editable = true}
dashboarding = {path = "dashboarding/", editable = true}
east = {path = "east/", editable = true}
webhook_handler = {path = "webhook_handler/", editable = true}
qatool = {path = "qatool/", editable = true}
```

I expect 15 entries in `easy-install.pth`. 

##### Actual result

I get 13, 11, etc, and each time you run it you get a different set of local pth entries.

No changes to Pipfile or to Pipfile.lock, just rerunning pipenv install

Examples:
```
/home/gavin/LexMachina/src/deus_lex/vendor/Dalton
/home/gavin/LexMachina/src/deus_lex/east
/home/gavin/LexMachina/src/deus_lex/vendor/pyramid-mustache
/home/gavin/LexMachina/src/deus_lex/citadel
/home/gavin/LexMachina/src/deus_lex/vendor/pyelasticsearch
/home/gavin/LexMachina/src/deus_lex/webhook_handler
/home/gavin/LexMachina/src/deus_lex/usertool
/home/gavin/LexMachina/src/deus_lex/analytics_services
/home/gavin/LexMachina/src/deus_lex/lexdb
/home/gavin/LexMachina/src/deus_lex/deus_lib
/home/gavin/LexMachina/src/deus_lex/winslow
/home/gavin/LexMachina/src/deus_lex/lexnlp-product/core/scripts
/home/gavin/LexMachina/src/deus_lex/qatool
```

```
/home/gavin/LexMachina/src/deus_lex/webhook_handler
/home/gavin/LexMachina/src/deus_lex/vendor/Dalton
/home/gavin/LexMachina/src/deus_lex/east
/home/gavin/LexMachina/src/deus_lex/dashboarding
/home/gavin/LexMachina/src/deus_lex/vendor/pyramid-mustache
/home/gavin/LexMachina/src/deus_lex/citadel
/home/gavin/LexMachina/src/deus_lex/bias
/home/gavin/LexMachina/src/deus_lex/usertool
/home/gavin/LexMachina/src/deus_lex/lexdb
/home/gavin/LexMachina/src/deus_lex/analytics_services
/home/gavin/LexMachina/src/deus_lex/winslow
/home/gavin/LexMachina/src/deus_lex/lexnlp-product/core/scripts
/home/gavin/LexMachina/src/deus_lex/qatool
/home/gavin/LexMachina/src/deus_lex/vendor/pyelasticsearch
```
[OUTPUT.txt](https://github.com/kennethreitz/pipenv/files/1497769/OUTPUT.txt)



##### Steps to replicate

Sadly I have no idea how to make a smaller test case as I have 0 idea what is going on. Happy to try and create one if someone can give me some hint as to how the heck this is even possible.

Pull request:
Do sequential installs on editable dependencies
- Fixes #1111 
- Uses existing infrastructure to add editable dependencies to the list
of sequentially installed packages
- Only alters functions used for this purpose
- Essentially there is a race condition because multiple editors are
trying to write to lib/site-packages/easy-install.pth for local editable
packages, so they overwrite one another

Patch:
diff --git a/pipenv/cli.py b/pipenv/cli.py
index 08d612d9..e76cbb65 100644
--- a/pipenv/cli.py
+++ b/pipenv/cli.py
@@ -33,7 +33,7 @@ from click_didyoumean import DYMCommandCollection
 from .project import Project
 from .utils import (
     convert_deps_from_pip, convert_deps_to_pip, is_required_version,
-    proper_case, pep423_name, split_vcs, resolve_deps, shellquote, is_vcs,
+    proper_case, pep423_name, split_file, merge_deps, resolve_deps, shellquote, is_vcs,
     python_version, suggest_package, find_windows_executable, is_file,
     prepare_pip_source_args, temp_environ, is_valid_url, download_file,
     get_requirement, need_update_check, touch_update_stamp
@@ -765,10 +765,10 @@ def do_install_dependencies(
     if skip_lock or only or not project.lockfile_exists:
         if not bare:
             click.echo(crayons.normal(u'Installing dependencies from Pipfile…', bold=True))
-            lockfile = split_vcs(project._lockfile)
+            lockfile = split_file(project._lockfile)
     else:
         with open(project.lockfile_location) as f:
-            lockfile = split_vcs(simplejson.load(f))
+            lockfile = split_file(simplejson.load(f))
 
         if not bare:
             click.echo(
@@ -783,42 +783,16 @@ def do_install_dependencies(
     # Allow pip to resolve dependencies when in skip-lock mode.
     no_deps = (not skip_lock)
 
-    deps = {}
-    vcs_deps = {}
-
-    # Store dev only deps for a requirements output
-    dev_deps = {}
-    dev_vcs_deps = {}
-
-    # Add development deps if --dev was passed.
-    if dev:
-        deps.update(lockfile['develop'])
-        vcs_deps.update(lockfile.get('develop-vcs', {}))
-
-        # Add only dev deps if requirements was passed
-        if requirements:
-            dev_deps.update(lockfile['develop'])
-            dev_vcs_deps.update(lockfile.get('develop-vcs', {}))
-
-
-    # Install default dependencies, always.
-    deps.update(lockfile['default'] if not only else {})
-    vcs_deps.update(lockfile.get('default-vcs', {}))
-
-    if ignore_hashes:
-        # Remove hashes from generated requirements.
-        for k, v in deps.items():
-            if 'hash' in v:
-                del v['hash']
-
-    # Convert the deps to pip-compatible arguments.
-    deps_list = [(d, ignore_hashes, blocking) for d in convert_deps_to_pip(deps, project, r=False, include_index=True)]
+    deps_list, dev_deps_list = merge_deps(
+        lockfile,
+        project,
+        dev=dev,
+        requirements=requirements,
+        ignore_hashes=ignore_hashes,
+        blocking=blocking,
+        only=only
+    )
     failed_deps_list = []
-
-    if len(vcs_deps):
-        deps_list.extend((d, True, True) for d in convert_deps_to_pip(vcs_deps, project, r=False))
-
-    # --requirements was passed.
     if requirements:
         # Output only default dependencies
         if not dev:
@@ -827,14 +801,9 @@ def do_install_dependencies(
 
         # Output only dev dependencies
         if dev:
-            dev_deps_list = [(d, ignore_hashes, blocking) for d in convert_deps_to_pip(dev_deps, project, r=False, include_index=True)]
-            if len(dev_vcs_deps):
-                dev_deps_list.extend((d, True, True) for d in convert_deps_to_pip(dev_vcs_deps, project, r=False))
-
             click.echo('\n'.join(d[0] for d in dev_deps_list))
             sys.exit(0)
 
-
     procs = []
 
     deps_list_bar = progress.bar(deps_list, label=INSTALL_LABEL if os.name != 'nt' else '')
diff --git a/pipenv/utils.py b/pipenv/utils.py
index e357ca18..c079523a 100644
--- a/pipenv/utils.py
+++ b/pipenv/utils.py
@@ -919,21 +919,100 @@ def proper_case(package_name):
     return good_name
 
 
-def split_vcs(split_file):
-    """Split VCS dependencies out from file."""
-
-    if 'packages' in split_file or 'dev-packages' in split_file:
-        sections = ('packages', 'dev-packages')
-    elif 'default' in split_file or 'develop' in split_file:
-        sections = ('default', 'develop')
+def split_section(input_file, section_suffix, test_function):
+    """
+    Split a pipfile or a lockfile section out by section name and test function
+    
+        :param dict input_file: A dictionary containing either a pipfile or lockfile
+        :param str section_suffix: A string of the name of the section
+        :param func test_function: A test function to test against the value in the key/value pair
+    
+    >>> split_section(my_lockfile, 'vcs', is_vcs)
+    {
+        'default': {
+            "six": {
+                "hashes": [
+                    "sha256:832dc0e10feb1aa2c68dcc57dbb658f1c7e65b9b61af69048abc87a2db00a0eb",
+                    "sha256:70e8a77beed4562e7f14fe23a786b54f6296e34344c23bc42f07b15018ff98e9"
+                ],
+                "version": "==1.11.0"
+            }
+        },
+        'default-vcs': {
+            "e1839a8": {
+                "editable": true,
+                "path": "."
+            }
+        }
+    }
+    """
+    pipfile_sections = ('packages', 'dev-packages')
+    lockfile_sections = ('default', 'develop')
+    if any(section in input_file for section in pipfile_sections):
+        sections = pipfile_sections
+    elif any(section in input_file for section in lockfile_sections):
+        sections = lockfile_sections
+    else:
+        # return the original file if we can't find any pipfile or lockfile sections
+        return input_file
 
-    # For each vcs entry in a given section, move it to section-vcs.
     for section in sections:
-        entries = split_file.get(section, {})
-        vcs_dict = dict((k, entries.pop(k)) for k in list(entries.keys()) if is_vcs(entries[k]))
-        split_file[section + '-vcs'] = vcs_dict
+        split_dict = {}
+        entries = input_file.get(section, {})
+        for k in list(entries.keys()):
+            if test_function(entries.get(k)):
+                split_dict[k] = entries.pop(k)
+        input_file['-'.join([section, section_suffix])] = split_dict
+    return input_file
+
+
+def split_file(file_dict):
+    """Split VCS and editable dependencies out from file."""
+    sections = {
+        'vcs': is_vcs,
+        'editable': lambda x: hasattr(x, 'keys') and x.get('editable')
+    }
+    for k, func in sections.items():
+        file_dict = split_section(file_dict, k, func)
+    return file_dict
 
-    return split_file
+
+def merge_deps(file_dict, project, dev=False, requirements=False, ignore_hashes=False, blocking=False, only=False):
+    """
+    Given a file_dict, merges dependencies and converts them to pip dependency lists.
+        :param dict file_dict: The result of calling :func:`pipenv.utils.split_file`
+        :param :class:`pipenv.project.Project` project: Pipenv project
+        :param bool dev=False: Flag indicating whether dev dependencies are to be installed 
+        :param bool requirements=False: Flag indicating whether to use a requirements file
+        :param bool ignore_hashes=False:
+        :param bool blocking=False:
+        :param bool only=False:
+        :return: Pip-converted 3-tuples of [deps, requirements_deps]
+    """
+    deps = []
+    requirements_deps = []
+
+    for section in list(file_dict.keys()):
+        # Turn develop-vcs into ['develop', 'vcs']
+        section_name, suffix = section.rsplit('-', 1) if '-' in section and not section == 'dev-packages' else (section, None)
+        if not file_dict[section] or section_name not in ('dev-packages', 'packages', 'default', 'develop'):
+            continue
+        is_dev = section_name in ('dev-packages', 'develop')
+
+        if ignore_hashes:
+            for k, v in file_dict[section]:
+                if 'hash' in v:
+                    del v['hash']
+
+        # Block and ignore hashes for all suffixed sections (vcs/editable)
+        no_hashes = True if suffix else ignore_hashes
+        block = True if suffix else blocking
+        include_index = True if not suffix else False
+        converted = convert_deps_to_pip(file_dict[section], project, r=False, include_index=include_index)
+        deps.extend((d, no_hashes, block) for d in converted)
+        if dev and is_dev and requirements:
+            requirements_deps.extend((d, no_hashes, block) for d in converted)
+    return deps, requirements_deps
 
 
 def recase_file(file_dict):


Test patch:
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 303c45aa..ffb875ef 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -131,11 +131,12 @@ class TestUtils:
     def test_is_vcs(self, entry, expected):
         assert pipenv.utils.is_vcs(entry) is expected
 
-    def test_split_vcs(self):
+    def test_split_file(self):
         pipfile_dict = {
             'packages': {
                 'requests': {'git': 'https://github.com/kennethreitz/requests.git'},
-                'Flask': '*'
+                'Flask': '*',
+                'tablib': {'path': '.', 'editable': True}
             },
             'dev-packages': {
                 'Django': '==1.10',
@@ -143,10 +144,11 @@ class TestUtils:
                 'crayons': {'hg': 'https://hg.alsonotreal.com/crayons'}
             }
         }
-        split_dict = pipenv.utils.split_vcs(pipfile_dict)
+        split_dict = pipenv.utils.split_file(pipfile_dict)
 
         assert list(split_dict['packages'].keys()) == ['Flask']
         assert split_dict['packages-vcs'] == {'requests': {'git': 'https://github.com/kennethreitz/requests.git'}}
+        assert split_dict['packages-editable'] == {'tablib': {'path': '.', 'editable': True}}
         assert list(split_dict['dev-packages'].keys()) == ['Django']
         assert 'click' in split_dict['dev-packages-vcs']
         assert 'crayons' in split_dict['dev-packages-vcs']