Patchwork [1,of,2] py3: keep stdout as defined by pycompat in procutil

login
register
mail settings
Submitter Denis Laxalde
Date Oct. 10, 2019, 2:16 p.m.
Message ID <63da27176ab9d0ac5dd5.1570716985@steppe.local>
Download mbox | patch
Permalink /patch/42189/
State Accepted
Headers show

Comments

Denis Laxalde - Oct. 10, 2019, 2:16 p.m.
# HG changeset patch
# User Denis Laxalde <denis.laxalde@logilab.fr>
# Date 1570716002 -7200
#      Thu Oct 10 16:00:02 2019 +0200
# Node ID 63da27176ab9d0ac5dd58e0a6037469ce4bf5435
# Parent  b0238700551593257f293fd838f2ac030d00cede
py3: keep stdout as defined by pycompat in procutil

According to https://docs.python.org/3/library/functions.html#open, it's
not possible to use 1 as buffering argument value in binary mode. This
is probably why line buffering does not work well in python3.

On the other hand, sys.stdout.buffer appears to be line-buffered already
on python3. So by not replacing it, there should be no behavior change.

This fixes buffering issue in "hg email" (confirmation prompt shown
before information to be confirmed).
Yuya Nishihara - Oct. 10, 2019, 2:53 p.m.
On Thu, 10 Oct 2019 16:16:25 +0200, Denis Laxalde wrote:
> # HG changeset patch
> # User Denis Laxalde <denis.laxalde@logilab.fr>
> # Date 1570716002 -7200
> #      Thu Oct 10 16:00:02 2019 +0200
> # Node ID 63da27176ab9d0ac5dd58e0a6037469ce4bf5435
> # Parent  b0238700551593257f293fd838f2ac030d00cede
> py3: keep stdout as defined by pycompat in procutil
> 
> According to https://docs.python.org/3/library/functions.html#open, it's
> not possible to use 1 as buffering argument value in binary mode. This
> is probably why line buffering does not work well in python3.
> 
> On the other hand, sys.stdout.buffer appears to be line-buffered already
> on python3. So by not replacing it, there should be no behavior change.

Can you check if it's still true under pager?

  (for example)
  $ hg log -k mpm
  ... will print f2fe7cf4ebb6 immediately, and more changesets with
  ... some delay.
  ...
  ... to test glibc behavior on py2, no output should be made before
  ... pager is attached. I have no idea about py3.

stdout is also line-buffered by default on Python 2, but it isn't under
pager because the buffering mode can be determined after the pager is
attached.
Denis Laxalde - Oct. 10, 2019, 3:16 p.m.
Yuya Nishihara a écrit :
> On Thu, 10 Oct 2019 16:16:25 +0200, Denis Laxalde wrote:
> > # HG changeset patch
> > # User Denis Laxalde <denis.laxalde@logilab.fr>
> > # Date 1570716002 -7200
> > #      Thu Oct 10 16:00:02 2019 +0200
> > # Node ID 63da27176ab9d0ac5dd58e0a6037469ce4bf5435
> > # Parent  b0238700551593257f293fd838f2ac030d00cede
> > py3: keep stdout as defined by pycompat in procutil
> > 
> > According to https://docs.python.org/3/library/functions.html#open, it's
> > not possible to use 1 as buffering argument value in binary mode. This
> > is probably why line buffering does not work well in python3.
> > 
> > On the other hand, sys.stdout.buffer appears to be line-buffered already
> > on python3. So by not replacing it, there should be no behavior change.
> 
> Can you check if it's still true under pager?
> 
>   (for example)
>   $ hg log -k mpm
>   ... will print f2fe7cf4ebb6 immediately, and more changesets with
>   ... some delay.

Yes, that's what I see. The same happens with py3 and this patch as with
5.1.1/py2.

>   ... to test glibc behavior on py2, no output should be made before
>   ... pager is attached. I have no idea about py3.
> 
> stdout is also line-buffered by default on Python 2, but it isn't under
> pager because the buffering mode can be determined after the pager is
> attached.
Yuya Nishihara - Oct. 10, 2019, 3:27 p.m.
On Thu, 10 Oct 2019 17:16:50 +0200, Denis Laxalde wrote:
> Yuya Nishihara a écrit :
> > On Thu, 10 Oct 2019 16:16:25 +0200, Denis Laxalde wrote:
> > > # HG changeset patch
> > > # User Denis Laxalde <denis.laxalde@logilab.fr>
> > > # Date 1570716002 -7200
> > > #      Thu Oct 10 16:00:02 2019 +0200
> > > # Node ID 63da27176ab9d0ac5dd58e0a6037469ce4bf5435
> > > # Parent  b0238700551593257f293fd838f2ac030d00cede
> > > py3: keep stdout as defined by pycompat in procutil
> > > 
> > > According to https://docs.python.org/3/library/functions.html#open, it's
> > > not possible to use 1 as buffering argument value in binary mode. This
> > > is probably why line buffering does not work well in python3.
> > > 
> > > On the other hand, sys.stdout.buffer appears to be line-buffered already
> > > on python3. So by not replacing it, there should be no behavior change.
> > 
> > Can you check if it's still true under pager?
> > 
> >   (for example)
> >   $ hg log -k mpm
> >   ... will print f2fe7cf4ebb6 immediately, and more changesets with
> >   ... some delay.
> 
> Yes, that's what I see. The same happens with py3 and this patch as with
> 5.1.1/py2.

Queued, thanks.

Maybe we can disable the stdout substitution at all, i.e.
"if not ispy3 and isatty(stdout):", but I don't have Windows machine to
test the behavior.

Patch

diff --git a/mercurial/utils/procutil.py b/mercurial/utils/procutil.py
index e779dc4..124c93e 100644
--- a/mercurial/utils/procutil.py
+++ b/mercurial/utils/procutil.py
@@ -53,7 +53,9 @@  if isatty(stdout):
     if pycompat.iswindows:
         # Windows doesn't support line buffering
         stdout = os.fdopen(stdout.fileno(), r'wb', 0)
-    else:
+    elif not pycompat.ispy3:
+        # on Python 3, stdout (sys.stdout.buffer) is already line buffered and
+        # buffering=1 is not handled in binary mode
         stdout = os.fdopen(stdout.fileno(), r'wb', 1)
 
 if pycompat.iswindows: