Patchwork [1,of,6,v2] pager: don't terminate with extreme prejudice on SIGPIPE (BC)

login
register
mail settings
Submitter Simon Farnsworth
Date Feb. 2, 2017, 7:18 p.m.
Message ID <f443bd95a948aaed36e8.1486063135@devvm022.lla2.facebook.com>
Download mbox | patch
Permalink /patch/18300/
State Accepted
Headers show

Comments

Simon Farnsworth - Feb. 2, 2017, 7:18 p.m.
# HG changeset patch
# User Simon Farnsworth <simonfar@fb.com>
# Date 1486063056 28800
#      Thu Feb 02 11:17:36 2017 -0800
# Node ID f443bd95a948aaed36e81edb884085fc2f0f5acf
# Parent  abf029200e198878a4576a87e095bd8d77d9cea9
pager: don't terminate with extreme prejudice on SIGPIPE (BC)

The default SIGPIPE handler causes Mercurial to exit immediately, without
running any Python cleanup code (except and finally blocks, atexit handlers
etc). This creates problems if you want to do something at exit.

If we need a different exit code for broken pipe from pager, then we should
code that ourselves in Python; this appears to have been cargo-culted from
the fork implementation of pager that's no longer used, where it was needed
to stop Broken Pipe errors appearing on the user's terminal.
Bryan O'Sullivan - Feb. 2, 2017, 8:04 p.m.
On Thu, Feb 2, 2017 at 11:18 AM, Simon Farnsworth <simonfar@fb.com> wrote:

> pager: don't terminate with extreme prejudice on SIGPIPE (BC)
>

Looks good!
Yuya Nishihara - Feb. 3, 2017, 1:59 p.m.
On Thu, 2 Feb 2017 12:04:09 -0800, Bryan O'Sullivan wrote:
> On Thu, Feb 2, 2017 at 11:18 AM, Simon Farnsworth <simonfar@fb.com> wrote:
> 
> > pager: don't terminate with extreme prejudice on SIGPIPE (BC)
> >
> 
> Looks good!

Queued this first patch, thanks.
Jun Wu - Feb. 7, 2017, 7:05 p.m.
Excerpts from Yuya Nishihara's message of 2017-02-03 22:59:07 +0900:
> On Thu, 2 Feb 2017 12:04:09 -0800, Bryan O'Sullivan wrote:
> > On Thu, Feb 2, 2017 at 11:18 AM, Simon Farnsworth <simonfar@fb.com> wrote:
> > 
> > > pager: don't terminate with extreme prejudice on SIGPIPE (BC)
> > >
> > 
> > Looks good!
> 
> Queued this first patch, thanks.

This actually breaks the pager practically. Running "hg log" with a long
history will not end even when the user presses "q" to exit the pager.
Augie Fackler - Feb. 7, 2017, 7:11 p.m.
On Tue, Feb 7, 2017 at 2:05 PM, Jun Wu <quark@fb.com> wrote:
> Excerpts from Yuya Nishihara's message of 2017-02-03 22:59:07 +0900:
>> On Thu, 2 Feb 2017 12:04:09 -0800, Bryan O'Sullivan wrote:
>> > On Thu, Feb 2, 2017 at 11:18 AM, Simon Farnsworth <simonfar@fb.com> wrote:
>> >
>> > > pager: don't terminate with extreme prejudice on SIGPIPE (BC)
>> > >
>> >
>> > Looks good!
>>
>> Queued this first patch, thanks.
>
> This actually breaks the pager practically. Running "hg log" with a long
> history will not end even when the user presses "q" to exit the pager.

Ugh. Can you send me a backout with an explanation about why we need
this in the commit message? Thanks!

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/hgext/pager.py b/hgext/pager.py
--- a/hgext/pager.py
+++ b/hgext/pager.py
@@ -153,8 +153,6 @@ 
         if usepager:
             ui.setconfig('ui', 'formatted', ui.formatted(), 'pager')
             ui.setconfig('ui', 'interactive', False, 'pager')
-            if util.safehasattr(signal, "SIGPIPE"):
-                signal.signal(signal.SIGPIPE, signal.SIG_DFL)
             ui._runpager(p)
         return orig(ui, options, cmd, cmdfunc)