Fix sequential sync losing track of pip install processes (#3537)

Fix sequential sync losing track of pip install processes

Co-authored-by: Dan Ryan <[email protected]>

Author: techalchemy

news/3537.bugfix.rst

@@ -0,0 +1 @@
+Fix ``sync --sequential`` ignoring ``pip install`` errors and logs.

pipenv/core.py

@@ -645,10 +645,10 @@ def do_where(virtualenv=False, bare=True):
             click.echo(location)
 
 
-def _cleanup_procs(procs, concurrent, failed_deps_queue, retry=True):
+def _cleanup_procs(procs, failed_deps_queue, retry=True):
     while not procs.empty():
         c = procs.get()
-        if concurrent:
+        if not c.blocking:
             c.block()
         failed = False
         if c.return_code != 0:
@@ -682,7 +682,7 @@ def _cleanup_procs(procs, concurrent, failed_deps_queue, retry=True):
 def batch_install(deps_list, procs, failed_deps_queue,
                   requirements_dir, no_deps=False, ignore_hashes=False,
                   allow_global=False, blocking=False, pypi_mirror=None,
-                  nprocs=PIPENV_MAX_SUBPROCESS, retry=True):
+                  retry=True):
     from .vendor.requirementslib.models.utils import strip_extras_markers_from_requirement
     failed = (not retry)
     if not failed:
@@ -753,12 +753,13 @@ def batch_install(deps_list, procs, failed_deps_queue,
                 extra_indexes=extra_indexes,
                 use_pep517=not failed,
             )
-            if procs.qsize() < nprocs:
-                c.dep = dep
-                procs.put(c)
+            c.dep = dep
+            if dep.is_vcs or dep.editable:
+                c.block()
 
+            procs.put(c)
             if procs.full() or procs.qsize() == len(deps_list):
-                _cleanup_procs(procs, not blocking, failed_deps_queue, retry=retry)
+                _cleanup_procs(procs, failed_deps_queue, retry=retry)
 
 
 def do_install_dependencies(
@@ -782,7 +783,6 @@ def do_install_dependencies(
     from six.moves import queue
     if requirements:
         bare = True
-    blocking = not concurrent
     # Load the lockfile if it exists, or if only is being used (e.g. lock is being used).
     if skip_lock or only or not project.lockfile_exists:
         if not bare:
@@ -818,27 +818,27 @@ def do_install_dependencies(
         )
         sys.exit(0)
 
-    procs = queue.Queue(maxsize=PIPENV_MAX_SUBPROCESS)
+    if concurrent:
+        nprocs = PIPENV_MAX_SUBPROCESS
+    else:
+        nprocs = 1
+    procs = queue.Queue(maxsize=nprocs)
     failed_deps_queue = queue.Queue()
     if skip_lock:
         ignore_hashes = True
 
     install_kwargs = {
         "no_deps": no_deps, "ignore_hashes": ignore_hashes, "allow_global": allow_global,
-        "blocking": blocking, "pypi_mirror": pypi_mirror
+        "blocking": not concurrent, "pypi_mirror": pypi_mirror
     }
-    if concurrent:
-        install_kwargs["nprocs"] = PIPENV_MAX_SUBPROCESS
-    else:
-        install_kwargs["nprocs"] = 1
 
     # with project.environment.activated():
     batch_install(
         deps_list, procs, failed_deps_queue, requirements_dir, **install_kwargs
     )
 
     if not procs.empty():
-        _cleanup_procs(procs, concurrent, failed_deps_queue)
+        _cleanup_procs(procs, failed_deps_queue)
 
     # Iterate over the hopefully-poorly-packaged dependencies…
     if not failed_deps_queue.empty():
@@ -850,15 +850,14 @@ def do_install_dependencies(
             failed_dep = failed_deps_queue.get()
             retry_list.append(failed_dep)
         install_kwargs.update({
-            "nprocs": 1,
             "retry": False,
             "blocking": True,
         })
         batch_install(
             retry_list, procs, failed_deps_queue, requirements_dir, **install_kwargs
         )
     if not procs.empty():
-        _cleanup_procs(procs, False, failed_deps_queue, retry=False)
+        _cleanup_procs(procs, failed_deps_queue, retry=False)
 
 
 def convert_three_to_python(three, python):
@@ -2507,6 +2506,8 @@ def do_run(command, args, three=None, python=False, pypi_mirror=None):
     previous_pipenv_active_value = os.environ.get("PIPENV_ACTIVE")
     os.environ["PIPENV_ACTIVE"] = vistir.misc.fs_str("1")
 
+    os.environ.pop("PIP_SHIMS_BASE_MODULE", None)
+
     try:
         script = project.build_script(command, args)
         cmd_string = ' '.join([script.command] + script.args)

tests/integration/test_sync.py

@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 from __future__ import absolute_import, print_function
+import json
 import os
 
 import pytest
@@ -68,3 +69,46 @@ def test_sync_should_not_lock(PipenvInstance, pypi):
         c = p.pipenv('sync')
         assert c.return_code == 0
         assert lockfile_content == p.lockfile
+
+
+@pytest.mark.sync
+@pytest.mark.lock
+def test_sync_sequential_detect_errors(PipenvInstance, pypi):
+    with PipenvInstance(pypi=pypi) as p:
+        with open(p.pipfile_path, 'w') as f:
+            contents = """
+[packages]
+requests = "*"
+        """.strip()
+            f.write(contents)
+
+        c = p.pipenv('lock')
+        assert c.return_code == 0
+
+        # Force hash mismatch when installing `requests`
+        lock = p.lockfile
+        lock['default']['requests']['hashes'] = ['sha256:' + '0' * 64]
+        with open(p.lockfile_path, 'w') as f:
+            json.dump(lock, f)
+
+        c = p.pipenv('sync --sequential')
+        assert c.return_code != 0
+
+
+@pytest.mark.sync
+@pytest.mark.lock
+def test_sync_sequential_verbose(PipenvInstance, pypi):
+    with PipenvInstance(pypi=pypi) as p:
+        with open(p.pipfile_path, 'w') as f:
+            contents = """
+[packages]
+requests = "*"
+        """.strip()
+            f.write(contents)
+
+        c = p.pipenv('lock')
+        assert c.return_code == 0
+
+        c = p.pipenv('sync --sequential --verbose')
+        for package in p.lockfile['default']:
+            assert 'Successfully installed {}'.format(package) in c.out