Patchwork [6,of,6] procutil: make stdout line-buffered on Windows if connected to TTY

login
register
mail settings
Submitter Manuel Jacob
Date July 5, 2020, 2:51 a.m.
Message ID <f330f7c409a975aa8dfa.1593917502@tmp.fritz.box>
Download mbox | patch
Permalink /patch/46624/
State Accepted
Headers show

Comments

Manuel Jacob - July 5, 2020, 2:51 a.m.
# HG changeset patch
# User Manuel Jacob <me@manueljacob.de>
# Date 1593855699 -7200
#      Sat Jul 04 11:41:39 2020 +0200
# Node ID f330f7c409a975aa8dfad6161d798192ac801ef7
# Parent  7fb9114235241b6d4354b22e2ba08138bde58642
# EXP-Topic stdio
procutil: make stdout line-buffered on Windows if connected to TTY

Windows doesn’t support line buffering. Previously, we worked around that by
setting the stream unbuffered. Instead, we can use our own line buffering we
already use on Python 3.
Yuya Nishihara - July 6, 2020, 11:26 a.m.
On Sun, 05 Jul 2020 04:51:42 +0200, Manuel Jacob wrote:
> # HG changeset patch
> # User Manuel Jacob <me@manueljacob.de>
> # Date 1593855699 -7200
> #      Sat Jul 04 11:41:39 2020 +0200
> # Node ID f330f7c409a975aa8dfad6161d798192ac801ef7
> # Parent  7fb9114235241b6d4354b22e2ba08138bde58642
> # EXP-Topic stdio
> procutil: make stdout line-buffered on Windows if connected to TTY
> 
> Windows doesn’t support line buffering. Previously, we worked around that by
> setting the stream unbuffered. Instead, we can use our own line buffering we
> already use on Python 3.
> 
> diff --git a/mercurial/utils/procutil.py b/mercurial/utils/procutil.py
> --- a/mercurial/utils/procutil.py
> +++ b/mercurial/utils/procutil.py
> @@ -84,23 +84,21 @@
>  stdin = pycompat.stdin
>  stdout = pycompat.stdout
>  
> +if pycompat.iswindows:
> +    stdout = platform.winstdout(stdout)
> +
>  # 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 (or unbuffered, on Windows)
> +# buffering.
>  if isatty(stdout):

Not directly related to this patch, but NUL device is a tty on Windows. :)

https://stackoverflow.com/q/3648711

The patch itself looks good to me.
Manuel Jacob - July 6, 2020, 11:58 a.m.
On 2020-07-06 13:26, Yuya Nishihara wrote:
> On Sun, 05 Jul 2020 04:51:42 +0200, Manuel Jacob wrote:
>> # HG changeset patch
>> # User Manuel Jacob <me@manueljacob.de>
>> # Date 1593855699 -7200
>> #      Sat Jul 04 11:41:39 2020 +0200
>> # Node ID f330f7c409a975aa8dfad6161d798192ac801ef7
>> # Parent  7fb9114235241b6d4354b22e2ba08138bde58642
>> # EXP-Topic stdio
>> procutil: make stdout line-buffered on Windows if connected to TTY
>> 
>> Windows doesn’t support line buffering. Previously, we worked around 
>> that by
>> setting the stream unbuffered. Instead, we can use our own line 
>> buffering we
>> already use on Python 3.
>> 
>> diff --git a/mercurial/utils/procutil.py b/mercurial/utils/procutil.py
>> --- a/mercurial/utils/procutil.py
>> +++ b/mercurial/utils/procutil.py
>> @@ -84,23 +84,21 @@
>>  stdin = pycompat.stdin
>>  stdout = pycompat.stdout
>> 
>> +if pycompat.iswindows:
>> +    stdout = platform.winstdout(stdout)
>> +
>>  # 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 (or unbuffered, on Windows)
>> +# buffering.
>>  if isatty(stdout):
> 
> Not directly related to this patch, but NUL device is a tty on Windows. 
> :)

I could see funny ways of working around that there’s no easy way to 
create PTYs on Windows (this is why some tests are skipped). Something 
like duping stdout to NUL, importing procutil, duping stdout back to the 
original stream. Is it worth trying or is it too obscure?

