]> arthur.barton.de Git - bup.git/commitdiff
Clean subprocess output without newliner
authorRob Browning <rlb@defaultvalue.org>
Sun, 26 Jun 2016 20:49:14 +0000 (15:49 -0500)
committerRob Browning <rlb@defaultvalue.org>
Mon, 15 Jan 2018 19:57:57 +0000 (13:57 -0600)
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 <rlb@defaultvalue.org>
Tested-by: Rob Browning <rlb@defaultvalue.org>
Documentation/bup-newliner.md [deleted file]
cmd/newliner-cmd.py [deleted file]
lib/bup/helpers.py
main.py

diff --git a/Documentation/bup-newliner.md b/Documentation/bup-newliner.md
deleted file mode 100644 (file)
index 6ae7ee6..0000000
+++ /dev/null
@@ -1,42 +0,0 @@
-% bup-newliner(1) Bup %BUP_VERSION%
-% Avery Pennarun <apenwarr@gmail.com>
-% %BUP_DATE%
-
-# NAME
-
-bup-newliner - make sure progress messages don't overlap with output
-
-# SYNOPSIS
-
-\<any command\> 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 (executable)
index 20027ab..0000000
+++ /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)
index 9f404afdfc2da4da51b05cde91881cb120f7bd3e..d05226ebc28207dd94435a6c920ef99c4933b721 100644 (file)
@@ -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 33d21dfeecdd201b763bd35f01f6336da437af6a..119826564d7266815639d978c9d11400c5364ddc 100755 (executable)
--- 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] '
         '<command> [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))