Merge pull request #3251 from pypa/bugfix/3239

Redirect all spinner output to stderr

news/3239.bugfix.rst

@@ -0,0 +1 @@
+Fixed a bug which caused spinner frames to be written to stdout during locking operations which could cause redirection pipes to fail.

pipenv/core.py

@@ -906,16 +906,19 @@ def do_create_virtualenv(python=None, site_packages=False, pypi_mirror=None):
 
     # Actually create the virtualenv.
     nospin = environments.PIPENV_NOSPIN
-    c = vistir.misc.run(
-        cmd, verbose=False, return_object=True,
-        spinner_name=environments.PIPENV_SPINNER, combine_stderr=False,
-        block=False, nospin=nospin, env=pip_config,
-    )
-    click.echo(crayons.blue("{0}".format(c.out)), err=True)
-    if c.returncode != 0:
-        raise exceptions.VirtualenvCreationException(
-            extra=[crayons.blue("{0}".format(c.err)),]
+    with create_spinner("Creating virtual environment...") as sp:
+        c = vistir.misc.run(
+            cmd, verbose=False, return_object=True, write_to_stdout=False,
+            combine_stderr=False, block=True, nospin=True, env=pip_config,
         )
+        click.echo(crayons.blue("{0}".format(c.out)), err=True)
+        if c.returncode != 0:
+            sp.fail(environments.PIPENV_SPINNER_FAIL_TEXT.format("Failed creating virtual environment"))
+            raise exceptions.VirtualenvCreationException(
+                extra=[crayons.blue("{0}".format(c.err)),]
+            )
+        else:
+            sp.green.ok(environments.PIPENV_SPINNER_OK_TEXT.format("Successfully created virtual environment!"))
 
     # Associate project directory with the environment.
     # This mimics Pew's "setproject".

pipenv/environment.py

@@ -193,7 +193,7 @@ def sys_path(self):
         elif any([sys.prefix == self.prefix, not self.is_venv]):
             return sys.path
         cmd_args = [self.python, "-c", "import json, sys; print(json.dumps(sys.path))"]
-        path, _ = vistir.misc.run(cmd_args, return_object=False, nospin=True, block=True, combine_stderr=False)
+        path, _ = vistir.misc.run(cmd_args, return_object=False, nospin=True, block=True, combine_stderr=False, write_to_stdout=False)
         path = json.loads(path.strip())
         return path
 
@@ -206,7 +206,7 @@ def sys_prefix(self):
         """
 
         command = [self.python, "-c" "import sys; print(sys.prefix)"]
-        c = vistir.misc.run(command, return_object=True, block=True, nospin=True)
+        c = vistir.misc.run(command, return_object=True, block=True, nospin=True, write_to_stdout=False)
         sys_prefix = vistir.compat.Path(vistir.misc.to_text(c.out).strip()).as_posix()
         return sys_prefix
 
@@ -412,7 +412,7 @@ def run(self, cmd, cwd=os.curdir):
         c = None
         with self.activated():
             script = vistir.cmdparse.Script.parse(cmd)
-            c = vistir.misc.run(script._parts, return_object=True, nospin=True, cwd=cwd)
+            c = vistir.misc.run(script._parts, return_object=True, nospin=True, cwd=cwd, write_to_stdout=False)
         return c
 
     def run_py(self, cmd, cwd=os.curdir):
@@ -431,7 +431,7 @@ def run_py(self, cmd, cwd=os.curdir):
         else:
             script = vistir.cmdparse.Script.parse([self.python, "-c"] + list(cmd))
         with self.activated():
-            c = vistir.misc.run(script._parts, return_object=True, nospin=True, cwd=cwd)
+            c = vistir.misc.run(script._parts, return_object=True, nospin=True, cwd=cwd, write_to_stdout=False)
         return c
 
     def run_activate_this(self):

pipenv/utils.py

@@ -482,7 +482,7 @@ def create_spinner(text, nospin=None, spinner_name=None):
     with vistir.spin.create_spinner(
             spinner_name=spinner_name,
             start_text=vistir.compat.fs_str(text),
-            nospin=nospin
+            nospin=nospin, write_to_stdout=False
     ) as sp:
         yield sp
 

pipenv/vendor/vistir/contextmanagers.py

@@ -103,13 +103,14 @@ def write(self, text):
 
 
 @contextmanager
-def spinner(spinner_name=None, start_text=None, handler_map=None, nospin=False):
+def spinner(spinner_name=None, start_text=None, handler_map=None, nospin=False, write_to_stdout=True):
     """Get a spinner object or a dummy spinner to wrap a context.
 
     :param str spinner_name: A spinner type e.g. "dots" or "bouncingBar" (default: {"bouncingBar"})
     :param str start_text: Text to start off the spinner with (default: {None})
     :param dict handler_map: Handler map for signals to be handled gracefully (default: {None})
     :param bool nospin: If true, use the dummy spinner (default: {False})
+    :param bool write_to_stdout: Writes to stdout if true, otherwise writes to stderr (default: True)
     :return: A spinner object which can be manipulated while alive
     :rtype: :class:`~vistir.spin.VistirSpinner`
 
@@ -136,14 +137,15 @@ def spinner(spinner_name=None, start_text=None, handler_map=None, nospin=False):
     use_yaspin = (has_yaspin is False) or (nospin is True)
     if has_yaspin is None or has_yaspin is True and not nospin:
         use_yaspin = True
-    if not start_text and nospin is False:
+    if start_text is None and use_yaspin is True:
         start_text = "Running..."
     with create_spinner(
         spinner_name=spinner_name,
         text=start_text,
         handler_map=handler_map,
         nospin=nospin,
-        use_yaspin=use_yaspin
+        use_yaspin=use_yaspin,
+        write_to_stdout=write_to_stdout
     ) as _spinner:
         yield _spinner
 

pipenv/vendor/vistir/misc.py

@@ -106,7 +106,7 @@ def _spawn_subprocess(script, env=None, block=True, cwd=None, combine_stderr=Tru
     from distutils.spawn import find_executable
 
     if not env:
-        env = {}
+        env = os.environ.copy()
     command = find_executable(script.command)
     options = {
         "env": env,
@@ -138,6 +138,7 @@ def _spawn_subprocess(script, env=None, block=True, cwd=None, combine_stderr=Tru
     return subprocess.Popen(script.cmdify(), **options)
 
 
+
 def _create_subprocess(
     cmd,
     env=None,
@@ -148,15 +149,16 @@ def _create_subprocess(
     spinner=None,
     combine_stderr=False,
     display_limit=200,
-    start_text=""
+    start_text="",
+    write_to_stdout=True
 ):
     if not env:
         env = {}
     try:
         c = _spawn_subprocess(cmd, env=env, block=block, cwd=cwd,
-                                                    combine_stderr=combine_stderr)
+                              combine_stderr=combine_stderr)
     except Exception as exc:
-        print("Error %s while executing command %s", exc, " ".join(cmd._parts))
+        sys.stderr.write("Error %s while executing command %s", exc, " ".join(cmd._parts))
         raise
     if not block:
         c.stdin.close()
@@ -193,9 +195,7 @@ def _create_subprocess(
                 err_line = fs_str("{0}".format(stderr_line))
                 if verbose and err_line is not None:
                     if spinner:
-                        spinner._hide_cursor()
-                        spinner.write_err(err_line)
-                        spinner._show_cursor()
+                        spinner.hide_and_write(err_line, target=spinner.stderr)
                     else:
                         sys.stderr.write(err_line)
                         sys.stderr.flush()
@@ -206,12 +206,12 @@ def _create_subprocess(
                     display_line = "{0}...".format(stdout_line[:display_limit])
                 if verbose and display_line is not None:
                     if spinner:
-                        spinner._hide_cursor()
-                        spinner.write_err(display_line)
-                        spinner._show_cursor()
+                        target = spinner.stdout if write_to_stdout else spinner.stderr
+                        spinner.hide_and_write(display_line, target=target)
                     else:
-                        sys.stderr.write(display_line)
-                        sys.stderr.flush()
+                        target = sys.stdout if write_to_stdout else sys.stderr
+                        target.write(display_line)
+                        target.flush()
                 if spinner:
                     spinner.text = to_native_string("{0} {1}".format(spinner_orig_text, display_line))
                 continue
@@ -252,7 +252,8 @@ def run(
     nospin=False,
     spinner_name=None,
     combine_stderr=True,
-    display_limit=200
+    display_limit=200,
+    write_to_stdout=True
 ):
     """Use `subprocess.Popen` to get the output of a command and decode it.
 
@@ -266,6 +267,7 @@ def run(
     :param str spinner_name: The name of the spinner to use if enabled, defaults to bouncingBar
     :param bool combine_stderr: Optionally merge stdout and stderr in the subprocess, false if nonblocking.
     :param int dispay_limit: The max width of output lines to display when using a spinner.
+    :param bool write_to_stdout: Whether to write to stdout when using a spinner, default True.
     :returns: A 2-tuple of (output, error) or a :class:`subprocess.Popen` object.
 
     .. Warning:: Merging standard out and standarad error in a nonblocking subprocess
@@ -296,7 +298,8 @@ def run(
     if block or not return_object:
         combine_stderr = False
     start_text = ""
-    with spinner(spinner_name=spinner_name, start_text=start_text, nospin=nospin) as sp:
+    with spinner(spinner_name=spinner_name, start_text=start_text, nospin=nospin,
+                 write_to_stdout=write_to_stdout) as sp:
         return _create_subprocess(
             cmd,
             env=_env,
@@ -306,10 +309,12 @@ def run(
             verbose=verbose,
             spinner=sp,
             combine_stderr=combine_stderr,
-            start_text=start_text
+            start_text=start_text,
+            write_to_stdout=True
         )
 
 
+
 def load_path(python):
     """Load the :mod:`sys.path` from the given python executable's environment as json