Patchwork pager: backed out changeset aaa751585325

login
register
mail settings
Submitter Jun Wu
Date Feb. 7, 2017, 7:23 p.m.
Message ID <8cfbd33c54779d1bbd50.1486495422@x1c>
Download mbox | patch
Permalink /patch/18339/
State Superseded
Delegated to: Jun Wu
Headers show

Comments

Jun Wu - Feb. 7, 2017, 7:23 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1486495380 28800
#      Tue Feb 07 11:23:00 2017 -0800
# Node ID 8cfbd33c54779d1bbd50412e99f8256eea954401
# Parent  a68510b69f413545722c086eaeb840dd5e8305b4
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 8cfbd33c5477
pager: backed out changeset aaa751585325

When a command needs a long time (ex. "hg log" on a big repo), and the user
exits the pager, they expect the hg command to exit immediately.

To be able to do that, terminating on SIGPIPE is important. Therefore revert
the change that ignores SIGPIPE.
Simon Farnsworth - Feb. 7, 2017, 8:10 p.m.
I've sent a patch ("[PATCH] dispatch: treat SIGPIPE as a termination 
signal (BC)") which fixes this up properly, instead of simply backing it 
out.

The underlying bug that aaa751585325 tries to fix is that setting 
SIGPIPE to SIG_DFL results in the interpreter not running any bytecode 
after SIGPIPE comes in, which isn't great if you're trying to do cleanup 
or logging at exit.

Simon

On 07/02/2017 19:23, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1486495380 28800
> #      Tue Feb 07 11:23:00 2017 -0800
> # Node ID 8cfbd33c54779d1bbd50412e99f8256eea954401
> # Parent  a68510b69f413545722c086eaeb840dd5e8305b4
> # Available At https://urldefense.proofpoint.com/v2/url?u=https-3A__bitbucket.org_quark-2Dzju_hg-2Ddraft&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=mEgSWILcY4c4W3zjApBQLA&m=Kkdsd1P9qEpbCBrbRyenM6kQC-sD5CNSJJxglAJzfec&s=3RFgXLLGBcJtw4HNz3jveuIu_mqKeZu-G4oap06m0AA&e=
> #              hg pull https://urldefense.proofpoint.com/v2/url?u=https-3A__bitbucket.org_quark-2Dzju_hg-2Ddraft&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=mEgSWILcY4c4W3zjApBQLA&m=Kkdsd1P9qEpbCBrbRyenM6kQC-sD5CNSJJxglAJzfec&s=3RFgXLLGBcJtw4HNz3jveuIu_mqKeZu-G4oap06m0AA&e=  -r 8cfbd33c5477
> pager: backed out changeset aaa751585325
>
> When a command needs a long time (ex. "hg log" on a big repo), and the user
> exits the pager, they expect the hg command to exit immediately.
>
> To be able to do that, terminating on SIGPIPE is important. Therefore revert
> the change that ignores SIGPIPE.
>
> diff --git a/hgext/pager.py b/hgext/pager.py
> --- a/hgext/pager.py
> +++ b/hgext/pager.py
> @@ -145,4 +145,6 @@ def uisetup(ui):
>              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)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=mEgSWILcY4c4W3zjApBQLA&m=Kkdsd1P9qEpbCBrbRyenM6kQC-sD5CNSJJxglAJzfec&s=lRjI_yaSrj_N7KUWb3_WFaRF_2KzfxHikgvyHTXS-Lg&e=
>
Jun Wu - Feb. 7, 2017, 8:59 p.m.
I've marked this as Superseded, as Simon's patch is a better fix.

Excerpts from Jun Wu's message of 2017-02-07 11:23:42 -0800:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1486495380 28800
> #      Tue Feb 07 11:23:00 2017 -0800
> # Node ID 8cfbd33c54779d1bbd50412e99f8256eea954401
> # Parent  a68510b69f413545722c086eaeb840dd5e8305b4
> # Available At https://bitbucket.org/quark-zju/hg-draft 
> #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r 8cfbd33c5477
> pager: backed out changeset aaa751585325
> 
> When a command needs a long time (ex. "hg log" on a big repo), and the user
> exits the pager, they expect the hg command to exit immediately.
> 
> To be able to do that, terminating on SIGPIPE is important. Therefore revert
> the change that ignores SIGPIPE.
> 
> diff --git a/hgext/pager.py b/hgext/pager.py
> --- a/hgext/pager.py
> +++ b/hgext/pager.py
> @@ -145,4 +145,6 @@ def uisetup(ui):
>              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)

Patch

diff --git a/hgext/pager.py b/hgext/pager.py
--- a/hgext/pager.py
+++ b/hgext/pager.py
@@ -145,4 +145,6 @@  def uisetup(ui):
             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)