[PATCH] chg: always wait for pager

[Augie Fackler]

On Wed, Apr 12, 2017 at 12:56:28PM +0100, Ryan McElroy wrote:
> 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.

chg is still semi-experimental (I think?) and we're close to a freeze. Queued for default.

>
> >
> > 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.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel