Re: [PATCH] pager: exit cleanly on SIGPIPE (BC)

[Yuya Nishihara]

On Wed, 8 Feb 2017 07:47:39 -0800, Simon Farnsworth wrote:
> # HG changeset patch
> # User Simon Farnsworth 
> # Date 1486568650 28800
> #  Wed Feb 08 07:44:10 2017 -0800
> # Node ID d50cda2a403786836d1f0d5c99401599dc4f43ec
> # Parent  1f51b4658f21bbb797e922d155c1046eddccf91d
> pager: exit cleanly on SIGPIPE (BC)

Queued per reviews, thanks.

> Changeset aaa751585325 removes SIGPIPE handling completely. This is wrong,
> as it means that Mercurial does not exit when the pager does. Instead, raise
> SignalInterrupt when SIGPIPE happens with a pager attached, to trigger the
> normal exit path.

Your comment on V1 "it breaks `hg serve`" reminded me of the weirdness of
SIGPIPE. If we did send() in paged hg process for example, the process could
be killed by SIGPIPE. With this patch, we still have a similar problem, but
the process should exit cleanly.

http://stackoverflow.com/questions/108183/
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] pager: exit cleanly on SIGPIPE (BC)

[David Soria Parra]

On Wed, Feb 08, 2017 at 07:47:39AM -0800, Simon Farnsworth wrote:
> # HG changeset patch
> # User Simon Farnsworth 
> # Date 1486568650 28800
> #  Wed Feb 08 07:44:10 2017 -0800
> # Node ID d50cda2a403786836d1f0d5c99401599dc4f43ec
> # Parent  1f51b4658f21bbb797e922d155c1046eddccf91d
> pager: exit cleanly on SIGPIPE (BC)
> 

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

Re: [PATCH] pager: exit cleanly on SIGPIPE (BC)

[Simon Farnsworth]

This replaces "dispatch: treat SIGPIPE as a termination signal (BC)".

I'm now debugging test-paths.t, so that I can stop being acclimatised to 
the red failure prompt after running tests, to improve my chances of 
noticing this sort of issue.


Simon

On 08/02/2017 15:47, Simon Farnsworth wrote:

# HG changeset patch
# User Simon Farnsworth 
# Date 1486568650 28800
#  Wed Feb 08 07:44:10 2017 -0800
# Node ID d50cda2a403786836d1f0d5c99401599dc4f43ec
# Parent  1f51b4658f21bbb797e922d155c1046eddccf91d
pager: exit cleanly on SIGPIPE (BC)

Changeset aaa751585325 removes SIGPIPE handling completely. This is wrong,
as it means that Mercurial does not exit when the pager does. Instead, raise
SignalInterrupt when SIGPIPE happens with a pager attached, to trigger the
normal exit path.

This will cause "killed!" to be printed to stderr (hence the BC warning),
but in the normal pager use case (where the pager gets both stderr and
stdout), this messsage is lost as we only get SIGPIPE when the pager quits.

diff --git a/hgext/pager.py b/hgext/pager.py
--- a/hgext/pager.py
+++ b/hgext/pager.py
@@ -72,6 +72,7 @@
 commands,
 dispatch,
 encoding,
+error,
 extensions,
 util,
 )
@@ -114,6 +115,9 @@
 os.dup2(stderrfd, util.stderr.fileno())
 pager.wait()

+def catchterm(*args):
+raise error.SignalInterrupt
+
 def uisetup(ui):
 class pagerui(ui.__class__):
 def _runpager(self, pagercmd):
@@ -153,6 +157,8 @@
 if usepager:
 ui.setconfig('ui', 'formatted', ui.formatted(), 'pager')
 ui.setconfig('ui', 'interactive', False, 'pager')
+if util.safehasattr(signal, "SIGPIPE"):
+signal.signal(signal.SIGPIPE, catchterm)
 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=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=JGdkzd8E23MJxg59Iu-jc-4lKtVDPSGtiCxGk7Tvyfk=26T_I6At53KfO_UPKKWvM8tgFUbZStBE44m6jbEfsrk=



--
Simon Farnsworth
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH] pager: exit cleanly on SIGPIPE (BC)

[Simon Farnsworth]

# HG changeset patch
# User Simon Farnsworth 
# Date 1486568650 28800
#  Wed Feb 08 07:44:10 2017 -0800
# Node ID d50cda2a403786836d1f0d5c99401599dc4f43ec
# Parent  1f51b4658f21bbb797e922d155c1046eddccf91d
pager: exit cleanly on SIGPIPE (BC)

Changeset aaa751585325 removes SIGPIPE handling completely. This is wrong,
as it means that Mercurial does not exit when the pager does. Instead, raise
SignalInterrupt when SIGPIPE happens with a pager attached, to trigger the
normal exit path.

This will cause "killed!" to be printed to stderr (hence the BC warning),
but in the normal pager use case (where the pager gets both stderr and
stdout), this messsage is lost as we only get SIGPIPE when the pager quits.

diff --git a/hgext/pager.py b/hgext/pager.py
--- a/hgext/pager.py
+++ b/hgext/pager.py
@@ -72,6 +72,7 @@
 commands,
 dispatch,
 encoding,
+error,
 extensions,
 util,
 )
@@ -114,6 +115,9 @@
 os.dup2(stderrfd, util.stderr.fileno())
 pager.wait()
 
+def catchterm(*args):
+raise error.SignalInterrupt
+
 def uisetup(ui):
 class pagerui(ui.__class__):
 def _runpager(self, pagercmd):
@@ -153,6 +157,8 @@
 if usepager:
 ui.setconfig('ui', 'formatted', ui.formatted(), 'pager')
 ui.setconfig('ui', 'interactive', False, 'pager')
+if util.safehasattr(signal, "SIGPIPE"):
+signal.signal(signal.SIGPIPE, catchterm)
 ui._runpager(p)
 return orig(ui, options, cmd, cmdfunc)
 
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel