[PATCH] chg: always wait for pager

Ryan McElroy rm at fb.com
Wed Apr 12 07:56:28 EDT 2017


On 4/12/17 2:36 AM, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark at fb.com>
> # Date 1491960700 25200
> #      Tue Apr 11 18:31:40 2017 -0700
> # Node ID 5d977772164deadb66b4c9d27c2148fcadb03f0a
> # Parent  f7b3677f66cd94fa01f345f6ab35229264aed179
> chg: always wait for pager

Should this be on STABLE? It seems like a kind of bad issue.

>
> Previously, when runcommand raises, chg aborts with, and does not wait for
> pager. The call stack is like:
>
>    hgc_runcommand -> handleresponse -> readchannel -> debugmsg("failed to
>    read channel") -> exit(255)
>
> That means, chg returns to the shell, then both the pager and the shell will
> read from the terminal at the same time, causing problems.
>
> This patch fixes that by using "atexit" to register the pager cleanup
> function so chg will always wait for pager even if runcommand raises.
>
> diff --git a/contrib/chg/chg.c b/contrib/chg/chg.c
> --- a/contrib/chg/chg.c
> +++ b/contrib/chg/chg.c
> @@ -448,9 +448,9 @@ int main(int argc, const char *argv[], c
>   
>   	setupsignalhandler(hgc_peerpid(hgc), hgc_peerpgid(hgc));
> +	atexit(waitpager);
>   	int exitcode = hgc_runcommand(hgc, argv + 1, argc - 1);
>   	restoresignalhandler();
>   	hgc_close(hgc);
>   	freecmdserveropts(&opts);
> -	waitpager();
>   
>   	return exitcode;
> diff --git a/tests/test-chg.t b/tests/test-chg.t
> --- a/tests/test-chg.t
> +++ b/tests/test-chg.t
> @@ -103,4 +103,35 @@ pager should be enabled if the attached
>     0:1f7b0de80e11
>   
> +chg waits for pager if runcommand raises
> +
> +  $ cat > $TESTTMP/crash.py <<EOF
> +  > from mercurial import cmdutil
> +  > cmdtable = {}
> +  > command = cmdutil.command(cmdtable)
> +  > @command('crash')
> +  > def pagercrash(ui, repo, *pats, **opts):
> +  >     ui.write('going to crash\n')
> +  >     raise Exception('.')
> +  > EOF
> +
> +  $ cat > $TESTTMP/fakepager.py <<EOF
> +  > import sys, time
> +  > for line in iter(sys.stdin.readline, ''):
> +  >     if 'crash' in line: # only interested in lines containing 'crash'
> +  >         # if chg exits when pager is sleeping (incorrectly), the output
> +  >         # will be captured by the next test case
> +  >         time.sleep(1)
> +  >         sys.stdout.write('crash-pager: %s' % line)
> +  > EOF
> +
> +  $ cat >> .hg/hgrc <<EOF
> +  > [extensions]
> +  > crash = $TESTTMP/crash.py
> +  > EOF
> +
> +  $ chg crash --pager=on --config ui.formatted=True 2>/dev/null
> +  crash-pager: going to crash
> +  [255]
> +
>     $ cd ..
>   
>

The code change looks good to me. I'm in favor of taking this so I'll 
mark as pre-reviewed.


More information about the Mercurial-devel mailing list