Patchwork hook: ensure stderr is flushed when an exception is raised, for test stability

login
register
mail settings
Submitter Matt Harbison
Date March 11, 2018, 3:30 a.m.
Message ID <5dd79bb7e5a3cf33d04c.1520739057@Envy>
Download mbox | patch
Permalink /patch/29275/
State Accepted
Headers show

Comments

Matt Harbison - March 11, 2018, 3:30 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1520737378 18000
#      Sat Mar 10 22:02:58 2018 -0500
# Node ID 5dd79bb7e5a3cf33d04c00e7c6c52070f472149d
# Parent  7c4e5abd9e7e903e6916b30ccc0d2b26d69a5fca
hook: ensure stderr is flushed when an exception is raised, for test stability

Windows has had issues with output order in test-ssh-proto-unbundle.t[1] since
it was created a few weeks ago.  Each of the problems occurred when an exception
was thrown out of the hook.

Now the only thing blocking D2720 is the fact that the "abort: ..." lines on
stderr are totally AWOL.  I have no idea where there are.

[1] https://buildbot.mercurial-scm.org/builders/Win7%20x86_64%20hg%20tests/builds/541/steps/run-tests.py%20%28python%202.7.13%29/logs/stdio
Yuya Nishihara - March 11, 2018, 9:32 a.m.
On Sat, 10 Mar 2018 22:30:57 -0500, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1520737378 18000
> #      Sat Mar 10 22:02:58 2018 -0500
> # Node ID 5dd79bb7e5a3cf33d04c00e7c6c52070f472149d
> # Parent  7c4e5abd9e7e903e6916b30ccc0d2b26d69a5fca
> hook: ensure stderr is flushed when an exception is raised, for test stability

Queued, thanks.

> diff --git a/mercurial/hook.py b/mercurial/hook.py
> --- a/mercurial/hook.py
> +++ b/mercurial/hook.py
> @@ -265,12 +265,12 @@ def runhooks(ui, repo, htype, hooks, thr
>                  raised = False
>  
>              res[hname] = r, raised
> +    finally:
> +        # The stderr is fully buffered on Windows when connected to a pipe.
> +        # A forcible flush is required to make small stderr data in the
> +        # remote side available to the client immediately.
> +        util.stderr.flush()
>  
> -            # The stderr is fully buffered on Windows when connected to a pipe.
> -            # A forcible flush is required to make small stderr data in the
> -            # remote side available to the client immediately.
> -            util.stderr.flush()

Appears that we didn't have to flush stderr per hook, so this change seems
good.

Patch

diff --git a/mercurial/hook.py b/mercurial/hook.py
--- a/mercurial/hook.py
+++ b/mercurial/hook.py
@@ -265,12 +265,12 @@  def runhooks(ui, repo, htype, hooks, thr
                 raised = False
 
             res[hname] = r, raised
+    finally:
+        # The stderr is fully buffered on Windows when connected to a pipe.
+        # A forcible flush is required to make small stderr data in the
+        # remote side available to the client immediately.
+        util.stderr.flush()
 
-            # The stderr is fully buffered on Windows when connected to a pipe.
-            # A forcible flush is required to make small stderr data in the
-            # remote side available to the client immediately.
-            util.stderr.flush()
-    finally:
         if _redirect and oldstdout >= 0:
             util.stdout.flush()  # write hook output to stderr fd
             os.dup2(oldstdout, stdoutno)