Patchwork [v2,stable] sshpeer: try harder to snag stderr when stdout closes unexpectedly

login
register
mail settings
Submitter Augie Fackler
Date April 21, 2017, 8:32 p.m.
Message ID <8862fd11cc7d27512a4b.1492806752@augie-macbookair2.roam.corp.google.com>
Download mbox | patch
Permalink /patch/20275/
State Accepted
Headers show

Comments

Augie Fackler - April 21, 2017, 8:32 p.m.
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1492114180 14400
#      Thu Apr 13 16:09:40 2017 -0400
# Branch stable
# Node ID 8862fd11cc7d27512a4b4c2a56c983d68609660b
# Parent  6e0368b6e0bb2aa5210daec091c0200583553a78
sshpeer: try harder to snag stderr when stdout closes unexpectedly

Resolves test failures on FreeBSD, but I'm not happy about the fix.

A previous version of this also wrapped readline by putting the hack
in the _call method on doublepipe. That was confusing for readers and
wasn't necessary - just doing this on read() is sufficient to fix the
bugs I'm observing. We can always come back and do readline later if
needed.
Matt Harbison - April 22, 2017, 2:55 a.m.
On Fri, 21 Apr 2017 16:32:32 -0400, Augie Fackler <raf@durin42.com> wrote:

> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1492114180 14400
> #      Thu Apr 13 16:09:40 2017 -0400
> # Branch stable
> # Node ID 8862fd11cc7d27512a4b4c2a56c983d68609660b
> # Parent  6e0368b6e0bb2aa5210daec091c0200583553a78
> sshpeer: try harder to snag stderr when stdout closes unexpectedly

This also fixes both flakey tests on Windows for me.  Thanks.

> Resolves test failures on FreeBSD, but I'm not happy about the fix.
>
> A previous version of this also wrapped readline by putting the hack
> in the _call method on doublepipe. That was confusing for readers and
> wasn't necessary - just doing this on read() is sufficient to fix the
> bugs I'm observing. We can always come back and do readline later if
> needed.
>
> diff --git a/mercurial/sshpeer.py b/mercurial/sshpeer.py
> --- a/mercurial/sshpeer.py
> +++ b/mercurial/sshpeer.py
> @@ -91,7 +91,15 @@ class doublepipe(object):
>          return self._call('write', data)
>     def read(self, size):
> -        return self._call('read', size)
> +        r = self._call('read', size)
> +        if size != 0 and not r:
> +            # We've observed a condition that indicates the
> +            # stdout closed unexpectedly. Check stderr one
> +            # more time and snag anything that's there before
> +            # letting anyone know the main part of the pipe
> +            # closed prematurely.
> +            _forwardoutput(self._ui, self._side)
> +        return r
>     def readline(self):
>          return self._call('readline')
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - April 22, 2017, 3:26 a.m.
On Fri, 21 Apr 2017 16:32:32 -0400, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1492114180 14400
> #      Thu Apr 13 16:09:40 2017 -0400
> # Branch stable
> # Node ID 8862fd11cc7d27512a4b4c2a56c983d68609660b
> # Parent  6e0368b6e0bb2aa5210daec091c0200583553a78
> sshpeer: try harder to snag stderr when stdout closes unexpectedly
> 
> Resolves test failures on FreeBSD, but I'm not happy about the fix.

Queued, thanks.

> A previous version of this also wrapped readline by putting the hack
> in the _call method on doublepipe. That was confusing for readers and
> wasn't necessary - just doing this on read() is sufficient to fix the
> bugs I'm observing. We can always come back and do readline later if
> needed.

write() was also wrapped.

Patch

diff --git a/mercurial/sshpeer.py b/mercurial/sshpeer.py
--- a/mercurial/sshpeer.py
+++ b/mercurial/sshpeer.py
@@ -91,7 +91,15 @@  class doublepipe(object):
         return self._call('write', data)
 
     def read(self, size):
-        return self._call('read', size)
+        r = self._call('read', size)
+        if size != 0 and not r:
+            # We've observed a condition that indicates the
+            # stdout closed unexpectedly. Check stderr one
+            # more time and snag anything that's there before
+            # letting anyone know the main part of the pipe
+            # closed prematurely.
+            _forwardoutput(self._ui, self._side)
+        return r
 
     def readline(self):
         return self._call('readline')