[PATCH] pager: preserve Hg's exit code (and fix Windows support) (issue3225)

[Brodie Rao]

On Fri, May 11, 2012 at 3:49 PM,  <brodie at bitheap.org> wrote:
> # HG changeset patch
> # User Brodie Rao <brodie at sf.io>
> # Date 1336743937 -7200
> # Branch stable

FYI: This was supposed to be flagged for stable in the email subject,
but I messed up my patchbomb command.

> # Node ID e11b4d33bf59e5f51fe5224e590f85bbfce8bde0
> # Parent  e22d6b1dec1d4bda684a8d90e8d4852500d69495
> pager: preserve Hg's exit code (and fix Windows support) (issue3225)
>
> This changes how the pager extension invokes the pager. Prior to this change,
> the extension would fork Hg and exec the pager in the parent process. This
> loses Hg exit code, and it doesn't work on Windows.
>
> Now the pager is invoked using the subprocess library, and an atexit handler is
> registered that makes Hg wait for the pager to exit before it exits itself.
>
> Note that if you exit the pager before Hg is done running, you'll get an exit
> code of 255, which is caused by Python blowing up due to a broken pipe. If you
> set pager.quiet=True, you'll get the OS-level return code of 141.
>
> diff --git a/hgext/pager.py b/hgext/pager.py
> --- a/hgext/pager.py
> +++ b/hgext/pager.py
> @@ -53,37 +53,27 @@ used. Use a boolean value like yes, no,
>  normal behavior.
>  '''
>
> -import sys, os, signal, shlex, errno
> +import atexit, sys, os, signal, subprocess
>  from mercurial import commands, dispatch, util, extensions
>  from mercurial.i18n import _
>
>  def _runpager(p):
> -    if not util.safehasattr(os, 'fork'):
> -        sys.stdout = util.popen(p, 'wb')
> -        if util.isatty(sys.stderr):
> -            sys.stderr = sys.stdout
> -        return
> -    fdin, fdout = os.pipe()
> -    pid = os.fork()
> -    if pid == 0:
> -        os.close(fdin)
> -        os.dup2(fdout, sys.stdout.fileno())
> -        if util.isatty(sys.stderr):
> -            os.dup2(fdout, sys.stderr.fileno())
> -        os.close(fdout)
> -        return
> -    os.dup2(fdin, sys.stdin.fileno())
> -    os.close(fdin)
> -    os.close(fdout)
> -    try:
> -        os.execvp('/bin/sh', ['/bin/sh', '-c', p])
> -    except OSError, e:
> -        if e.errno == errno.ENOENT:
> -            # no /bin/sh, try executing the pager directly
> -            args = shlex.split(p)
> -            os.execvp(args[0], args)
> -        else:
> -            raise
> +    pager = subprocess.Popen(p, shell=True, bufsize=-1,
> +                             close_fds=util.closefds, stdin=subprocess.PIPE,
> +                             stdout=sys.stdout, stderr=sys.stderr)
> +
> +    stdout = os.dup(sys.stdout.fileno())
> +    stderr = os.dup(sys.stderr.fileno())
> +    os.dup2(pager.stdin.fileno(), sys.stdout.fileno())
> +    if util.isatty(sys.stderr):
> +        os.dup2(pager.stdin.fileno(), sys.stderr.fileno())
> +
> +    @atexit.register
> +    def killpager():
> +        pager.stdin.close()
> +        os.dup2(stdout, sys.stdout.fileno())
> +        os.dup2(stderr, sys.stderr.fileno())
> +        pager.wait()
>
>  def uisetup(ui):
>     if ui.plain() or '--debugger' in sys.argv or not util.isatty(sys.stdout):