From 00fb1f1b2a53935ca7d5ce95ea4cf56b7f9bcc3d Mon Sep 17 00:00:00 2001 From: Rob Browning Date: Sun, 26 Jun 2016 15:49:14 -0500 Subject: [PATCH] Clean subprocess output without newliner Instead of launching a newliner subprocess from main.py to clean up the output from other subprocesses (with respect to \r, shorter lines as compared to the previous line, etc.), do something similar directly from main.py via select, and then drop all of the attendant signal forwarding, subprocess detaching via setsid(), etc. that the newliner arrangement had accumulated. Among other things, this should make bup much more responsive to TERM/QUIT/etc. signals (i.e. C-c). Previously the index/save/... subprocess might continue running even after bup appeared to have quit. To clean up the output, regularly check tty_width() and always write "whole" space-padded lines, tracking stdout and stderr independently (bup-newliner redirected both to stdout). Related: aa6f2c87e6f1292f1fa22f618532b65a5565d604 b7a524ccb662c9ed3ebd786da0f45f459929ef45 119f9dd3b3421c31578105954a34fc5e32826ae5 073b383882e73904ae04179e69a68419d8d57199 Signed-off-by: Rob Browning Tested-by: Rob Browning --- Documentation/bup-newliner.md | 42 -------- cmd/newliner-cmd.py | 53 ---------- lib/bup/helpers.py | 9 +- main.py | 183 +++++++++++++++++++++------------- 4 files changed, 118 insertions(+), 169 deletions(-) delete mode 100644 Documentation/bup-newliner.md delete mode 100755 cmd/newliner-cmd.py diff --git a/Documentation/bup-newliner.md b/Documentation/bup-newliner.md deleted file mode 100644 index 6ae7ee6..0000000 --- a/Documentation/bup-newliner.md +++ /dev/null @@ -1,42 +0,0 @@ -% bup-newliner(1) Bup %BUP_VERSION% -% Avery Pennarun -% %BUP_DATE% - -# NAME - -bup-newliner - make sure progress messages don't overlap with output - -# SYNOPSIS - -\ 2>&1 | bup newliner - -# DESCRIPTION - -`bup newliner` is run automatically by bup. You shouldn't -need it unless you're using it in some other program. - -Progress messages emitted by bup (and some other tools) are -of the form "Message ### content\\r", that is, a status -message containing a variable-length number, followed by a -carriage return character and no newline. If these -messages are printed more than once, they overwrite each -other, so what the user sees is a single line with a -continually-updating number. - -This works fine until some other message is printed. For -example, progress messages are usually printed to stderr, -but other program messages might be printed to stdout. If -those messages are shorter than the progress message line, -the screen will be left with weird looking artifacts as the -two messages get mixed together. - -`bup newliner` prints extra space characters at the right -time to make sure that doesn't happen. - -If you're running a program that has problems with these -artifacts, you can usually fix them by piping its stdout -*and* its stderr through bup newliner. - -# BUP - -Part of the `bup`(1) suite. diff --git a/cmd/newliner-cmd.py b/cmd/newliner-cmd.py deleted file mode 100755 index 20027ab..0000000 --- a/cmd/newliner-cmd.py +++ /dev/null @@ -1,53 +0,0 @@ -#!/bin/sh -"""": # -*-python-*- -bup_python="$(dirname "$0")/bup-python" || exit $? -exec "$bup_python" "$0" ${1+"$@"} -""" -# end of bup preamble -import sys, os, re -from bup import options -from bup import _helpers # fixes up sys.argv on import - -optspec = """ -bup newliner -""" -o = options.Options(optspec) -(opt, flags, extra) = o.parse(sys.argv[1:]) - -if extra: - o.fatal("no arguments expected") - -r = re.compile(r'([\r\n])') -lastlen = 0 -all = '' -width = options._tty_width() or 78 -while 1: - l = r.split(all, 1) - if len(l) <= 1: - if len(all) >= 160: - sys.stdout.write('%s\n' % all[:78]) - sys.stdout.flush() - all = all[78:] - try: - b = os.read(sys.stdin.fileno(), 4096) - except KeyboardInterrupt: - break - if not b: - break - all += b - else: - assert(len(l) == 3) - (line, splitchar, all) = l - if splitchar == '\r': - line = line[:width] - sys.stdout.write('%-*s%s' % (lastlen, line, splitchar)) - if splitchar == '\r': - lastlen = len(line) - else: - lastlen = 0 - sys.stdout.flush() - -if lastlen: - sys.stdout.write('%-*s\r' % (lastlen, '')) -if all: - sys.stdout.write('%s\n' % all) diff --git a/lib/bup/helpers.py b/lib/bup/helpers.py index 9f404af..d05226e 100644 --- a/lib/bup/helpers.py +++ b/lib/bup/helpers.py @@ -11,6 +11,10 @@ import hashlib, heapq, math, operator, time, grp, tempfile from bup import _helpers from bup import compat +# This function should really be in helpers, not in bup.options. But we +# want options.py to be standalone so people can include it in other projects. +from bup.options import _tty_width as tty_width + class Nonlocal: """Helper to deal with Python scoping issues""" @@ -24,11 +28,6 @@ sc_arg_max = os.sysconf('SC_ARG_MAX') if sc_arg_max == -1: # "no definite limit" - let's choose 2M sc_arg_max = 2 * 1024 * 1024 -# This function should really be in helpers, not in bup.options. But we -# want options.py to be standalone so people can include it in other projects. -from bup.options import _tty_width -tty_width = _tty_width - def last(iterable): result = None diff --git a/main.py b/main.py index 33d21df..1198265 100755 --- a/main.py +++ b/main.py @@ -5,8 +5,12 @@ exec "$bup_python" "$0" ${1+"$@"} """ # end of bup preamble -import sys, os, subprocess, signal, getopt +import errno, re, sys, os, subprocess, signal, getopt +from fcntl import F_GETFL, F_SETFL +from subprocess import PIPE +from sys import stderr, stdout +import fcntl, select argv = sys.argv exe = os.path.realpath(argv[0]) @@ -32,13 +36,10 @@ os.environ['BUP_RESOURCE_PATH'] = resourcepath from bup import helpers -from bup.compat import wrap_main +from bup.compat import add_ex_tb, chain_ex, wrap_main from bup.helpers import atoi, columnate, debug1, log, tty_width -# after running 'bup newliner', the tty_width() ioctl won't work anymore -os.environ['WIDTH'] = str(tty_width()) - def usage(msg=""): log('Usage: bup [-?|--help] [-d BUP_DIR] [--debug] [--profile] ' ' [options...]\n\n') @@ -145,74 +146,118 @@ def force_tty(): if fix_stdout or fix_stderr: amt = (fix_stdout and 1 or 0) + (fix_stderr and 2 or 0) os.environ['BUP_FORCE_TTY'] = str(amt) - os.setsid() # make sure ctrl-c is sent just to us, not to child too - -if fix_stdout or fix_stderr: - realf = fix_stderr and 2 or 1 - drealf = os.dup(realf) # Popen goes crazy with stdout=2 - n = subprocess.Popen([subpath('newliner')], - stdin=subprocess.PIPE, stdout=drealf, - close_fds=True, preexec_fn=force_tty) - os.close(drealf) - outf = fix_stdout and n.stdin.fileno() or None - errf = fix_stderr and n.stdin.fileno() or None -else: - n = None - outf = None - errf = None -ret = 95 -p = None -forward_signals = True -def handler(signum, frame): - debug1('\nbup: signal %d received\n' % signum) - if not p or not forward_signals: +sep_rx = re.compile(r'([\r\n])') + +def print_clean_line(dest, content, width, sep=None): + """Write some or all of content, followed by sep, to the dest fd after + padding the content with enough spaces to fill the current + terminal width or truncating it to the terminal width if sep is a + carriage return.""" + global sep_rx + assert sep in ('\r', '\n', None) + if not content: + if sep: + os.write(dest, sep) return - if signum != signal.SIGTSTP: - os.kill(p.pid, signum) - else: # SIGTSTP: stop the child, then ourselves. - os.kill(p.pid, signal.SIGSTOP) - signal.signal(signal.SIGTSTP, signal.SIG_DFL) - os.kill(os.getpid(), signal.SIGTSTP) - # Back from suspend -- reestablish the handler. - signal.signal(signal.SIGTSTP, handler) - ret = 94 - -def main(): - signal.signal(signal.SIGTERM, handler) - signal.signal(signal.SIGINT, handler) - signal.signal(signal.SIGTSTP, handler) - signal.signal(signal.SIGCONT, handler) + for x in content: + assert not sep_rx.match(x) + content = ''.join(content) + if sep == '\r' and len(content) > width: + content = content[width:] + os.write(dest, content) + if len(content) < width: + os.write(dest, ' ' * (width - len(content))) + os.write(dest, sep) +def filter_output(src_out, src_err, dest_out, dest_err): + """Transfer data from src_out to dest_out and src_err to dest_err via + print_clean_line until src_out and src_err close.""" + global sep_rx + assert not isinstance(src_out, bool) + assert not isinstance(src_err, bool) + assert not isinstance(dest_out, bool) + assert not isinstance(dest_err, bool) + assert src_out is not None or src_err is not None + assert (src_out is None) == (dest_out is None) + assert (src_err is None) == (dest_err is None) + pending = {} + pending_ex = None try: - try: - c = (do_profile and [sys.executable, '-m', 'cProfile'] or []) + subcmd - if not n and not outf and not errf: - # shortcut when no bup-newliner stuff is needed - os.execvp(c[0], c) - else: - p = subprocess.Popen(c, stdout=outf, stderr=errf, - preexec_fn=force_tty) - while 1: - # if we get a signal while waiting, we have to keep waiting, just - # in case our child doesn't die. - ret = p.wait() - forward_signals = False - break - except OSError as e: - log('%s: %s\n' % (subcmd[0], e)) - ret = 98 - finally: - if p and p.poll() == None: - os.kill(p.pid, signal.SIGTERM) - p.wait() - if n: - n.stdin.close() + fds = tuple([x for x in (src_out, src_err) if x is not None]) + for fd in fds: + flags = fcntl.fcntl(fd, F_GETFL) + assert fcntl.fcntl(fd, F_SETFL, flags | os.O_NONBLOCK) == 0 + while fds: + ready_fds, _, _ = select.select(fds, [], []) + width = tty_width() + for fd in ready_fds: + buf = os.read(fd, 4096) + dest = dest_out if fd == src_out else dest_err + if not buf: + fds = tuple([x for x in fds if x is not fd]) + print_clean_line(dest, pending.pop(fd, []), width) + else: + split = sep_rx.split(buf) + if len(split) > 2: + while len(split) > 1: + content, sep = split[:2] + split = split[2:] + print_clean_line(dest, + pending.pop(fd, []) + [content], + width, + sep) + else: + assert(len(split) == 1) + pending.setdefault(fd, []).extend(split) + except BaseException as ex: + pending_ex = chain_ex(add_ex_tb(ex), pending_ex) + try: + # Try to finish each of the streams + for fd, pending_items in pending.iteritems(): + dest = dest_out if fd == src_out else dest_err try: - n.wait() - except: - pass - sys.exit(ret) + print_clean_line(dest, pending_items, width) + except (EnvironmentError, EOFError) as ex: + pending_ex = chain_ex(add_ex_tb(ex), pending_ex) + except BaseException as ex: + pending_ex = chain_ex(add_ex_tb(ex), pending_ex) + if pending_ex: + raise pending_ex -wrap_main(main) +def run_subcmd(subcmd): + + c = (do_profile and [sys.executable, '-m', 'cProfile'] or []) + subcmd + if not (fix_stdout or fix_stderr): + os.execvp(c[0], c) + + p = None + try: + p = subprocess.Popen(c, + stdout=PIPE if fix_stdout else sys.stdout, + stderr=PIPE if fix_stderr else sys.stderr, + preexec_fn=force_tty, + bufsize=4096, + close_fds=True) + # Assume p will receive these signals and quit, which will + # then cause us to quit. + for sig in (signal.SIGINT, signal.SIGTERM, signal.SIGQUIT): + signal.signal(sig, signal.SIG_IGN) + + filter_output(fix_stdout and p.stdout.fileno() or None, + fix_stderr and p.stderr.fileno() or None, + fix_stdout and sys.stdout.fileno() or None, + fix_stderr and sys.stderr.fileno() or None) + return p.wait() + except BaseException as ex: + add_ex_tb(ex) + try: + if p and p.poll() == None: + os.kill(p.pid, signal.SIGTERM) + p.wait() + except BaseException as kill_ex: + raise chain_ex(add_ex_tb(kill_ex), ex) + raise ex + +wrap_main(lambda : run_subcmd(subcmd)) -- 2.39.2