Patchwork [v3] util: always force line buffered stdout when stdout is a tty (BC)

login
register
mail settings
Submitter Simon Farnsworth
Date Feb. 3, 2017, 11:15 p.m.
Message ID <08b6e1bb20ca6bd1465a.1486163706@devvm022.lla2.facebook.com>
Download mbox | patch
Permalink /patch/18324/
State Accepted
Headers show

Comments

Simon Farnsworth - Feb. 3, 2017, 11:15 p.m.
# HG changeset patch
# User Simon Farnsworth <simonfar@fb.com>
# Date 1486163427 28800
#      Fri Feb 03 15:10:27 2017 -0800
# Node ID 08b6e1bb20ca6bd1465a3d22bab49debfe776bae
# Parent  1f51b4658f21bbb797e922d155c1046eddccf91d
util: always force line buffered stdout when stdout is a tty (BC)

pager replaced stdout with a line buffered version to work around glibc
deciding on a buffering strategy on the first write to stdout. This is going
to make my next patch hard, as replacing stdout will make tracking time
spent blocked on it more challenging.

Move the line buffering requirement to util.py, and remove it from pager.
This means that the abuse of ui.formatted=True and pager set to cat or equivalent
no longer results in a line-buffered output to a pipe, hence (BC), although
I don't expect anyone to be affected
Yuya Nishihara - Feb. 5, 2017, 5:37 a.m.
On Fri, 3 Feb 2017 15:15:06 -0800, Simon Farnsworth wrote:
> # HG changeset patch
> # User Simon Farnsworth <simonfar@fb.com>
> # Date 1486163427 28800
> #      Fri Feb 03 15:10:27 2017 -0800
> # Node ID 08b6e1bb20ca6bd1465a3d22bab49debfe776bae
> # Parent  1f51b4658f21bbb797e922d155c1046eddccf91d
> util: always force line buffered stdout when stdout is a tty (BC)

Queued, thanks.

> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -63,6 +63,18 @@
>  urlreq = pycompat.urlreq
>  xmlrpclib = pycompat.xmlrpclib
>  
> +def isatty(fp):
> +    try:
> +        return fp.isatty()
> +    except AttributeError:
> +        return False
> +
> +# glibc determines buffering on first write to stdout - if we replace a TTY
> +# destined stdout with a pipe destined stdout (e.g. pager), we want line
> +# buffering
> +if isatty(stdout):
> +    stdout = os.fdopen(stdout.fileno(), 'wb', 1)
> +
>  if pycompat.osname == 'nt':
>      from . import windows as platform
>      stdout = platform.winstdout(pycompat.stdout)

Fixed this in flight (otherwise stdout would be closed by GC):

 if pycompat.osname == 'nt':
     from . import windows as platform
-    stdout = platform.winstdout(pycompat.stdout)
+    stdout = platform.winstdout(stdout)

Patch

diff --git a/hgext/pager.py b/hgext/pager.py
--- a/hgext/pager.py
+++ b/hgext/pager.py
@@ -87,14 +87,10 @@ 
                              close_fds=util.closefds, stdin=subprocess.PIPE,
                              stdout=util.stdout, stderr=util.stderr)
 
-    # back up original file objects and descriptors
-    olduifout = ui.fout
-    oldstdout = util.stdout
+    # back up original file descriptors
     stdoutfd = os.dup(util.stdout.fileno())
     stderrfd = os.dup(util.stderr.fileno())
 
-    # create new line-buffered stdout so that output can show up immediately
-    ui.fout = util.stdout = newstdout = os.fdopen(util.stdout.fileno(), 'wb', 1)
     os.dup2(pager.stdin.fileno(), util.stdout.fileno())
     if ui._isatty(util.stderr):
         os.dup2(pager.stdin.fileno(), util.stderr.fileno())
@@ -103,15 +99,10 @@ 
     def killpager():
         if util.safehasattr(signal, "SIGINT"):
             signal.signal(signal.SIGINT, signal.SIG_IGN)
-        pager.stdin.close()
-        ui.fout = olduifout
-        util.stdout = oldstdout
-        # close new stdout while it's associated with pager; otherwise stdout
-        # fd would be closed when newstdout is deleted
-        newstdout.close()
-        # restore original fds: stdout is open again
+        # restore original fds, closing pager.stdin copies in the process
         os.dup2(stdoutfd, util.stdout.fileno())
         os.dup2(stderrfd, util.stderr.fileno())
+        pager.stdin.close()
         pager.wait()
 
 def uisetup(ui):
diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -63,6 +63,18 @@ 
 urlreq = pycompat.urlreq
 xmlrpclib = pycompat.xmlrpclib
 
+def isatty(fp):
+    try:
+        return fp.isatty()
+    except AttributeError:
+        return False
+
+# glibc determines buffering on first write to stdout - if we replace a TTY
+# destined stdout with a pipe destined stdout (e.g. pager), we want line
+# buffering
+if isatty(stdout):
+    stdout = os.fdopen(stdout.fileno(), 'wb', 1)
+
 if pycompat.osname == 'nt':
     from . import windows as platform
     stdout = platform.winstdout(pycompat.stdout)
@@ -2750,12 +2762,6 @@ 
     u.user = u.passwd = None
     return str(u)
 
-def isatty(fp):
-    try:
-        return fp.isatty()
-    except AttributeError:
-        return False
-
 timecount = unitcountfn(
     (1, 1e3, _('%.0f s')),
     (100, 1, _('%.1f s')),