* [Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema
@ 2019-05-06 21:38 Eduardo Habkost
2019-05-07 5:21 ` Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Eduardo Habkost @ 2019-05-06 21:38 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Michael Roth, Markus Armbruster, Cleber Rosa,
Philippe Mathieu-Daudé
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema
2019-05-06 21:38 [Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema Eduardo Habkost
@ 2019-05-07 5:21 ` Philippe Mathieu-Daudé
2019-05-07 14:13 ` Daniel P. Berrangé
2019-05-08 9:19 ` Alex Bennée
2 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-07 5:21 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
Cc: Michael Roth, Thomas Huth, Markus Armbruster, Cleber Rosa
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")
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema
2019-05-06 21:38 [Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema Eduardo Habkost
2019-05-07 5:21 ` Philippe Mathieu-Daudé
@ 2019-05-07 14:13 ` Daniel P. Berrangé
2019-05-07 14:45 ` Eduardo Habkost
2019-05-08 9:19 ` Alex Bennée
2 siblings, 1 reply; 8+ messages in thread
From: Daniel P. Berrangé @ 2019-05-07 14:13 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Thomas Huth, qemu-devel, Michael Roth, Markus Armbruster,
Cleber Rosa, Philippe Mathieu-Daudé
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 :|
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema
2019-05-07 14:13 ` Daniel P. Berrangé
@ 2019-05-07 14:45 ` Eduardo Habkost
2019-05-08 13:04 ` Markus Armbruster
0 siblings, 1 reply; 8+ messages in thread
From: Eduardo Habkost @ 2019-05-07 14:45 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Thomas Huth, qemu-devel, Michael Roth, Markus Armbruster,
Cleber Rosa, Philippe Mathieu-Daudé
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema
2019-05-06 21:38 [Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema Eduardo Habkost
2019-05-07 5:21 ` Philippe Mathieu-Daudé
2019-05-07 14:13 ` Daniel P. Berrangé
@ 2019-05-08 9:19 ` Alex Bennée
2 siblings, 0 replies; 8+ messages in thread
From: Alex Bennée @ 2019-05-08 9:19 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Cleber Rosa, Philippe Mathieu-Daudé,
Michael Roth, Markus Armbruster
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema
2019-05-07 14:45 ` Eduardo Habkost
@ 2019-05-08 13:04 ` Markus Armbruster
2019-05-08 17:53 ` Eduardo Habkost
0 siblings, 1 reply; 8+ messages in thread
From: Markus Armbruster @ 2019-05-08 13:04 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Thomas Huth, qemu-devel, Michael Roth, Cleber Rosa,
Philippe Mathieu-Daudé
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema
2019-05-08 13:04 ` Markus Armbruster
@ 2019-05-08 17:53 ` Eduardo Habkost
2019-05-09 8:42 ` Markus Armbruster
0 siblings, 1 reply; 8+ messages in thread
From: Eduardo Habkost @ 2019-05-08 17:53 UTC (permalink / raw)
To: Markus Armbruster
Cc: Thomas Huth, qemu-devel, Michael Roth, Cleber Rosa,
Philippe Mathieu-Daudé
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema
2019-05-08 17:53 ` Eduardo Habkost
@ 2019-05-09 8:42 ` Markus Armbruster
0 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2019-05-09 8:42 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Thomas Huth, Cleber Rosa, Philippe Mathieu-Daudé, qemu-devel,
Michael Roth
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 :)
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-05-09 8:43 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-06 21:38 [Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema Eduardo Habkost
2019-05-07 5:21 ` Philippe Mathieu-Daudé
2019-05-07 14:13 ` Daniel P. Berrangé
2019-05-07 14:45 ` Eduardo Habkost
2019-05-08 13:04 ` Markus Armbruster
2019-05-08 17:53 ` Eduardo Habkost
2019-05-09 8:42 ` Markus Armbruster
2019-05-08 9:19 ` Alex Bennée
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).