[Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema

Eduardo Habkost posted 1 patch 5 years ago
Test asan passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190506213817.14344-1-ehabkost@redhat.com
tests/Makefile.include | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema
Posted by Eduardo Habkost 5 years ago
test-qapi.py doesn't force a specific encoding for stderr or
stdout, but the reference files used by check-qapi-schema are in
UTF-8.  This breaks check-qapi-schema under certain circumstances
(e.g. if using the C locale and Python < 3.7).

We need to make sure test-qapi.py always generate UTF-8 output
somehow.  On Python 3.7+ we can do it using
`sys.stdout.reconfigure(...)`, but we need a solution that works
with older Python versions.

Instead of trying a hack like reopening sys.stdout and
sys.stderr, we can just tell Python to use UTF-8 for I/O encoding
when running test-qapi.py.  Do it by setting PYTHONIOENCODING.

Reported-by: Thomas Huth <thuth@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/Makefile.include | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 7c8b9c84b2..af88ab6f8b 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -1103,7 +1103,7 @@ check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF)
 .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y))
 $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json
 	$(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts \
-		$(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \
+		PYTHONIOENCODING=utf-8 $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \
 		$^ >$*.test.out 2>$*.test.err; \
 		echo $$? >$*.test.exit, \
 		"TEST","$*.out")
-- 
2.18.0.rc1.1.g3f1ff2140


Re: [Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema
Posted by Alex Bennée 4 years, 12 months ago
Eduardo Habkost <ehabkost@redhat.com> writes:

> test-qapi.py doesn't force a specific encoding for stderr or
> stdout, but the reference files used by check-qapi-schema are in
> UTF-8.  This breaks check-qapi-schema under certain circumstances
> (e.g. if using the C locale and Python < 3.7).
>
> We need to make sure test-qapi.py always generate UTF-8 output
> somehow.  On Python 3.7+ we can do it using
> `sys.stdout.reconfigure(...)`, but we need a solution that works
> with older Python versions.
>
> Instead of trying a hack like reopening sys.stdout and
> sys.stderr, we can just tell Python to use UTF-8 for I/O encoding
> when running test-qapi.py.  Do it by setting PYTHONIOENCODING.
>
> Reported-by: Thomas Huth <thuth@redhat.com>
> Tested-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  tests/Makefile.include | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 7c8b9c84b2..af88ab6f8b 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -1103,7 +1103,7 @@ check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF)
>  .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y))
>  $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json
>  	$(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts \
> -		$(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \
> +		PYTHONIOENCODING=utf-8 $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \
>  		$^ >$*.test.out 2>$*.test.err; \
>  		echo $$? >$*.test.exit, \
>  		"TEST","$*.out")


--
Alex Bennée

Re: [Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema
Posted by Daniel P. Berrangé 4 years, 12 months ago
On Mon, May 06, 2019 at 06:38:17PM -0300, Eduardo Habkost wrote:
> test-qapi.py doesn't force a specific encoding for stderr or
> stdout, but the reference files used by check-qapi-schema are in
> UTF-8.  This breaks check-qapi-schema under certain circumstances
> (e.g. if using the C locale and Python < 3.7).
> 
> We need to make sure test-qapi.py always generate UTF-8 output
> somehow.  On Python 3.7+ we can do it using
> `sys.stdout.reconfigure(...)`, but we need a solution that works
> with older Python versions.
> 
> Instead of trying a hack like reopening sys.stdout and
> sys.stderr, we can just tell Python to use UTF-8 for I/O encoding
> when running test-qapi.py.  Do it by setting PYTHONIOENCODING.
> 
> Reported-by: Thomas Huth <thuth@redhat.com>
> Tested-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  tests/Makefile.include | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 7c8b9c84b2..af88ab6f8b 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -1103,7 +1103,7 @@ check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF)
>  .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y))
>  $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json
>  	$(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts \
> -		$(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \
> +		PYTHONIOENCODING=utf-8 $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \

I see PYTHONIOENCODING exists since 2.6 which is nice.

How about we actually change $(PYTHON) so that it always includes
PYTHONIOENCODING=utf-8 ?

That way we avoid continuing to play whack-a-mole with more utf-8
bugs in future.

It would also let us revert this:

  commit de685ae5e9a4b523513033bd6cadc8187a227170
  Author: Markus Armbruster <armbru@redhat.com>
  Date:   Mon Jun 18 19:59:57 2018 +0200

    qapi: Open files with encoding='utf-8'

which had to provide separate logic for py2 vs py3 :-(

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema
Posted by Eduardo Habkost 4 years, 12 months ago
On Tue, May 07, 2019 at 03:13:45PM +0100, Daniel P. Berrangé wrote:
> On Mon, May 06, 2019 at 06:38:17PM -0300, Eduardo Habkost wrote:
> > test-qapi.py doesn't force a specific encoding for stderr or
> > stdout, but the reference files used by check-qapi-schema are in
> > UTF-8.  This breaks check-qapi-schema under certain circumstances
> > (e.g. if using the C locale and Python < 3.7).
> > 
> > We need to make sure test-qapi.py always generate UTF-8 output
> > somehow.  On Python 3.7+ we can do it using
> > `sys.stdout.reconfigure(...)`, but we need a solution that works
> > with older Python versions.
> > 
> > Instead of trying a hack like reopening sys.stdout and
> > sys.stderr, we can just tell Python to use UTF-8 for I/O encoding
> > when running test-qapi.py.  Do it by setting PYTHONIOENCODING.
> > 
> > Reported-by: Thomas Huth <thuth@redhat.com>
> > Tested-by: Thomas Huth <thuth@redhat.com>
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  tests/Makefile.include | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index 7c8b9c84b2..af88ab6f8b 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -1103,7 +1103,7 @@ check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF)
> >  .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y))
> >  $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json
> >  	$(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts \
> > -		$(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \
> > +		PYTHONIOENCODING=utf-8 $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \
> 
> I see PYTHONIOENCODING exists since 2.6 which is nice.
> 
> How about we actually change $(PYTHON) so that it always includes
> PYTHONIOENCODING=utf-8 ?
> 
> That way we avoid continuing to play whack-a-mole with more utf-8
> bugs in future.
> 
> It would also let us revert this:
> 
>   commit de685ae5e9a4b523513033bd6cadc8187a227170
>   Author: Markus Armbruster <armbru@redhat.com>
>   Date:   Mon Jun 18 19:59:57 2018 +0200
> 
>     qapi: Open files with encoding='utf-8'
> 
> which had to provide separate logic for py2 vs py3 :-(

Not every Python script in the QEMU tree is run by our makefiles
and scripts using $(PYTHON).  We need to ensure our scripts and
modules won't break when run directly from the command line, too.
Setting PYTHONIOENCODING everywhere would just hide these bugs
from us.

-- 
Eduardo

Re: [Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema
Posted by Markus Armbruster 4 years, 12 months ago
Eduardo Habkost <ehabkost@redhat.com> writes:

> On Tue, May 07, 2019 at 03:13:45PM +0100, Daniel P. Berrangé wrote:
>> On Mon, May 06, 2019 at 06:38:17PM -0300, Eduardo Habkost wrote:
>> > test-qapi.py doesn't force a specific encoding for stderr or
>> > stdout, but the reference files used by check-qapi-schema are in
>> > UTF-8.  This breaks check-qapi-schema under certain circumstances
>> > (e.g. if using the C locale and Python < 3.7).
>> > 
>> > We need to make sure test-qapi.py always generate UTF-8 output
>> > somehow.  On Python 3.7+ we can do it using
>> > `sys.stdout.reconfigure(...)`, but we need a solution that works
>> > with older Python versions.
>> > 
>> > Instead of trying a hack like reopening sys.stdout and
>> > sys.stderr, we can just tell Python to use UTF-8 for I/O encoding
>> > when running test-qapi.py.  Do it by setting PYTHONIOENCODING.
>> > 
>> > Reported-by: Thomas Huth <thuth@redhat.com>
>> > Tested-by: Thomas Huth <thuth@redhat.com>
>> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> > ---
>> >  tests/Makefile.include | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > 
>> > diff --git a/tests/Makefile.include b/tests/Makefile.include
>> > index 7c8b9c84b2..af88ab6f8b 100644
>> > --- a/tests/Makefile.include
>> > +++ b/tests/Makefile.include
>> > @@ -1103,7 +1103,7 @@ check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF)
>> >  .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y))
>> >  $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json
>> >  	$(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts \
>> > -		$(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \
>> > +		PYTHONIOENCODING=utf-8 $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \
>> 
>> I see PYTHONIOENCODING exists since 2.6 which is nice.
>> 
>> How about we actually change $(PYTHON) so that it always includes
>> PYTHONIOENCODING=utf-8 ?
>> 
>> That way we avoid continuing to play whack-a-mole with more utf-8
>> bugs in future.
>> 
>> It would also let us revert this:
>> 
>>   commit de685ae5e9a4b523513033bd6cadc8187a227170
>>   Author: Markus Armbruster <armbru@redhat.com>
>>   Date:   Mon Jun 18 19:59:57 2018 +0200
>> 
>>     qapi: Open files with encoding='utf-8'
>> 
>> which had to provide separate logic for py2 vs py3 :-(

The separate logic will soon be history.  I'd welcome getting rid of the
remainder anyway.

> Not every Python script in the QEMU tree is run by our makefiles
> and scripts using $(PYTHON).  We need to ensure our scripts and
> modules won't break when run directly from the command line, too.
> Setting PYTHONIOENCODING everywhere would just hide these bugs
> from us.

I agree for Python scripts that are meant to be run that way (assuming
such scripts exist).  For all the others (including all the QAPI-related
scripts), I'd be quite fine with

1. Our build system runs all Python scripts with the
PYTHONIOENCODING=utf-8

2. If you run a Python script yourself, you get to specify the
PYTHONIOENCODING=utf-8, or use a suitable locale.  Enabling UTF-8 mode
with PYTHONUTF8=1 or -X utf8 could also work.

Re: [Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema
Posted by Eduardo Habkost 4 years, 12 months ago
On Wed, May 08, 2019 at 03:04:43PM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > On Tue, May 07, 2019 at 03:13:45PM +0100, Daniel P. Berrangé wrote:
> >> On Mon, May 06, 2019 at 06:38:17PM -0300, Eduardo Habkost wrote:
> >> > test-qapi.py doesn't force a specific encoding for stderr or
> >> > stdout, but the reference files used by check-qapi-schema are in
> >> > UTF-8.  This breaks check-qapi-schema under certain circumstances
> >> > (e.g. if using the C locale and Python < 3.7).
> >> > 
> >> > We need to make sure test-qapi.py always generate UTF-8 output
> >> > somehow.  On Python 3.7+ we can do it using
> >> > `sys.stdout.reconfigure(...)`, but we need a solution that works
> >> > with older Python versions.
> >> > 
> >> > Instead of trying a hack like reopening sys.stdout and
> >> > sys.stderr, we can just tell Python to use UTF-8 for I/O encoding
> >> > when running test-qapi.py.  Do it by setting PYTHONIOENCODING.
> >> > 
> >> > Reported-by: Thomas Huth <thuth@redhat.com>
> >> > Tested-by: Thomas Huth <thuth@redhat.com>
> >> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> >> > ---
> >> >  tests/Makefile.include | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> > 
> >> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> >> > index 7c8b9c84b2..af88ab6f8b 100644
> >> > --- a/tests/Makefile.include
> >> > +++ b/tests/Makefile.include
> >> > @@ -1103,7 +1103,7 @@ check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF)
> >> >  .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y))
> >> >  $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json
> >> >  	$(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts \
> >> > -		$(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \
> >> > +		PYTHONIOENCODING=utf-8 $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \
> >> 
> >> I see PYTHONIOENCODING exists since 2.6 which is nice.
> >> 
> >> How about we actually change $(PYTHON) so that it always includes
> >> PYTHONIOENCODING=utf-8 ?
> >> 
> >> That way we avoid continuing to play whack-a-mole with more utf-8
> >> bugs in future.
> >> 
> >> It would also let us revert this:
> >> 
> >>   commit de685ae5e9a4b523513033bd6cadc8187a227170
> >>   Author: Markus Armbruster <armbru@redhat.com>
> >>   Date:   Mon Jun 18 19:59:57 2018 +0200
> >> 
> >>     qapi: Open files with encoding='utf-8'
> >> 
> >> which had to provide separate logic for py2 vs py3 :-(
> 
> The separate logic will soon be history.  I'd welcome getting rid of the
> remainder anyway.

Which remainder?  Do you mean the encoding='utf-8' arguments to
open()?

> 
> > Not every Python script in the QEMU tree is run by our makefiles
> > and scripts using $(PYTHON).  We need to ensure our scripts and
> > modules won't break when run directly from the command line, too.
> > Setting PYTHONIOENCODING everywhere would just hide these bugs
> > from us.
> 
> I agree for Python scripts that are meant to be run that way (assuming
> such scripts exist).  [...]

All scripts inside ./scripts are meant to be run directly from
the command line, aren't they?


>                [...]  For all the others (including all the QAPI-related
> scripts), I'd be quite fine with
> 
> 1. Our build system runs all Python scripts with the
> PYTHONIOENCODING=utf-8
> 
> 2. If you run a Python script yourself, you get to specify the
> PYTHONIOENCODING=utf-8, or use a suitable locale.  Enabling UTF-8 mode
> with PYTHONUTF8=1 or -X utf8 could also work.

I'm OK if we don't actively try to fix those bugs and just expect
people to set PYTHONIOENCODING.  But I don't think we should
reject patches that make the Python code work with non-utf8
locales if it's an easy fix.

-- 
Eduardo

Re: [Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema
Posted by Markus Armbruster 4 years, 12 months ago
Eduardo Habkost <ehabkost@redhat.com> writes:

> On Wed, May 08, 2019 at 03:04:43PM +0200, Markus Armbruster wrote:
>> Eduardo Habkost <ehabkost@redhat.com> writes:
>> 
>> > On Tue, May 07, 2019 at 03:13:45PM +0100, Daniel P. Berrangé wrote:
>> >> On Mon, May 06, 2019 at 06:38:17PM -0300, Eduardo Habkost wrote:
>> >> > test-qapi.py doesn't force a specific encoding for stderr or
>> >> > stdout, but the reference files used by check-qapi-schema are in
>> >> > UTF-8.  This breaks check-qapi-schema under certain circumstances
>> >> > (e.g. if using the C locale and Python < 3.7).
>> >> > 
>> >> > We need to make sure test-qapi.py always generate UTF-8 output
>> >> > somehow.  On Python 3.7+ we can do it using
>> >> > `sys.stdout.reconfigure(...)`, but we need a solution that works
>> >> > with older Python versions.
>> >> > 
>> >> > Instead of trying a hack like reopening sys.stdout and
>> >> > sys.stderr, we can just tell Python to use UTF-8 for I/O encoding
>> >> > when running test-qapi.py.  Do it by setting PYTHONIOENCODING.
>> >> > 
>> >> > Reported-by: Thomas Huth <thuth@redhat.com>
>> >> > Tested-by: Thomas Huth <thuth@redhat.com>
>> >> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> >> > ---
>> >> >  tests/Makefile.include | 2 +-
>> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >> > 
>> >> > diff --git a/tests/Makefile.include b/tests/Makefile.include
>> >> > index 7c8b9c84b2..af88ab6f8b 100644
>> >> > --- a/tests/Makefile.include
>> >> > +++ b/tests/Makefile.include
>> >> > @@ -1103,7 +1103,7 @@ check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF)
>> >> >  .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y))
>> >> >  $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json
>> >> >  	$(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts \
>> >> > -		$(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \
>> >> > +		PYTHONIOENCODING=utf-8 $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \
>> >> 
>> >> I see PYTHONIOENCODING exists since 2.6 which is nice.
>> >> 
>> >> How about we actually change $(PYTHON) so that it always includes
>> >> PYTHONIOENCODING=utf-8 ?
>> >> 
>> >> That way we avoid continuing to play whack-a-mole with more utf-8
>> >> bugs in future.
>> >> 
>> >> It would also let us revert this:
>> >> 
>> >>   commit de685ae5e9a4b523513033bd6cadc8187a227170
>> >>   Author: Markus Armbruster <armbru@redhat.com>
>> >>   Date:   Mon Jun 18 19:59:57 2018 +0200
>> >> 
>> >>     qapi: Open files with encoding='utf-8'
>> >> 
>> >> which had to provide separate logic for py2 vs py3 :-(
>> 
>> The separate logic will soon be history.  I'd welcome getting rid of the
>> remainder anyway.
>
> Which remainder?  Do you mean the encoding='utf-8' arguments to
> open()?

I'd welcome a revert the whole commit.

>> > Not every Python script in the QEMU tree is run by our makefiles
>> > and scripts using $(PYTHON).  We need to ensure our scripts and
>> > modules won't break when run directly from the command line, too.
>> > Setting PYTHONIOENCODING everywhere would just hide these bugs
>> > from us.
>> 
>> I agree for Python scripts that are meant to be run that way (assuming
>> such scripts exist).  [...]
>
> All scripts inside ./scripts are meant to be run directly from
> the command line, aren't they?

Counter-example: you're welcome to run scripts/qapi-gen.py by hand for
whatever purpose, but if you mess up your build that way, you're on your
own.

>>                [...]  For all the others (including all the QAPI-related
>> scripts), I'd be quite fine with
>> 
>> 1. Our build system runs all Python scripts with the
>> PYTHONIOENCODING=utf-8
>> 
>> 2. If you run a Python script yourself, you get to specify the
>> PYTHONIOENCODING=utf-8, or use a suitable locale.  Enabling UTF-8 mode
>> with PYTHONUTF8=1 or -X utf8 could also work.
>
> I'm OK if we don't actively try to fix those bugs and just expect
> people to set PYTHONIOENCODING.  But I don't think we should
> reject patches that make the Python code work with non-utf8
> locales if it's an easy fix.

I'm not going to interfere with easy fixes to code I don't maintain :)

Re: [Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema
Posted by Philippe Mathieu-Daudé 4 years, 12 months ago
On 5/6/19 11:38 PM, Eduardo Habkost wrote:
> test-qapi.py doesn't force a specific encoding for stderr or
> stdout, but the reference files used by check-qapi-schema are in
> UTF-8.  This breaks check-qapi-schema under certain circumstances
> (e.g. if using the C locale and Python < 3.7).
> 
> We need to make sure test-qapi.py always generate UTF-8 output
> somehow.  On Python 3.7+ we can do it using
> `sys.stdout.reconfigure(...)`, but we need a solution that works
> with older Python versions.
> 
> Instead of trying a hack like reopening sys.stdout and
> sys.stderr, we can just tell Python to use UTF-8 for I/O encoding
> when running test-qapi.py.  Do it by setting PYTHONIOENCODING.
> 
> Reported-by: Thomas Huth <thuth@redhat.com>
> Tested-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  tests/Makefile.include | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 7c8b9c84b2..af88ab6f8b 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -1103,7 +1103,7 @@ check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF)
>  .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y))
>  $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json
>  	$(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts \
> -		$(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \
> +		PYTHONIOENCODING=utf-8 $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \
>  		$^ >$*.test.out 2>$*.test.err; \
>  		echo $$? >$*.test.exit, \
>  		"TEST","$*.out")
>