From 589b1a4f20c0c38b30135c3cbb3905228c72062b Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Fri, 16 Nov 2018 23:17:32 -0500 Subject: [PATCH 1/3] Redirect all spinner output to stderr - Fixes #3239 Signed-off-by: Dan Ryan --- news/3239.bugfix.rst | 1 + pipenv/utils.py | 2 +- pipenv/vendor/vistir/contextmanagers.py | 10 +- pipenv/vendor/vistir/misc.py | 7 +- pipenv/vendor/vistir/spin.py | 139 +++++++++++++++++++----- 5 files changed, 126 insertions(+), 33 deletions(-) create mode 100644 news/3239.bugfix.rst diff --git a/news/3239.bugfix.rst b/news/3239.bugfix.rst new file mode 100644 index 0000000000..526e80e12a --- /dev/null +++ b/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. diff --git a/pipenv/utils.py b/pipenv/utils.py index 9f62e2b234..22837d264e 100644 --- a/pipenv/utils.py +++ b/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 diff --git a/pipenv/vendor/vistir/contextmanagers.py b/pipenv/vendor/vistir/contextmanagers.py index 0920a9c3dd..32fad7ca4d 100644 --- a/pipenv/vendor/vistir/contextmanagers.py +++ b/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,17 @@ 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 not start_text and use_yaspin is True: start_text = "Running..." + else: + start_text = "" 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 diff --git a/pipenv/vendor/vistir/misc.py b/pipenv/vendor/vistir/misc.py index a9a127d81a..870fa38bb4 100644 --- a/pipenv/vendor/vistir/misc.py +++ b/pipenv/vendor/vistir/misc.py @@ -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, diff --git a/pipenv/vendor/vistir/spin.py b/pipenv/vendor/vistir/spin.py index e7311555f7..acb428a535 100644 --- a/pipenv/vendor/vistir/spin.py +++ b/pipenv/vendor/vistir/spin.py @@ -4,6 +4,8 @@ import os import signal import sys +import threading +import time import colorama import cursor @@ -34,14 +36,18 @@ class DummySpinner(object): def __init__(self, text="", **kwargs): colorama.init() from .misc import decode_for_output - self.text = to_native_string(decode_for_output(text)) + self.text = to_native_string(decode_for_output(text)) if text else "" self.stdout = kwargs.get("stdout", sys.stdout) self.stderr = kwargs.get("stderr", sys.stderr) self.out_buff = StringIO() + self.write_to_stdout = kwargs.get("write_to_stdout", False) def __enter__(self): if self.text and self.text != "None": - self.write_err(self.text) + if self.write_to_stdout: + self.write(self.text) + else: + self.write_err(self.text) return self def __exit__(self, exc_type, exc_val, traceback): @@ -72,16 +78,24 @@ def _close_output_buffer(self): def fail(self, exitcode=1, text="FAIL"): from .misc import decode_for_output if text and text != "None": - self.write_err(decode_for_output(text)) + if self.write_to_stdout: + self.write(decode_for_output(text)) + else: + self.write_err(decode_for_output(text)) self._close_output_buffer() def ok(self, text="OK"): if text and text != "None": - self.stderr.write(self.text) + if self.write_to_stdout: + self.stdout.write(self.text) + else: + self.stderr.write(self.text) self._close_output_buffer() return 0 def write(self, text=None): + if not self.write_to_stdout: + return self.write_err(text) from .misc import decode_for_output if text is None or isinstance(text, six.string_types) and text == "None": pass @@ -102,11 +116,11 @@ def write_err(self, text=None): self.stderr.write(CLEAR_LINE) @staticmethod - def _hide_cursor(): + def _hide_cursor(target=None): pass @staticmethod - def _show_cursor(): + def _show_cursor(target=None): pass @@ -114,14 +128,18 @@ def _show_cursor(): class VistirSpinner(base_obj): + "A spinner class for handling spinners on windows and posix." + def __init__(self, *args, **kwargs): - """Get a spinner object or a dummy spinner to wrap a context. + """ + Get a spinner object or a dummy spinner to wrap a context. Keyword Arguments: :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) """ self.handler = handler @@ -145,36 +163,42 @@ def __init__(self, *args, **kwargs): kwargs["text"] = start_text if start_text is not None else _text kwargs["sigmap"] = sigmap kwargs["spinner"] = getattr(Spinners, spinner_name, "") + write_to_stdout = kwargs.pop("write_to_stdout", True) self.stdout = kwargs.pop("stdout", sys.stdout) self.stderr = kwargs.pop("stderr", sys.stderr) self.out_buff = StringIO() - super(VistirSpinner, self).__init__(*args, **kwargs) + self.write_to_stdout = write_to_stdout self.is_dummy = bool(yaspin is None) + super(VistirSpinner, self).__init__(*args, **kwargs) - def ok(self, text="OK"): + def ok(self, text="OK", err=False): """Set Ok (success) finalizer to a spinner.""" # Do not display spin text for ok state self._text = None _text = text if text else "OK" - self._freeze(_text) + err = err or not self.write_to_stdout + self._freeze(_text, err=err) - def fail(self, text="FAIL"): + def fail(self, text="FAIL", err=False): """Set fail finalizer to a spinner.""" # Do not display spin text for fail state self._text = None _text = text if text else "FAIL" - self._freeze(_text) + err = err or not self.write_to_stdout + self._freeze(_text, err=err) def write(self, text): + if not self.write_to_stdout: + return self.write_err(text) from .misc import to_text sys.stdout.write("\r") self.stdout.write(CLEAR_LINE) if text is None: text = "" text = to_native_string("{0}\n".format(text)) - sys.stdout.write(text) + self.stdout.write(text) self.out_buff.write(to_text(text)) def write_err(self, text): @@ -189,7 +213,46 @@ def write_err(self, text): self.stderr.write(text) self.out_buff.write(to_text(text)) - def _freeze(self, final_text): + def start(self): + if self._sigmap: + self._register_signal_handlers() + + target = self.stdout if self.write_to_stdout else self.stderr + if target.isatty(): + self._hide_cursor(target=target) + + self._stop_spin = threading.Event() + self._hide_spin = threading.Event() + self._spin_thread = threading.Thread(target=self._spin) + self._spin_thread.start() + + def stop(self): + if self._dfl_sigmap: + # Reset registered signal handlers to default ones + self._reset_signal_handlers() + + if self._spin_thread: + self._stop_spin.set() + self._spin_thread.join() + + target = self.stdout if self.write_to_stdout else self.stderr + if target.isatty(): + target.write("\r") + + if self.write_to_stdout: + self._clear_line() + else: + self._clear_err() + + if target.isatty(): + self._show_cursor(target=target) + if self.stderr and self.stderr != sys.stderr: + self.stderr.close() + if self.stdout and self.stdout != sys.stdout: + self.stdout.close() + self.out_buff.close() + + def _freeze(self, final_text, err=False): """Stop spinner, compose last frame and 'freeze' it.""" if not final_text: final_text = "" @@ -199,15 +262,10 @@ def _freeze(self, final_text): # Should be stopped here, otherwise prints after # self._freeze call will mess up the spinner self.stop() - self.stdout.write(self._last_frame) - - def stop(self, *args, **kwargs): - if self.stderr and self.stderr != sys.stderr: - self.stderr.close() - if self.stdout and self.stdout != sys.stdout: - self.stdout.close() - self.out_buff.close() - super(VistirSpinner, self).stop(*args, **kwargs) + if err or not self.write_to_stdout: + self.stderr.write(self._last_frame) + else: + self.stdout.write(self._last_frame) def _compose_color_func(self): fn = functools.partial( @@ -236,6 +294,29 @@ def _compose_out(self, frame, mode=None): out = to_native_string("{0} {1}\n".format(frame, text)) return out + def _spin(self): + target = self.stdout if self.write_to_stdout else self.stderr + clear_fn = self._clear_line if self.write_to_stdout else self._clear_err + while not self._stop_spin.is_set(): + + if self._hide_spin.is_set(): + # Wait a bit to avoid wasting cycles + time.sleep(self._interval) + continue + + # Compose output + spin_phase = next(self._cycle) + out = self._compose_out(spin_phase) + + # Write + target.write(out) + clear_fn() + target.flush() + + # Wait + time.sleep(self._interval) + target.write("\b") + def _register_signal_handlers(self): # SIGKILL cannot be caught or ignored, and the receiving # process cannot perform any clean-up upon receiving this @@ -273,12 +354,16 @@ def _reset_signal_handlers(self): signal.signal(sig, sig_handler) @staticmethod - def _hide_cursor(): - cursor.hide() + def _hide_cursor(target=None): + if not target: + target = sys.stdout + cursor.hide(stream=target) @staticmethod - def _show_cursor(): - cursor.show() + def _show_cursor(target=None): + if not target: + target = sys.stdout + cursor.show(stream=target) @staticmethod def _clear_err(): From 17c93ebf0faab6f74ed41047a11f6a13ff229da1 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Sun, 18 Nov 2018 17:44:06 -0500 Subject: [PATCH 2/3] Fix stdout spinner writing Signed-off-by: Dan Ryan --- pipenv/core.py | 21 ++++++++++++--------- pipenv/environment.py | 8 ++++---- pipenv/vendor/vistir/misc.py | 25 ++++++++++++------------- pipenv/vendor/vistir/spin.py | 24 ++++++++++++++++++++++++ 4 files changed, 52 insertions(+), 26 deletions(-) diff --git a/pipenv/core.py b/pipenv/core.py index 8c48d7fa31..3b30462542 100644 --- a/pipenv/core.py +++ b/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". diff --git a/pipenv/environment.py b/pipenv/environment.py index 7a7fd2ea02..0f1614738e 100644 --- a/pipenv/environment.py +++ b/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 @@ -413,7 +413,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): @@ -432,7 +432,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): diff --git a/pipenv/vendor/vistir/misc.py b/pipenv/vendor/vistir/misc.py index 870fa38bb4..74108d6f73 100644 --- a/pipenv/vendor/vistir/misc.py +++ b/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, @@ -148,15 +148,16 @@ def _create_subprocess( spinner=None, combine_stderr=False, display_limit=200, - start_text="" + start_text="", + write_to_stdout=True ): if not env: - env = {} + env = os.environ.copy() 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 +194,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 +205,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 diff --git a/pipenv/vendor/vistir/spin.py b/pipenv/vendor/vistir/spin.py index acb428a535..c6ae5884bc 100644 --- a/pipenv/vendor/vistir/spin.py +++ b/pipenv/vendor/vistir/spin.py @@ -93,6 +93,18 @@ def ok(self, text="OK"): self._close_output_buffer() return 0 + def hide_and_write(self, text, target=None): + if not target: + target = self.stdout + from .misc import decode_for_output + if text is None or isinstance(text, six.string_types) and text == "None": + pass + target.write(decode_for_output("\r")) + self._hide_cursor(target=target) + target.write(decode_for_output("{0}\n".format(text))) + target.write(CLEAR_LINE) + self._show_cursor(target=target) + def write(self, text=None): if not self.write_to_stdout: return self.write_err(text) @@ -189,6 +201,18 @@ def fail(self, text="FAIL", err=False): err = err or not self.write_to_stdout self._freeze(_text, err=err) + def hide_and_write(self, text, target=None): + if not target: + target = self.stdout + from .misc import decode_for_output + if text is None or isinstance(text, six.string_types) and text == "None": + pass + target.write(decode_for_output("\r")) + self._hide_cursor(target=target) + target.write(decode_for_output("{0}\n".format(text))) + target.write(CLEAR_LINE) + self._show_cursor(target=target) + def write(self, text): if not self.write_to_stdout: return self.write_err(text) From ae389f06e71e9e48186525123f4816087f052b38 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Mon, 19 Nov 2018 00:02:20 -0500 Subject: [PATCH 3/3] Update vistir Signed-off-by: Dan Ryan --- pipenv/vendor/vistir/contextmanagers.py | 4 +--- pipenv/vendor/vistir/misc.py | 7 +++++-- pipenv/vendor/vistir/spin.py | 2 -- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/pipenv/vendor/vistir/contextmanagers.py b/pipenv/vendor/vistir/contextmanagers.py index 32fad7ca4d..77fbb9df38 100644 --- a/pipenv/vendor/vistir/contextmanagers.py +++ b/pipenv/vendor/vistir/contextmanagers.py @@ -137,10 +137,8 @@ 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 use_yaspin is True: + if start_text is None and use_yaspin is True: start_text = "Running..." - else: - start_text = "" with create_spinner( spinner_name=spinner_name, text=start_text, diff --git a/pipenv/vendor/vistir/misc.py b/pipenv/vendor/vistir/misc.py index 74108d6f73..110766b4a6 100644 --- a/pipenv/vendor/vistir/misc.py +++ b/pipenv/vendor/vistir/misc.py @@ -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, @@ -152,7 +153,7 @@ def _create_subprocess( write_to_stdout=True ): if not env: - env = os.environ.copy() + env = {} try: c = _spawn_subprocess(cmd, env=env, block=block, cwd=cwd, combine_stderr=combine_stderr) @@ -308,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 diff --git a/pipenv/vendor/vistir/spin.py b/pipenv/vendor/vistir/spin.py index c6ae5884bc..5e975b6fa2 100644 --- a/pipenv/vendor/vistir/spin.py +++ b/pipenv/vendor/vistir/spin.py @@ -46,8 +46,6 @@ def __enter__(self): if self.text and self.text != "None": if self.write_to_stdout: self.write(self.text) - else: - self.write_err(self.text) return self def __exit__(self, exc_type, exc_val, traceback):