Support building with py2 or py3
v4 cover Support building with py2 or py3
This is an update for my previously posted series:
v2: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg06528.html
This series enables some level of CI testing for py3 so that our CI jobs will get coverage of both py2 and py3 builds to avoid bitrot.
I did a test travis build with py 3.0 and py 3.6 and got success:
https://travis-ci.org/berrange/qemu/builds/328223261
The goal was to achieve the following
./configure --python=/usr/bin/python3 make make check
This still requires passing python path to configure explicitly. A further improvement would be for configure to automatically detect a pythjon 3 binary and use it preferentially to python 2.
I have not attempted to fix/validate the block I/O tests. I would expect them to be broken, but easily fixable with the similar kind of scope changes as seen here. I felt it better to tackle that separately to avoid this initial series getting too large.
Although the Python 2 EOL date is 2020, we already have distros which are not shipping Python 2 by default (Fedora >= 26 has dropped Py2 from the default install). Any new releases of long life and/or enterprise distros may well not ship Python 2 given that it would go EOL long before the EOL of the distro itself. IOW QEMU does have a fairly pressing need to be able to support Python 3 for building.
A request for py3 is tracked here:
https://bugs.launchpad.net/qemu/+bug/1708462
NB, Patch 10 here is not related to python 3 work - it was just a temporary pre-requisite of pulling in the keycodemapdb update.
Changes since v3:
- Remove space before '(' in print() function calls (Phillippe)
- Force use of en_US.UTF-8 for QAPI code generation (Patchew)
Changes since v2:
- Pull in fix for keycodemapdb
- Enable testing with Travis
- Enable testing with Fedora Docker images
- Fix for sort ordering to fix 'make check-qapi-schema'
- Fix for signrom data
Daniel P. Berrange (12):
qapi: use items()/values() intead of iteritems()/itervalues()
qapi: Use OrderedDict from standard library if available
qapi: adapt to moved location of StringIO module in py3
qapi: Adapt to moved location of 'maketrans' function in py3
qapi: remove '-q' arg to diff when comparing QAPI output
qapi: ensure stable sort ordering when checking QAPI entities
qapi: force a UTF-8 locale for running Python
scripts: ensure signrom treats data as bytes
configure: allow use of python 3
ui: update keycodemapdb to get py3 fixes
travis: improve python version test coverage
docker: change Fedora images to run with python3
Miika S (1):
input: add missing JIS keys to virtio input
.travis.yml | 14 +++++++---- Makefile | 22 +++++++++-------- configure | 5 ++-- hw/input/virtio-input-hid.c | 7 ++++++ qapi/ui.json | 5 +++- scripts/qapi.py | 43 ++++++++++++++++++++-------------- scripts/qapi2texi.py | 11 +++++---- scripts/signrom.py | 4 ++-- tests/Makefile.include | 6 ++--- tests/docker/dockerfiles/fedora.docker | 3 ++- tests/qapi-schema/test-qapi.py | 43 +++++++++++++++++----------------- ui/keycodemapdb | 2 +- 12 files changed, 96 insertions(+), 69 deletions(-)
Signature · 1 line
2.14.3
PATCH v4 1/13 qapi: use items()/values() intead of iteritems()/itervalues()
The iteritems()/itervalues() methods are gone in py3, but the items()/values() methods are still around. The latter are less efficient than the former in py2, but this has unmeasurably small impact on QEMU build time, so taking portability over efficiency is a net win.
--- scripts/qapi.py | 24 +++++++++++------------ scripts/qapi2texi.py | 11 ++++++----- tests/qapi-schema/test-qapi.py | 43 +++++++++++++++++++++--------------------- 3 files changed, 40 insertions(+), 38 deletions(-)
scripts/qapi.py +12 -12
@@ -11,6 +11,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 print_function import errno import getopt import os
@@ -244,7 +245,7 @@ class QAPIDoc(object): "'Returns:' is only valid for commands") def check(self): - bogus = [name for name, section in self.args.iteritems() + bogus = [name for name, section in self.args.items() if not section.member] if bogus: raise QAPISemError(
@@ -299,7 +300,7 @@ class QAPISchemaParser(object): if not isinstance(pragma, dict): raise QAPISemError( info, "Value of 'pragma' must be a dictionary") - for name, value in pragma.iteritems(): + for name, value in pragma.items(): self._pragma(name, value, info) else: expr_elem = {'expr': expr,
@@ -1467,7 +1468,7 @@ class QAPISchema(object): self._def_exprs() self.check() except QAPIError as err: - print >>sys.stderr, err + print(err, file=sys.stderr) exit(1) def _def_entity(self, ent):
@@ -1565,7 +1566,7 @@ class QAPISchema(object): def _make_members(self, data, info): return [self._make_member(key, value, info) - for (key, value) in data.iteritems()] + for (key, value) in data.items()] def _def_struct_type(self, expr, info, doc): name = expr['struct']
@@ -1597,11 +1598,11 @@ class QAPISchema(object): name, info, doc, 'base', self._make_members(base, info))) if tag_name: variants = [self._make_variant(key, value) - for (key, value) in data.iteritems()] + for (key, value) in data.items()] members = [] else: variants = [self._make_simple_variant(key, value, info) - for (key, value) in data.iteritems()] + for (key, value) in data.items()] typ = self._make_implicit_enum_type(name, info, [v.name for v in variants]) tag_member = QAPISchemaObjectTypeMember('type', typ, False)
@@ -1616,7 +1617,7 @@ class QAPISchema(object): name = expr['alternate'] data = expr['data'] variants = [self._make_variant(key, value) - for (key, value) in data.iteritems()] + for (key, value) in data.items()] tag_member = QAPISchemaObjectTypeMember('type', 'QType', False) self._def_entity( QAPISchemaAlternateType(name, info, doc,
@@ -1931,7 +1932,7 @@ def parse_command_line(extra_options='', extra_long_options=[]): ['source', 'header', 'prefix=', 'output-dir='] + extra_long_options) except getopt.GetoptError as err: - print >>sys.stderr, "%s: %s" % (sys.argv[0], str(err)) + print("%s: %s" % (sys.argv[0], str(err)), file=sys.stderr) sys.exit(1) output_dir = ''
@@ -1945,9 +1946,8 @@ def parse_command_line(extra_options='', extra_long_options=[]): if o in ('-p', '--prefix'): match = re.match(r'([A-Za-z_.-][A-Za-z0-9_.-]*)?', a) if match.end() != len(a): - print >>sys.stderr, \ - "%s: 'funny character '%s' in argument of --prefix" \ - % (sys.argv[0], a[match.end()]) + print("%s: 'funny character '%s' in argument of --prefix" \ + % (sys.argv[0], a[match.end()]), file=sys.stderr) sys.exit(1) prefix = a elif o in ('-o', '--output-dir'):
@@ -1964,7 +1964,7 @@ def parse_command_line(extra_options='', extra_long_options=[]): do_h = True if len(args) != 1: - print >>sys.stderr, "%s: need exactly one argument" % sys.argv[0] + print("%s: need exactly one argument" % sys.argv[0], file=sys.stderr) sys.exit(1) fname = args[0]
scripts/qapi2texi.py +6 -5
@@ -4,6 +4,7 @@ # This work is licensed under the terms of the GNU LGPL, version 2+. # See the COPYING file in the top-level directory. """This script produces the documentation of a qapi schema in texinfo format""" +from __future__ import print_function import re import sys
@@ -145,7 +146,7 @@ def texi_member(member, suffix=''): def texi_members(doc, what, base, variants, member_func): """Format the table of members""" items = '' - for section in doc.args.itervalues(): + for section in doc.args.values(): # TODO Drop fallbacks when undocumented members are outlawed if section.text: desc = texi_format(section.text)
@@ -274,15 +275,15 @@ def texi_schema(schema): def main(argv): """Takes schema argument, prints result to stdout""" if len(argv) != 2: - print >>sys.stderr, "%s: need exactly 1 argument: SCHEMA" % argv[0] + print("%s: need exactly 1 argument: SCHEMA" % argv[0], file=sys.stderr) sys.exit(1) schema = qapi.QAPISchema(argv[1]) if not qapi.doc_required: - print >>sys.stderr, ("%s: need pragma 'doc-required' " - "to generate documentation" % argv[0]) + print("%s: need pragma 'doc-required' " + "to generate documentation" % argv[0], file=sys.stderr) sys.exit(1) - print texi_schema(schema) + print(texi_schema(schema)) if __name__ == '__main__':
tests/qapi-schema/test-qapi.py +22 -21
79 lines not on this page · raw message
Signature · 1 line
2.14.3
PATCH v4 2/13 qapi: Use OrderedDict from standard library if available
In reply to Daniel P. BerrangeThe OrderedDict class appeared in the 'collections' module from python 2.7 onwards, so use that in preference to our local backport if available.
--- scripts/qapi.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
scripts/qapi.py +4 -1
@@ -18,7 +18,10 @@ import os import re import string import sys -from ordereddict import OrderedDict +try: + from collections import OrderedDict +except: + from ordereddict import OrderedDict builtin_types = { 'null': 'QTYPE_QNULL',
Signature · 1 line
2.14.3
PATCH v4 3/13 qapi: adapt to moved location of StringIO module in py3
In reply to Daniel P. Berrange--- scripts/qapi.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
scripts/qapi.py +5 -2
@@ -22,6 +22,10 @@ try: from collections import OrderedDict except: from ordereddict import OrderedDict +try: + from StringIO import StringIO +except ImportError: + from io import StringIO builtin_types = { 'null': 'QTYPE_QNULL',
@@ -1995,8 +1999,7 @@ def open_output(output_dir, do_c, do_h, prefix, c_file, h_file, if really: return open(name, opt) else: - import StringIO - return StringIO.StringIO() + return StringIO() fdef = maybe_open(do_c, c_file, 'w') fdecl = maybe_open(do_h, h_file, 'w')
Signature · 1 line
2.14.3
PATCH v4 4/13 qapi: Adapt to moved location of 'maketrans' function in py3
In reply to Daniel P. Berrange--- scripts/qapi.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
scripts/qapi.py +4 -1
@@ -1734,7 +1734,10 @@ def c_enum_const(type_name, const_name, prefix=None): type_name = prefix return camel_to_upper(type_name) + '_' + c_name(const_name, False).upper() -c_name_trans = string.maketrans('.-', '__') +if hasattr(str, 'maketrans'): + c_name_trans = str.maketrans('.-', '__') +else: + c_name_trans = string.maketrans('.-', '__') # Map @name to a valid C identifier.
Signature · 1 line
2.14.3
Re: [Qemu-devel] [PATCH v4 04/13] qapi: Adapt to moved location of 'maketrans' function in py3
Quoted 4 lines · Daniel P. Berrange
Signed-off-by Daniel P. Berrange <[email protected]>--- scripts/qapi.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
Quoted 18 lines
scripts/qapi.py +4
-1@@ -1734,7 +1734,10 @@ def c_enum_const(type_name, const_name, prefix=None): type_name = prefix return camel_to_upper(type_name) + '_' + c_name(const_name, False).upper() -c_name_trans = string.maketrans('.-', '__') +if hasattr(str, 'maketrans'): + c_name_trans = str.maketrans('.-', '__') +else: + c_name_trans = string.maketrans('.-', '__') # Map @name to a valid C identifier.
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
PATCH v4 5/13 qapi: remove '-q' arg to diff when comparing QAPI output
In reply to Daniel P. BerrangeWhen the qapi schema tests fail they merely print that the expected output didn't match the actual output. This is largely useless when trying diagnose what went wrong. Removing the '-q' arg to diff means that it is still silent on successful tests, but when it fails we'll see details of the incorrect output.
--- tests/Makefile.include | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
tests/Makefile.include +3 -3
@@ -908,10 +908,10 @@ $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json $^ >$*.test.out 2>$*.test.err; \ echo $$? >$*.test.exit, \ "TEST","$*.out") - @diff -q $(SRC_PATH)/$*.out $*.test.out + @diff $(SRC_PATH)/$*.out $*.test.out @# Sanitize error messages (make them independent of build directory) - @perl -p -e 's|\Q$(SRC_PATH)\E/||g' $*.test.err | diff -q $(SRC_PATH)/$*.err - - @diff -q $(SRC_PATH)/$*.exit $*.test.exit + @perl -p -e 's|\Q$(SRC_PATH)\E/||g' $*.test.err | diff $(SRC_PATH)/$*.err - + @diff $(SRC_PATH)/$*.exit $*.test.exit .PHONY: check-tests/qapi-schema/doc-good.texi check-tests/qapi-schema/doc-good.texi: tests/qapi-schema/doc-good.test.texi
Signature · 1 line
2.14.3
Re: [Qemu-devel] [PATCH v4 05/13] qapi: remove '-q' arg to diff when comparing QAPI output
Quoted 11 lines · Daniel P. Berrange
When the qapi schema tests fail they merely print that the expected output didn't match the actual output. This is largely useless when trying diagnose what went wrong. Removing the '-q' arg to diff means that it is still silent on successful tests, but when it fails we'll see details of the incorrect output.
Signed-off-by Daniel P. Berrange <[email protected]>--- tests/Makefile.include | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
PATCH v4 6/13 qapi: ensure stable sort ordering when checking QAPI entities
In reply to Daniel P. BerrangeSome early python 3.x versions will have different default ordering when calling the 'values()' method on a dict, compared to python 2.x and later 3.x versions. Explicitly sort the items to get a stable ordering.
--- scripts/qapi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
scripts/qapi.py +1 -1
@@ -1678,7 +1678,7 @@ class QAPISchema(object): assert False def check(self): - for ent in self._entity_dict.values(): + for (name, ent) in sorted(self._entity_dict.items()): ent.check(self) def visit(self, visitor):
Signature · 1 line
2.14.3
PATCH v4 7/13 qapi: force a UTF-8 locale for running Python
In reply to Daniel P. BerrangePython2 did not validate locale correctness when reading input data, so would happily read UTF-8 data in non-UTF-8 locales. Python3 is strict so if you try to read UTF-8 data in the C locale, it will raise an error for any UTF-8 bytes that aren't representable in 7-bit ascii encoding. e.g.
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 54: ordinal not in range(128) Traceback (most recent call last):
File "/tmp/qemu-test/src/scripts/qapi-commands.py", line 317, in <module>
schema = QAPISchema(input_file)
File "/tmp/qemu-test/src/scripts/qapi.py", line 1468, in __init__
parser = QAPISchemaParser(open(fname, 'r'))
File "/tmp/qemu-test/src/scripts/qapi.py", line 301, in __init__
previously_included)
File "/tmp/qemu-test/src/scripts/qapi.py", line 348, in _include
exprs_include = QAPISchemaParser(fobj, previously_included, info)
File "/tmp/qemu-test/src/scripts/qapi.py", line 271, in __init__
self.src = fp.read()
File "/usr/lib64/python3.5/encodings/ascii.py", line 26, in decode
return codecs.ascii_decode(input, self.errors)[0]
Many distros support a new C.UTF-8 locale that is like the C locale, but with UTF-8 instead of 7-bit ASCII. That is not entirely portable though, so this patch instead forces the en_US.UTF-8 locale, which is pretty similar but more widely available.
We set LANG, rather than only LC_CTYPE, since generated source ought to be independant of all of the user's locale settings.
This patch only forces UTF-8 for QAPI scripts, since that is the one showing the immediate error under Python3 with C locale, but potentially we ought to force this for all python scripts used in the build process.
--- Makefile | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)
Makefile +12 -10
@@ -17,6 +17,8 @@ ifneq ($(wildcard config-host.mak),) all: include config-host.mak +PYTHON_UTF8 = LANG=en_US.UTF-8 $(PYTHON) + git-submodule-update: .PHONY: git-submodule-update
@@ -471,17 +473,17 @@ qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \ + $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-types.py \ $(gen-out-type) -o qga/qapi-generated -p "qga-" $<, \ "GEN","$@") qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \ + $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-visit.py \ $(gen-out-type) -o qga/qapi-generated -p "qga-" $<, \ "GEN","$@") qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c :\ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \ + $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-commands.py \ $(gen-out-type) -o qga/qapi-generated -p "qga-" $<, \ "GEN","$@")
@@ -502,27 +504,27 @@ qapi-modules = $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qapi/common.json \ qapi-types.c qapi-types.h :\ $(qapi-modules) $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \ + $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-types.py \ $(gen-out-type) -o "." -b $<, \ "GEN","$@") qapi-visit.c qapi-visit.h :\ $(qapi-modules) $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \ + $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-visit.py \ $(gen-out-type) -o "." -b $<, \ "GEN","$@") qapi-event.c qapi-event.h :\ $(qapi-modules) $(SRC_PATH)/scripts/qapi-event.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py \ + $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-event.py \ $(gen-out-type) -o "." $<, \ "GEN","$@") qmp-commands.h qmp-marshal.c :\ $(qapi-modules) $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \ + $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-commands.py \ $(gen-out-type) -o "." $<, \ "GEN","$@") qmp-introspect.h qmp-introspect.c :\ $(qapi-modules) $(SRC_PATH)/scripts/qapi-introspect.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-introspect.py \ + $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-introspect.py \ $(gen-out-type) -o "." $<, \ "GEN","$@")
@@ -792,10 +794,10 @@ qemu-img-cmds.texi: $(SRC_PATH)/qemu-img-cmds.hx $(SRC_PATH)/scripts/hxtool docs/interop/qemu-qmp-qapi.texi docs/interop/qemu-ga-qapi.texi: $(SRC_PATH)/scripts/qapi2texi.py $(qapi-py) docs/interop/qemu-qmp-qapi.texi: $(qapi-modules) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > $@,"GEN","$@") + $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi2texi.py $< > $@,"GEN","$@") docs/interop/qemu-ga-qapi.texi: $(SRC_PATH)/qga/qapi-schema.json - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > $@,"GEN","$@") + $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi2texi.py $< > $@,"GEN","$@") qemu.1: qemu-doc.texi qemu-options.texi qemu-monitor.texi qemu-monitor-info.texi qemu.1: qemu-option-trace.texi
Signature · 1 line
2.14.3
Re: [Qemu-devel] [PATCH v4 07/13] qapi: force a UTF-8 locale for running Python
Quoted 4 lines · Daniel P. Berrange
Python2 did not validate locale correctness when reading input data, so would happily read UTF-8 data in non-UTF-8 locales. Python3 is strict so if you try to read UTF-8 data in the C locale, it will raise an error for any UTF-8 bytes that aren't representable in 7-bit ascii encoding.
Urgh, that sounds like a Python bug. The C locale is defined by POSIX to be 8-bit clean (ie. a superset of ascii with 256 characters, not strict ascii with only 128 characters and 128 bytes that form encoding errors).
But that doesn't change the fact that we have to work around python's
braindead misinterpretation of reality.
Quoted 17 lines
e.g.
UnicodeDecodeError: 'ascii' codec can't decode byte
0xc3in position 54: ordinal not in range(128) Traceback (most recent call last):
File "/tmp/qemu-test/src/scripts/qapi-commands.py", line 317, in <module>
schema =QAPISchema(input_file)
File "/tmp/qemu-test/src/scripts/qapi.py", line 1468, in__init__
parser =QAPISchemaParser(open(fname, 'r'))
File "/tmp/qemu-test/src/scripts/qapi.py", line 301, in__init__
previously_included)
File "/tmp/qemu-test/src/scripts/qapi.py", line 348, in_include
exprs_include=QAPISchemaParser(fobj, previously_included, info)
File "/tmp/qemu-test/src/scripts/qapi.py", line 271, in__init__
self.src =fp.read()
File "/usr/lib64/python3.5/encodings/ascii.py", line 26, in decode
returncodecs.ascii_decode(input, self.errors)[0]
Quoted 7 lines
Many distros support a new C.UTF-8 locale that is like the C locale, but with UTF-8 instead of 7-bit ASCII. That is not entirely portable though, so this patch instead forces the en_US.UTF-8 locale, which is pretty similar but more widely available.
We set LANG, rather than only
LC_CTYPE, since generated source ought to be independant of all of the user's locale settings.
s/independant/independent/
LANG is the lowest-priority setting - if the user has explicitly set LC_CTYPE or LC_ALL, their settings override what is in LANG.
Quoted 15 lines
This patch only forces UTF-8 for QAPI scripts, since that is the one showing the immediate error under Python3 with C locale, but potentially we ought to force this for all python scripts used in the build process.
Signed-off-by Daniel P. Berrange <[email protected]>--- Makefile | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)Makefile +0
-0
Makefile ifneq ($(wildcard config-host.mak),) · line 20
Quoted 4 lines
Makefile +1
-0@@ -17,6 +17,8 @@ ifneq ($(wildcard config-host.mak),) all: include config-host.mak +PYTHON_UTF8 = LANG=en_US.UTF-8 $(PYTHON)
I'm worried that this is not reproducible in the face of a user that explicitly sets different locale env-vars with higher priority than LANG.
Makefile qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py · line 476
Quoted 10 lines
Makefile +2
-1+ git-submodule-update: .PHONY: git-submodule-update@@ -471,17 +473,17 @@ qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \ + $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-types.py \
But once we agree on the right override to stuff into PYTHON_UTF8, the rest of the patch converting invocations to PYTHON_UTF8 makes sense.
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v4 07/13] qapi: force a UTF-8 locale for running Python
Quoted 11 lines · Eric Blake
Quoted 4 lines · Daniel P. Berrange
Python2 did not validate locale correctness when reading input data, so would happily read UTF-8 data in non-UTF-8 locales. Python3 is strict so if you try to read UTF-8 data in the C locale, it will raise an error for any UTF-8 bytes that aren't representable in 7-bit ascii encoding.
Urgh, that sounds like a Python bug. The C locale is defined by POSIX to be 8-bit clean (ie. a superset of ascii with 256 characters, not strict ascii with only 128 characters and 128 bytes that form encoding errors).
But that doesn't change the fact that we have to work around python's
braindead misinterpretation of reality.
FYI there is some background on this behaviour here:
https://www.python.org/dev/peps/pep-0538/
NB that doc says the new C-is-UTF-8 assumpion is for Python 3.7 or later, but Fedora backported it to F27's Python 3.6 :-)
The failure can be seen on Fedora with 3.0 -> 3.5 only. (BTW you can install many Python 3.x versions concurrently on Fedora which is handy for testing)
Quoted 48 lines
Quoted 17 lines
e.g.
UnicodeDecodeError: 'ascii' codec can't decode byte
0xc3in position 54: ordinal not in range(128) Traceback (most recent call last):
File "/tmp/qemu-test/src/scripts/qapi-commands.py", line 317, in <module>
schema =QAPISchema(input_file)
File "/tmp/qemu-test/src/scripts/qapi.py", line 1468, in__init__
parser =QAPISchemaParser(open(fname, 'r'))
File "/tmp/qemu-test/src/scripts/qapi.py", line 301, in__init__
previously_included)
File "/tmp/qemu-test/src/scripts/qapi.py", line 348, in_include
exprs_include=QAPISchemaParser(fobj, previously_included, info)
File "/tmp/qemu-test/src/scripts/qapi.py", line 271, in__init__
self.src =fp.read()
File "/usr/lib64/python3.5/encodings/ascii.py", line 26, in decode
returncodecs.ascii_decode(input, self.errors)[0]Quoted 7 lines
Many distros support a new C.UTF-8 locale that is like the C locale, but with UTF-8 instead of 7-bit ASCII. That is not entirely portable though, so this patch instead forces the en_US.UTF-8 locale, which is pretty similar but more widely available.
We set LANG, rather than only
LC_CTYPE, since generated source ought to be independant of all of the user's locale settings.s/independant/independent/
LANG is the lowest-priority setting - if the user has explicitly set
LC_CTYPEorLC_ALL, their settings override what is in LANG.Quoted 18 lines
This patch only forces UTF-8 for QAPI scripts, since that is the one showing the immediate error under Python3 with C locale, but potentially we ought to force this for all python scripts used in the build process.
Signed-off-by Daniel P. Berrange <[email protected]>--- Makefile | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)Makefile +0
-0@@ -17,6 +17,8 @@ ifneq ($(wildcard config-host.mak),) all: include config-host.mak
Quoted 4 lines
Makefile ifneq ($(wildcard config-host.mak),) · line 20
Quoted 1 line
Makefile +1
-0+PYTHON_UTF8 = LANG=en_US.UTF-8 $(PYTHON)I'm worried that this is not reproducible in the face of a user that explicitly sets different locale env-vars with higher priority than LANG.
You might remember a similar issue affecting libvirt-glib/libosinfo when glib-mkenums was rewritten to use Python instead of Perl. For that I ended up doing
LC_ALL= LANG=C LC_CTYPE=en_US.UTF-8
Quoted 6 lines
Quoted 6 lines
Makefile +1
-0+ git-submodule-update: .PHONY: git-submodule-update@@ -471,17 +473,17 @@ qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py
Quoted 7 lines
Makefile qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py · line 476
Quoted 4 lines
Makefile +1
-1qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \ + $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-types.py \But once we agree on the right override to stuff into
PYTHON_UTF8, the rest of the patch converting invocations toPYTHON_UTF8makes sense.
Any thoughts on whether we should apply this more widely to our build to make its output predictable regardless of user's locale ?
Regards,
Daniel
Signature · 3 lines
|: 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 :|
PATCH v4 8/13 scripts: ensure signrom treats data as bytes
In reply to Daniel P. Berrange--- scripts/signrom.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
scripts/signrom.py +2 -2
@@ -18,7 +18,7 @@ fin = open(sys.argv[1], 'rb') fout = open(sys.argv[2], 'wb') magic = fin.read(2) -if magic != '\x55\xaa': +if magic != b'\x55\xaa': sys.exit("%s: option ROM does not begin with magic 55 aa" % sys.argv[1]) size_byte = ord(fin.read(1))
@@ -33,7 +33,7 @@ elif len(data) < size: # Add padding if necessary, rounding the whole input to a multiple of # 512 bytes according to the third byte of the input. # size-1 because a final byte is added below to store the checksum. - data = data.ljust(size-1, '\0') + data = data.ljust(size-1, b'\0') else: if ord(data[-1:]) != 0: sys.stderr.write('WARNING: ROM includes nonzero checksum\n')
Signature · 1 line
2.14.3
PATCH v4 9/13 configure: allow use of python 3
In reply to Daniel P. Berrange--- configure | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
configure +2 -3
@@ -1598,9 +1598,8 @@ fi # Note that if the Python conditional here evaluates True we will exit # with status 1 which is a shell 'false' value. -if ! $python -c 'import sys; sys.exit(sys.version_info < (2,6) or sys.version_info >= (3,))'; then - error_exit "Cannot use '$python', Python 2.6 or later is required." \ - "Note that Python 3 or later is not yet supported." \ +if ! $python -c 'import sys; sys.exit(sys.version_info < (2,6))'; then + error_exit "Cannot use '$python', Python 2 >= 2.6 or Python 3 is required." \ "Use --python=/path/to/python to specify a supported Python." fi
Signature · 1 line
2.14.3
PATCH v4 10/13 input: add missing JIS keys to virtio input
In reply to Daniel P. BerrangeFrom: Miika S <[email protected]>
keycodemapdb updated to add the QKeyCodes muhenkan and katakanahiragana
--- hw/input/virtio-input-hid.c | 7 +++++++ qapi/ui.json | 5 ++++- ui/keycodemapdb | 2 +- 3 files changed, 12 insertions(+), 2 deletions(-)
hw/input/virtio-input-hid.c +7 -0
@@ -139,6 +139,13 @@ static const unsigned int keymap_qcode[Q_KEY_CODE__MAX] = { [Q_KEY_CODE_META_L] = KEY_LEFTMETA, [Q_KEY_CODE_META_R] = KEY_RIGHTMETA, [Q_KEY_CODE_MENU] = KEY_MENU, + + [Q_KEY_CODE_MUHENKAN] = KEY_MUHENKAN, + [Q_KEY_CODE_HENKAN] = KEY_HENKAN, + [Q_KEY_CODE_KATAKANAHIRAGANA] = KEY_KATAKANAHIRAGANA, + [Q_KEY_CODE_COMPOSE] = KEY_COMPOSE, + [Q_KEY_CODE_RO] = KEY_RO, + [Q_KEY_CODE_YEN] = KEY_YEN, };static const unsigned int keymap_button[INPUT_BUTTON__MAX ] = {
qapi/ui.json +4 -1
@@ -748,6 +748,9 @@ # @ac_bookmarks: since 2.10 # altgr, altgr_r: dropped in 2.10 # +# @muhenkan: since 2.12 +# @katakanahiragana: since 2.12 +# # 'sysrq' was mistakenly added to hack around the fact that # the ps2 driver was not generating correct scancodes sequences # when 'alt+print' was pressed. This flaw is now fixed and the
@@ -775,7 +778,7 @@ 'left', 'up', 'down', 'right', 'insert', 'delete', 'stop', 'again', 'props', 'undo', 'front', 'copy', 'open', 'paste', 'find', 'cut', 'lf', 'help', 'meta_l', 'meta_r', 'compose', 'pause', - 'ro', 'hiragana', 'henkan', 'yen', + 'ro', 'hiragana', 'henkan', 'yen', 'muhenkan', 'katakanahiragana', 'kp_comma', 'kp_equals', 'power', 'sleep', 'wake', 'audionext', 'audioprev', 'audiostop', 'audioplay', 'audiomute', 'volumeup', 'volumedown', 'mediaselect',
ui/keycodemapdb +1 -1
@@ -1 +1 @@ -Subproject commit 10739aa26051a5d49d88132604539d3ed085e72e +Subproject commit 05dad417e9d0b37ee1fba33056d91a6b734b3357
Signature · 1 line
2.14.3
Re: [Qemu-devel] [PATCH v4 10/13] input: add missing JIS keys to virtio input
Quoted 10 lines · Daniel P. Berrange
From: Miika S <[email protected]>
keycodemapdb updated to add the QKeyCodes muhenkan and katakanahiragana
Signed-off-by Miika S <[email protected]>--- hw/input/virtio-input-hid.c | 7 +++++++ qapi/ui.json | 5 ++++- ui/keycodemapdb | 2 +- 3 files changed, 12 insertions(+), 2 deletions(-)
Why are you bumping the submodule here and again in 11/13? Can those two patches be squashed together?
Quoted 5 lines
+++ b/ui/keycodemapdb
ui/keycodemapdb +1
-1@@ -1 +1 @@ -Subproject commit 10739aa26051a5d49d88132604539d3ed085e72e +Subproject commit 05dad417e9d0b37ee1fba33056d91a6b734b3357
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v4 10/13] input: add missing JIS keys to virtio input
Quoted 12 lines · Eric Blake
Quoted 10 lines · Daniel P. Berrange
From: Miika S <[email protected]>
keycodemapdb updated to add the QKeyCodes muhenkan and katakanahiragana
Signed-off-by Miika S <[email protected]>--- hw/input/virtio-input-hid.c | 7 +++++++ qapi/ui.json | 5 ++++- ui/keycodemapdb | 2 +- 3 files changed, 12 insertions(+), 2 deletions(-)
Quoted 2 lines
Why are you bumping the submodule here and again in 11/13? Can those two patches be squashed together?
This patch is just temporarily pulled from another patch series, which I expect to get merged before this one does.
Quoted 4 lines
ui/keycodemapdb line 1
Quoted 4 lines
+++ b/ui/keycodemapdb
ui/keycodemapdb +1
-1@@ -1 +1 @@ -Subproject commit 10739aa26051a5d49d88132604539d3ed085e72e +Subproject commit 05dad417e9d0b37ee1fba33056d91a6b734b3357
Regards,
Daniel
Signature · 3 lines
|: 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 :|
PATCH v4 11/13 ui: update keycodemapdb to get py3 fixes
In reply to Daniel P. Berrange--- ui/keycodemapdb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ui/keycodemapdb +1 -1
@@ -1 +1 @@ -Subproject commit 05dad417e9d0b37ee1fba33056d91a6b734b3357 +Subproject commit 6b3d716e2b6472eb7189d3220552280ef3d832ce
Signature · 1 line
2.14.3
PATCH v4 12/13 travis: improve python version test coverage
In reply to Daniel P. BerrangeCurrently travis declares ancient python 2.4 is desired. Update that to 2.6 which is the oldest version any targetted distros still needs. If we just list a python 3 version at the top level this will double the number of travis jobs we run which is unreasonable.
So arbitrarily pick the clang test matrix entries to build with python 3.0 and 3.6, to extend coverage of python versions, without increasing job count or build time.
--- .travis.yml | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
.travis.yml +9 -5
@@ -1,7 +1,7 @@ sudo: false language: c python: - - "2.4" + - "2.6" compiler: - gcc cache: ccache
@@ -115,15 +115,17 @@ matrix: - sudo apt-get build-dep -qq qemu - wget -O - http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar -xvJ - git submodule update --init --recursive - # Trusty System build with latest stable clang + # Trusty System build with latest stable clang & python 3.0 - sudo: required addons: dist: trusty language: generic compiler: none + python: + - "3.0" env: - COMPILER_NAME=clang CXX=clang++-3.9 CC=clang-3.9 - - CONFIG="--disable-linux-user --cc=clang-3.9 --cxx=clang++-3.9" + - CONFIG="--disable-linux-user --cc=clang-3.9 --cxx=clang++-3.9 --python=/usr/bin/python3" before_install: - wget -nv -O - http://llvm.org/apt/llvm-snapshot.gpg.key | sudo apt-key add - - sudo apt-add-repository -y 'deb http://llvm.org/apt/trusty llvm-toolchain-trusty-3.9 main'
@@ -134,15 +136,17 @@ matrix: - git submodule update --init --recursive before_script: - ./configure ${CONFIG} || cat config.log - # Trusty Linux User build with latest stable clang + # Trusty Linux User build with latest stable clang & python 3.6 - sudo: required addons: dist: trusty language: generic compiler: none + python: + - "3.6" env: - COMPILER_NAME=clang CXX=clang++-3.9 CC=clang-3.9 - - CONFIG="--disable-system --cc=clang-3.9 --cxx=clang++-3.9" + - CONFIG="--disable-system --cc=clang-3.9 --cxx=clang++-3.9 --python=/usr/bin/python3" before_install: - wget -nv -O - http://llvm.org/apt/llvm-snapshot.gpg.key | sudo apt-key add - - sudo apt-add-repository -y 'deb http://llvm.org/apt/trusty llvm-toolchain-trusty-3.9 main'
Signature · 1 line
2.14.3
PATCH v4 13/13 docker: change Fedora images to run with python3
In reply to Daniel P. BerrangeFedora has switched to Python 3 by default, so it makes sense to use that for testing QEMU builds, so we get testing of Python 3 compatibility.
--- tests/docker/dockerfiles/fedora.docker | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
tests/docker/dockerfiles/fedora.docker +2 -1
@@ -1,6 +1,6 @@ FROM fedora:latest ENV PACKAGES \ - ccache gettext git tar PyYAML sparse flex bison python2 bzip2 hostname \ + ccache gettext git tar PyYAML sparse flex bison python3 bzip2 hostname \ glib2-devel pixman-devel zlib-devel SDL-devel libfdt-devel \ gcc gcc-c++ clang make perl which bc findutils libaio-devel \ nettle-devel \
@@ -12,6 +12,7 @@ ENV PACKAGES \ mingw64-gtk2 mingw64-gtk3 mingw64-gnutls mingw64-nettle mingw64-libtasn1 \ mingw64-libjpeg-turbo mingw64-libpng mingw64-curl mingw64-libssh2 \ mingw64-bzip2 +ENV QEMU_CONFIGURE_OPTS --python=/usr/bin/python3 RUN dnf install -y $PACKAGES RUN rpm -q $PACKAGES | sort > /packages.txt
Signature · 1 line
2.14.3
Re: [Qemu-devel] [PATCH v4 13/13] docker: change Fedora images to run with python3
Quoted 21 lines · Daniel P. Berrange
Fedora has switched to Python 3 by default, so it makes sense to use that for testing QEMU builds, so we get testing of Python 3 compatibility.
Signed-off-by Daniel P. Berrange <[email protected]>--- tests/docker/dockerfiles/fedora.docker | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)tests/docker/dockerfiles/fedora.docker +1
-1@@ -1,6 +1,6 @@ FROM fedora:latest ENV PACKAGES \ - ccache gettext git tar PyYAML sparse flex bison python2 bzip2 hostname \ + ccache gettext git tar PyYAML sparse flex bison python3 bzip2 hostname \ glib2-devel pixman-devel zlib-devel SDL-devel libfdt-devel \ gcc gcc-c++ clang make perl which bc findutils libaio-devel \ nettle-devel \
tests/docker/dockerfiles/fedora.docker ENV PACKAGES \ · line 18
Quoted 7 lines
tests/docker/dockerfiles/fedora.docker +1
-0@@ -12,6 +12,7 @@ ENV PACKAGES \ mingw64-gtk2 mingw64-gtk3 mingw64-gnutls mingw64-nettle mingw64-libtasn1 \ mingw64-libjpeg-turbo mingw64-libpng mingw64-curl mingw64-libssh2 \ mingw64-bzip2 +ENV QEMU_CONFIGURE_OPTS --python=/usr/bin/python3 RUN dnf install -y $PACKAGES RUN rpm -q $PACKAGES | sort > /packages.txt
QAPI changes stands out in the series' diffstat, so: