[Qemu-devel] [PATCH v3 0/2] Fix compilation with python-3 if en_US.UTF-8 is unavailable

[Qemu-devel] [PATCH v3 0/2] Fix compilation with python-3 if en_US.UTF-8 is unavailable

[Matthias Maier]

Hi,

this is hopefully the final iteration of the patches.

v3:
 - reverse order of patches
 - rename second patch to "Revert commit ..."
 - drop "UTF-8" argument in decode()/encode(); bot functions default to
   utf-8 already.

v2:
 - ensure compatibility with python2 by only calling encode()/decode() if
   script is run with a python3 interpreter

Best,
Matthias

[Qemu-devel] [PATCH v3 1/2] qapi: open files in binary mode and use explicit decoding/encoding in common.py

[Matthias Maier]

This is a different approach to fix the locale dependent encode/decode
problem in common.py utilizing the binary read/write mode [1,2], and (if
a python 3 interpreter is used) with explicit decode/encode arguments
[3].

This approach is preferred over the fix in commit d4e5ec877ca because it
is (a) locale independent, and (b) does not depend on the en_US.UTF_8
locale to be available.

[1] https://docs.python.org/3.6/library/stdtypes.html#bytes.decode
[2] https://docs.python.org/3.6/library/stdtypes.html#str.encode
[3] https://docs.python.org/3/howto/unicode.html

Signed-off-by: Arfrever Frehtes Taifersar Arahesis <arfrever.fta@gmail.com>
Signed-off-by: Matthias Maier <tamiko@43-1.org>
---
 scripts/qapi/common.py | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 2462fc0291..a368e11a38 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -16,6 +16,7 @@ import errno
 import os
 import re
 import string
+import sys
 from collections import OrderedDict
 
 builtin_types = {
@@ -259,6 +260,8 @@ class QAPISchemaParser(object):
         previously_included.append(os.path.abspath(fp.name))
         self.incl_info = incl_info
         self.src = fp.read()
+        if sys.version_info[0] >= 3:
+            self.src = self.src.decode()
         if self.src == '' or self.src[-1] != '\n':
             self.src += '\n'
         self.cursor = 0
@@ -340,7 +343,7 @@ class QAPISchemaParser(object):
             return None
 
         try:
-            fobj = open(incl_fname, 'r')
+            fobj = open(incl_fname, 'rb')
         except IOError as e:
             raise QAPISemError(info, '%s: %s' % (e.strerror, incl_fname))
         return QAPISchemaParser(fobj, previously_included, info)
@@ -1492,7 +1495,7 @@ class QAPISchemaEvent(QAPISchemaEntity):
 class QAPISchema(object):
     def __init__(self, fname):
         self._fname = fname
-        parser = QAPISchemaParser(open(fname, 'r'))
+        parser = QAPISchemaParser(open(fname, 'rb'))
         exprs = check_exprs(parser.exprs)
         self.docs = parser.docs
         self._entity_list = []
@@ -2006,9 +2009,11 @@ class QAPIGen(object):
                 if e.errno != errno.EEXIST:
                     raise
         fd = os.open(pathname, os.O_RDWR | os.O_CREAT, 0o666)
-        f = os.fdopen(fd, 'r+')
+        f = os.fdopen(fd, 'r+b')
         text = (self._top(fname) + self._preamble + self._body
                 + self._bottom(fname))
+        if sys.version_info[0] >= 3:
+            text = text.encode()
         oldtext = f.read(len(text) + 1)
         if text != oldtext:
             f.seek(0)
-- 
2.16.4

[Qemu-devel] [PATCH v3 2/2] Revert commit d4e5ec877ca

[Matthias Maier]

This commit removes the PYTHON_UTF8 workaround. The problem with setting

  LC_ALL= LANG=C LC_CTYPE=en_US.UTF-8

is that the en_US.UTF-8 locale might not be available. In this case
setting above locales results in build errors even though another UTF-8
locale was originally set [1]. The only stable way of fixing the
encoding problem is by explicitly annotating encoding/decoding in the
python script.

[1] https://bugs.gentoo.org/657766

Signed-off-by: Arfrever Frehtes Taifersar Arahesis <arfrever.fta@gmail.com>
Signed-off-by: Matthias Maier <tamiko@43-1.org>
---
 Makefile               | 6 ++----
 tests/Makefile.include | 6 +++---
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index e46f2b625a..7ed9cc4a21 100644
--- a/Makefile
+++ b/Makefile
@@ -20,8 +20,6 @@ ifneq ($(wildcard config-host.mak),)
 all:
 include config-host.mak
 
-PYTHON_UTF8 = LC_ALL= LANG=C LC_CTYPE=en_US.UTF-8 $(PYTHON)
-
 git-submodule-update:
 
 .PHONY: git-submodule-update
@@ -576,7 +574,7 @@ qga/qapi-generated/qga-qapi-commands.h qga/qapi-generated/qga-qapi-commands.c \
 qga/qapi-generated/qga-qapi-doc.texi: \
 qga/qapi-generated/qapi-gen-timestamp ;
 qga/qapi-generated/qapi-gen-timestamp: $(SRC_PATH)/qga/qapi-schema.json $(qapi-py)
-	$(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-gen.py \
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-gen.py \
 		-o qga/qapi-generated -p "qga-" $<, \
 		"GEN","$(@:%-timestamp=%)")
 	@>$@
@@ -676,7 +674,7 @@ qapi/qapi-introspect.h qapi/qapi-introspect.c \
 qapi/qapi-doc.texi: \
 qapi-gen-timestamp ;
 qapi-gen-timestamp: $(qapi-modules) $(qapi-py)
-	$(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-gen.py \
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-gen.py \
 		-o "qapi" -b $<, \
 		"GEN","$(@:%-timestamp=%)")
 	@>$@
diff --git a/tests/Makefile.include b/tests/Makefile.include
index ca91da26cb..88f1bc1242 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -678,13 +678,13 @@ tests/test-qapi-events.c tests/test-qapi-events.h \
 tests/test-qapi-introspect.c tests/test-qapi-introspect.h: \
 tests/test-qapi-gen-timestamp ;
 tests/test-qapi-gen-timestamp: $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(qapi-py)
-	$(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-gen.py \
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-gen.py \
 		-o tests -p "test-" $<, \
 		"GEN","$(@:%-timestamp=%)")
 	@>$@
 
 tests/qapi-schema/doc-good.test.texi: $(SRC_PATH)/tests/qapi-schema/doc-good.json $(qapi-py)
-	$(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-gen.py \
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-gen.py \
 		-o tests/qapi-schema -p "doc-good-" $<, \
 		"GEN","$@")
 	@mv tests/qapi-schema/doc-good-qapi-doc.texi $@
@@ -942,7 +942,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_UTF8) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \
+		$(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \
 		$^ >$*.test.out 2>$*.test.err; \
 		echo $$? >$*.test.exit, \
 		"TEST","$*.out")
-- 
2.16.4

Re: [Qemu-devel] [PATCH v3 1/2] qapi: open files in binary mode and use explicit decoding/encoding in common.py

[Markus Armbruster]

Matthias Maier <tamiko@43-1.org> writes:

> This is a different approach to fix the locale dependent encode/decode
> problem in common.py utilizing the binary read/write mode [1,2], and (if
> a python 3 interpreter is used) with explicit decode/encode arguments
> [3].

Why can't we simply pass encoding='utf-8' to open()?

> This approach is preferred over the fix in commit d4e5ec877ca because it
> is (a) locale independent, and (b) does not depend on the en_US.UTF_8
> locale to be available.
>
> [1] https://docs.python.org/3.6/library/stdtypes.html#bytes.decode
> [2] https://docs.python.org/3.6/library/stdtypes.html#str.encode
> [3] https://docs.python.org/3/howto/unicode.html
>
> Signed-off-by: Arfrever Frehtes Taifersar Arahesis <arfrever.fta@gmail.com>
> Signed-off-by: Matthias Maier <tamiko@43-1.org>

Re: [Qemu-devel] [PATCH v3 1/2] qapi: open files in binary mode and use explicit decoding/encoding in common.py

[Matthias Maier]

On Mon, Jun 18, 2018, at 00:25 CDT, Markus Armbruster <armbru@redhat.com> wrote:

> Matthias Maier <tamiko@43-1.org> writes:
>
>> This is a different approach to fix the locale dependent encode/decode
>> problem in common.py utilizing the binary read/write mode [1,2], and (if
>> a python 3 interpreter is used) with explicit decode/encode arguments
>> [3].
>
> Why can't we simply pass encoding='utf-8' to open()?

Because this breaks python-2 compatibility.

Best,
Matthias

Re: [Qemu-devel] [PATCH v3 1/2] qapi: open files in binary mode and use explicit decoding/encoding in common.py

[Eduardo Habkost]

On Mon, Jun 18, 2018 at 07:25:14AM +0200, Markus Armbruster wrote:
> Matthias Maier <tamiko@43-1.org> writes:
> 
> > This is a different approach to fix the locale dependent encode/decode
> > problem in common.py utilizing the binary read/write mode [1,2], and (if
> > a python 3 interpreter is used) with explicit decode/encode arguments
> > [3].
> 
> Why can't we simply pass encoding='utf-8' to open()?

This wouldn't work in Python 2.7 (where the `open()` builtin
doesn't support the `encoding` parameter).

io.open(..., encoding='utf-8') should work, though.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 1/2] qapi: open files in binary mode and use explicit decoding/encoding in common.py

[Markus Armbruster]

Eduardo Habkost <ehabkost@redhat.com> writes:

> On Mon, Jun 18, 2018 at 07:25:14AM +0200, Markus Armbruster wrote:
>> Matthias Maier <tamiko@43-1.org> writes:
>> 
>> > This is a different approach to fix the locale dependent encode/decode
>> > problem in common.py utilizing the binary read/write mode [1,2], and (if
>> > a python 3 interpreter is used) with explicit decode/encode arguments
>> > [3].
>> 
>> Why can't we simply pass encoding='utf-8' to open()?
>
> This wouldn't work in Python 2.7 (where the `open()` builtin
> doesn't support the `encoding` parameter).
>
> io.open(..., encoding='utf-8') should work, though.

This falls apart because then f.read() returns objects of type 'unicode'
in Python 2, breaking isinstance(..., str) predicates in several places.

What I asked for is something else: wrap the version conditional around
open() instead of around the conversion from bytes to str.  Coding up
the (trivial) patch is easier than explaining it in more detail than
"pass encoding='utf-8' to open()", so I did just that.

Re: [Qemu-devel] [PATCH v3 1/2] qapi: open files in binary mode and use explicit decoding/encoding in common.py

[Eduardo Habkost]

On Mon, Jun 18, 2018 at 07:54:26PM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > On Mon, Jun 18, 2018 at 07:25:14AM +0200, Markus Armbruster wrote:
> >> Matthias Maier <tamiko@43-1.org> writes:
> >> 
> >> > This is a different approach to fix the locale dependent encode/decode
> >> > problem in common.py utilizing the binary read/write mode [1,2], and (if
> >> > a python 3 interpreter is used) with explicit decode/encode arguments
> >> > [3].
> >> 
> >> Why can't we simply pass encoding='utf-8' to open()?
> >
> > This wouldn't work in Python 2.7 (where the `open()` builtin
> > doesn't support the `encoding` parameter).
> >
> > io.open(..., encoding='utf-8') should work, though.
> 
> This falls apart because then f.read() returns objects of type 'unicode'
> in Python 2, breaking isinstance(..., str) predicates in several places.

If the existing code already works with Python 3, we can import
Python 3 string semantics in Python 2.7 so we have just one
string API to care about.

This should do it:

  from __future__ import unicode_literals
  from builtins import str
  from builtins import open

We also need a small fixup to the argparse parameters to ensure
they are of the Python3-like str type instead of the Python 2.7
str type.

Full patch below.

After this patch, we should be able to unconditionally call
open(..., encoding='utf-8') or .read().decode('utf-8'), without
any Python version checks.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
diff --git a/scripts/qapi-gen.py b/scripts/qapi-gen.py
index 3d98ca2e0c..a0dbc4c913 100755
--- a/scripts/qapi-gen.py
+++ b/scripts/qapi-gen.py
@@ -5,6 +5,8 @@
 # See the COPYING file in the top-level directory.
 
 from __future__ import print_function
+from __future__ import unicode_literals
+from builtins import str
 import argparse
 import re
 import sys
@@ -23,13 +25,13 @@ def main(argv):
     parser.add_argument('-b', '--builtins', action='store_true',
                         help="generate code for built-in types")
     parser.add_argument('-o', '--output-dir', action='store', default='',
-                        help="write output to directory OUTPUT_DIR")
+                        type=str, help="write output to directory OUTPUT_DIR")
     parser.add_argument('-p', '--prefix', action='store', default='',
-                        help="prefix for symbols")
+                        type=str, help="prefix for symbols")
     parser.add_argument('-u', '--unmask-non-abi-names', action='store_true',
                         dest='unmask',
                         help="expose non-ABI names in introspection")
-    parser.add_argument('schema', action='store')
+    parser.add_argument('schema', action='store', type=str)
     args = parser.parse_args()
 
     match = re.match(r'([A-Za-z_.-][A-Za-z0-9_.-]*)?', args.prefix)
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index 3b0867c14f..df69481a4b 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -13,6 +13,7 @@ This work is licensed under the terms of the GNU GPL, version 2.
 See the COPYING file in the top-level directory.
 """
 
+from __future__ import unicode_literals
 from qapi.common import *
 
 
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 2462fc0291..7ff5d1bbdd 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -12,6 +12,9 @@
 # See the COPYING file in the top-level directory.
 
 from __future__ import print_function
+from __future__ import unicode_literals
+from builtins import str
+from builtins import open
 import errno
 import os
 import re
diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index b5630844f9..d223d5cf46 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -6,6 +6,7 @@
 """This script produces the documentation of a qapi schema in texinfo format"""
 
 from __future__ import print_function
+from __future__ import unicode_literals
 import re
 import qapi.common
 
diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
index 4426861ff1..a58d557ccd 100644
--- a/scripts/qapi/events.py
+++ b/scripts/qapi/events.py
@@ -12,6 +12,7 @@ This work is licensed under the terms of the GNU GPL, version 2.
 See the COPYING file in the top-level directory.
 """
 
+from __future__ import unicode_literals
 from qapi.common import *
 
 
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 5b6c72c7b2..4a582d3a5d 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -10,6 +10,8 @@ This work is licensed under the terms of the GNU GPL, version 2.
 See the COPYING file in the top-level directory.
 """
 
+from __future__ import unicode_literals
+from builtins import str
 from qapi.common import *
 
 
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index 64d9c0fb37..a530838f97 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -13,6 +13,7 @@ This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.
 """
 
+from __future__ import unicode_literals
 from qapi.common import *
 
 
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 5d72d8936c..a6479fa4fa 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -13,6 +13,7 @@ This work is licensed under the terms of the GNU GPL, version 2.
 See the COPYING file in the top-level directory.
 """
 
+from __future__ import unicode_literals
 from qapi.common import *
 
 
-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 1/2] qapi: open files in binary mode and use explicit decoding/encoding in common.py

[Eduardo Habkost]

On Mon, Jun 18, 2018 at 06:17:04PM -0300, Eduardo Habkost wrote:
> On Mon, Jun 18, 2018 at 07:54:26PM +0200, Markus Armbruster wrote:
> > Eduardo Habkost <ehabkost@redhat.com> writes:
> > 
> > > On Mon, Jun 18, 2018 at 07:25:14AM +0200, Markus Armbruster wrote:
> > >> Matthias Maier <tamiko@43-1.org> writes:
> > >> 
> > >> > This is a different approach to fix the locale dependent encode/decode
> > >> > problem in common.py utilizing the binary read/write mode [1,2], and (if
> > >> > a python 3 interpreter is used) with explicit decode/encode arguments
> > >> > [3].
> > >> 
> > >> Why can't we simply pass encoding='utf-8' to open()?
> > >
> > > This wouldn't work in Python 2.7 (where the `open()` builtin
> > > doesn't support the `encoding` parameter).
> > >
> > > io.open(..., encoding='utf-8') should work, though.
> > 
> > This falls apart because then f.read() returns objects of type 'unicode'
> > in Python 2, breaking isinstance(..., str) predicates in several places.
> 
> If the existing code already works with Python 3, we can import
> Python 3 string semantics in Python 2.7 so we have just one
> string API to care about.
> 
> This should do it:
> 
>   from __future__ import unicode_literals
>   from builtins import str
>   from builtins import open

Oops, I just noticed that this needs the 'future' package to be
installed.

I will submit a patch series later requiring python-future on
./configure.  In the meantime, probably your series is good
enough.

-- 
Eduardo