Patchwork util: flush stderr explicitly after using warnings.warn()

login
register
mail settings
Submitter Pulkit Goyal
Date June 19, 2020, 3:12 p.m.
Message ID <dc3c03ad1dc77fba867a.1592579540@workspace>
Download mbox | patch
Permalink /patch/46534/
State Accepted
Headers show

Comments

Pulkit Goyal - June 19, 2020, 3:12 p.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1592579534 -19800
#      Fri Jun 19 20:42:14 2020 +0530
# Node ID dc3c03ad1dc77fba867a7381249103081609a95e
# Parent  1bd88e1bd312b02fa3cba45fccb147f316b372eb
# EXP-Topic chg-test
util: flush stderr explicitly after using warnings.warn()

Due to some unknown reasons, when using chg with python3, the warnings.warn()
output is not flushed.

Fixes test-devel-warnings.t on py3 with chg.
Manuel Jacob - June 19, 2020, 7:16 p.m.
On 2020-06-19 17:12, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1592579534 -19800
> #      Fri Jun 19 20:42:14 2020 +0530
> # Node ID dc3c03ad1dc77fba867a7381249103081609a95e
> # Parent  1bd88e1bd312b02fa3cba45fccb147f316b372eb
> # EXP-Topic chg-test
> util: flush stderr explicitly after using warnings.warn()
> 
> Due to some unknown reasons, when using chg with python3, the 
> warnings.warn()
> output is not flushed.

The warnings module won’t flush output: 
https://github.com/python/cpython/blob/v3.8.3/Lib/warnings.py#L30 This 
is probably worth fixing on its own, but it’s outside of Mercurial.

Python 2 uses the libc streams. Python 3 has its own streams and 
initialization.

On many systems, libc sets stderr to unbuffered. (I couldn’t verify, but 
I think it is not necessarily the case on Windows. Maybe the warning is 
not even flushed without chg on Python 2 on Windows.) Python 3.8 sets 
sys.stderr fully buffered if not interactive and not explicitly 
configured unbuffered (source: 
https://docs.python.org/3.8/library/sys.html#sys.stderr). Python 3.9 
sets sys.stderr always line-buffered if not explicitly configured 
unbuffered (source: 
https://docs.python.org/3.9/library/sys.html#sys.stderr).

I think there’s a problem for both stdout and stderr. Glibc decides 
buffering on the first write to the stream. Python 3 decides buffering 
at interpreter startup. To solve the problem generally, sys.stdout and 
sys.stderr need to be replaced after each fork of chgserver.

> Fixes test-devel-warnings.t on py3 with chg.
> 
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -204,6 +204,8 @@ def nouideprecwarn(msg, version, stackle
>              b" update your code.)"
>          ) % version
>          warnings.warn(pycompat.sysstr(msg), DeprecationWarning, 
> stacklevel + 1)
> +        # on python 3 with chg, we will need to explicitly flush the 
> output
> +        sys.stderr.flush()
> 
> 
>  DIGESTS = {
> 
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Manuel Jacob - June 19, 2020, 7:34 p.m.
One thing to add:

Currently, chgcmdserver.attachio() duplicates some logic from procutil, 
but not completely. chgcmdserver.attachio() misses e.g. Windows fixes 
and Python 3 line buffering workarounds (f9734b2d59cc, which fixed a 
previous attempt in 227ba1afcb65).

It would be much better if both places shared the same code.

On 2020-06-19 21:16, Manuel Jacob wrote:
> On 2020-06-19 17:12, Pulkit Goyal wrote:
>> # HG changeset patch
>> # User Pulkit Goyal <7895pulkit@gmail.com>
>> # Date 1592579534 -19800
>> #      Fri Jun 19 20:42:14 2020 +0530
>> # Node ID dc3c03ad1dc77fba867a7381249103081609a95e
>> # Parent  1bd88e1bd312b02fa3cba45fccb147f316b372eb
>> # EXP-Topic chg-test
>> util: flush stderr explicitly after using warnings.warn()
>> 
>> Due to some unknown reasons, when using chg with python3, the 
>> warnings.warn()
>> output is not flushed.
> 
> The warnings module won’t flush output:
> https://github.com/python/cpython/blob/v3.8.3/Lib/warnings.py#L30 This
> is probably worth fixing on its own, but it’s outside of Mercurial.
> 
> Python 2 uses the libc streams. Python 3 has its own streams and 
> initialization.
> 
> On many systems, libc sets stderr to unbuffered. (I couldn’t verify,
> but I think it is not necessarily the case on Windows. Maybe the
> warning is not even flushed without chg on Python 2 on Windows.)
> Python 3.8 sets sys.stderr fully buffered if not interactive and not
> explicitly configured unbuffered (source:
> https://docs.python.org/3.8/library/sys.html#sys.stderr). Python 3.9
> sets sys.stderr always line-buffered if not explicitly configured
> unbuffered (source:
> https://docs.python.org/3.9/library/sys.html#sys.stderr).
> 
> I think there’s a problem for both stdout and stderr. Glibc decides
> buffering on the first write to the stream. Python 3 decides buffering
> at interpreter startup. To solve the problem generally, sys.stdout and
> sys.stderr need to be replaced after each fork of chgserver.
> 
>> Fixes test-devel-warnings.t on py3 with chg.
>> 
>> diff --git a/mercurial/util.py b/mercurial/util.py
>> --- a/mercurial/util.py
>> +++ b/mercurial/util.py
>> @@ -204,6 +204,8 @@ def nouideprecwarn(msg, version, stackle
>>              b" update your code.)"
>>          ) % version
>>          warnings.warn(pycompat.sysstr(msg), DeprecationWarning, 
>> stacklevel + 1)
>> +        # on python 3 with chg, we will need to explicitly flush the 
>> output
>> +        sys.stderr.flush()
>> 
>> 
>>  DIGESTS = {
>> 
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - June 20, 2020, 1:58 a.m.
On Fri, 19 Jun 2020 20:42:20 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1592579534 -19800
> #      Fri Jun 19 20:42:14 2020 +0530
> # Node ID dc3c03ad1dc77fba867a7381249103081609a95e
> # Parent  1bd88e1bd312b02fa3cba45fccb147f316b372eb
> # EXP-Topic chg-test
> util: flush stderr explicitly after using warnings.warn()
> 
> Due to some unknown reasons, when using chg with python3, the warnings.warn()
> output is not flushed.
> 
> Fixes test-devel-warnings.t on py3 with chg.
> 
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -204,6 +204,8 @@ def nouideprecwarn(msg, version, stackle
>              b" update your code.)"
>          ) % version
>          warnings.warn(pycompat.sysstr(msg), DeprecationWarning, stacklevel + 1)
> +        # on python 3 with chg, we will need to explicitly flush the output
> +        sys.stderr.flush()

Queued, thanks.

It's generally good to flush the TextIO as we do use the underlying
BytesIO extensively.

Patch

diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -204,6 +204,8 @@  def nouideprecwarn(msg, version, stackle
             b" update your code.)"
         ) % version
         warnings.warn(pycompat.sysstr(msg), DeprecationWarning, stacklevel + 1)
+        # on python 3 with chg, we will need to explicitly flush the output
+        sys.stderr.flush()
 
 
 DIGESTS = {