> https://stackoverflow.com/q/3648711
> 
> The patch itself looks good to me.
Yuya Nishihara - July 6, 2020, 12:59 p.m.
On Mon, 06 Jul 2020 13:58:54 +0200, Manuel Jacob wrote:
> On 2020-07-06 13:26, Yuya Nishihara wrote:
> > On Sun, 05 Jul 2020 04:51:42 +0200, Manuel Jacob wrote:
> >> # HG changeset patch
> >> # User Manuel Jacob <me@manueljacob.de>
> >> # Date 1593855699 -7200
> >> #      Sat Jul 04 11:41:39 2020 +0200
> >> # Node ID f330f7c409a975aa8dfad6161d798192ac801ef7
> >> # Parent  7fb9114235241b6d4354b22e2ba08138bde58642
> >> # EXP-Topic stdio
> >> procutil: make stdout line-buffered on Windows if connected to TTY
> >> 
> >> Windows doesn’t support line buffering. Previously, we worked around 
> >> that by
> >> setting the stream unbuffered. Instead, we can use our own line 
> >> buffering we
> >> already use on Python 3.
> >> 
> >> diff --git a/mercurial/utils/procutil.py b/mercurial/utils/procutil.py
> >> --- a/mercurial/utils/procutil.py
> >> +++ b/mercurial/utils/procutil.py
> >> @@ -84,23 +84,21 @@
> >>  stdin = pycompat.stdin
> >>  stdout = pycompat.stdout
> >> 
> >> +if pycompat.iswindows:
> >> +    stdout = platform.winstdout(stdout)
> >> +
> >>  # 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 (or unbuffered, on Windows)
> >> +# buffering.
> >>  if isatty(stdout):
> > 
> > Not directly related to this patch, but NUL device is a tty on Windows. 
> > :)
> 
> I could see funny ways of working around that there’s no easy way to 
> create PTYs on Windows (this is why some tests are skipped). Something 
> like duping stdout to NUL, importing procutil, duping stdout back to the 
> original stream. Is it worth trying or is it too obscure?

Nah. I just pointed out the weird isatty() behavior as I remember the hack
in TortoiseHg.

Patch

diff --git a/mercurial/utils/procutil.py b/mercurial/utils/procutil.py
--- a/mercurial/utils/procutil.py
+++ b/mercurial/utils/procutil.py
@@ -84,23 +84,21 @@ 
 stdin = pycompat.stdin
 stdout = pycompat.stdout
 
+if pycompat.iswindows:
+    stdout = platform.winstdout(stdout)
+
 # 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 (or unbuffered, on Windows)
+# buffering.
 if isatty(stdout):
-    if pycompat.iswindows:
-        # Windows doesn't support line buffering
-        stdout = os.fdopen(stdout.fileno(), 'wb', 0)
-    elif pycompat.ispy3:
+    if pycompat.ispy3 or pycompat.iswindows:
         # On Python 3, buffered binary streams can't be set line-buffered.
+        # On Python 2, Windows doesn't support line buffering.
         # Therefore we have a wrapper that implements line buffering.
         stdout = make_line_buffered(stdout)
     else:
         stdout = os.fdopen(stdout.fileno(), 'wb', 1)
 
-if pycompat.iswindows:
-    stdout = platform.winstdout(stdout)
-
 
 findexe = platform.findexe
 _gethgcmd = platform.gethgcmd
diff --git a/tests/test-stdio.py b/tests/test-stdio.py
--- a/tests/test-stdio.py
+++ b/tests/test-stdio.py
@@ -88,7 +88,6 @@ 
     def test_stdout_ptys_unbuffered(self):
         self._test(_ptys, UNBUFFERED, python_args=['-u'])
 
-    # On Windows, test_stdout_ptys wouldn't pass, but it's skipped anyway.
     if not pycompat.ispy3 and not pycompat.iswindows:
         # On Python 2 on non-Windows, we manually open stdout in line-buffered
         # mode if connected to a TTY. We should check if Python was configured