Skip to contentSkip to conversation

qapi: support py2 & py3 in parallel

PATCH Superseded Daniel P. Berrange 4 participants 10 messages Aug 31, 2017 – Sep 8, 2017

v2 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

v2 cover qapi: support py2 & py3 in parallel

Since I claimed that supporting py2 & py3 in parallel would be easy for QEMU, I figured I ought to actually give it a try to backup that assertion.

This small patch series is the result of that effort. I tested this series on Fedora 26 using 2.7.13 and Python 3.6.2.

To test with py3, I hacked config-host.mak to change the PYTHON variable to point to 'python3' binary, then compared the following generated content for the files:

qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c qmp-introspect.c qmp-introspect.h

with that generated under py2 to see they are identical.

It is possible there's still more bugs hiding that could impact on 2.6 or earlier versions of 3.x or 2.7.x, so this probably needs a bit wider testing, but I think the series illustrates the broad scope of the changes we can expect. Only the need to adapt to different module import locations adds to the line count, and that's fairly minimal.

Daniel P. Berrange (5):
qapi: convert to use python print function instead of statement
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

 scripts/qapi.py      | 41 +++++++++++++++++++++++++----------------
 scripts/qapi2texi.py | 11 ++++++-----
 2 files changed, 31 insertions(+), 21 deletions(-)
Signature · 1 line
2.13.5
#1 lore raw

PATCH v2 1/5 qapi: convert to use python print function instead of statement

Get Py2 + 3 compatibility by using the print function instead of print statement. This works for 2.6 onwards.

---
 scripts/qapi.py      | 12 ++++++------
 scripts/qapi2texi.py |  9 +++++----
 2 files changed, 11 insertions(+), 10 deletions(-)
scripts/qapi.py +6 -6
index 8aa2775f12..a60b79c126 100644
@@ -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
@@ -1476,7 +1477,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):
@@ -1936,7 +1937,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 = ''
@@ -1950,9 +1951,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'):
@@ -1969,7 +1969,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 +5 -4
index a317526e51..9c57b01bb4 100755
@@ -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
 
@@ -285,15 +286,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__':
Signature · 1 line
2.13.5
#2 lore raw

PATCH v2 2/5 qapi: use items()/values() intead of iteritems()/itervalues()

In reply to Daniel P. Berrange

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      | 12 ++++++------
 scripts/qapi2texi.py |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)
scripts/qapi.py +6 -6
index a60b79c126..d89af7d6c6 100644
@@ -253,7 +253,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(
@@ -309,7 +309,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,
@@ -1575,7 +1575,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']
@@ -1607,11 +1607,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)
@@ -1626,7 +1626,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,
scripts/qapi2texi.py +1 -1
index 9c57b01bb4..bc43edabd7 100755
@@ -148,7 +148,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.content:
             desc = texi_format(str(section))
Signature · 1 line
2.13.5
#3 lore raw

PATCH v2 3/5 qapi: Use OrderedDict from standard library if available

In reply to Daniel P. Berrange

The 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
index d89af7d6c6..05cb1ee38c 100644
@@ -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.13.5
#4 lore raw

Re: [Qemu-devel] [PATCH v2 3/5] qapi: Use OrderedDict from standard library if available

Quoted 3 lines · Daniel P. Berrange

The OrderedDict class appeared in the 'collections' module from python 2.7 onwards, so use that in preference to our local backport if available.

Since we're now using argparse.py as a third-party import (commit 47e1cb1f) also for the sake of 2.6, can we treat ordereddict.py as the same sort of third-party import?

Quoted 23 lines
---
 scripts/qapi.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
scripts/qapi.py +4 -1
index d89af7d6c6..05cb1ee38c 100644
@@ -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 · 3 lines
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
#5 lore raw

PATCH v2 4/5 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
index 05cb1ee38c..3c06711e2c 100644
@@ -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',
@@ -2000,8 +2004,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.13.5
#6 lore raw

PATCH v2 5/5 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
index 3c06711e2c..853622badd 100644
@@ -1743,7 +1743,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.13.5
#7 lore raw
In reply to Daniel P. Berrange
Quoted 15 lines · Daniel P. Berrange

Since I claimed that supporting py2 & py3 in parallel would be easy for QEMU, I figured I ought to actually give it a try to backup that assertion.

This small patch series is the result of that effort. I tested this series on Fedora 26 using 2.7.13 and Python 3.6.2.

To test with py3, I hacked config-host.mak to change the PYTHON variable to point to 'python3' binary, then compared the following generated content for the files:

qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c qmp-introspect.c qmp-introspect.h

Quoted 8 lines

with that generated under py2 to see they are identical.

It is possible there's still more bugs hiding that could impact on 2.6 or earlier versions of 3.x or 2.7.x, so this probably needs a bit wider testing, but I think the series illustrates the broad scope of the changes we can expect. Only the need to adapt to different module import locations adds to the line count, and that's fairly minimal.

This hasn't made it to the front of my review queue, but I got a quick question meanwhile.

I guess this was triggered by the discussion of David's "[PATCH]
scripts: Support building with Python 3". How does it differ from David's patch? Is it derived from it? Or is it an independently developed replacement?

#8 lore raw
Quoted 26 lines · Markus Armbruster
Quoted 15 lines · Daniel P. Berrange

Since I claimed that supporting py2 & py3 in parallel would be easy for QEMU, I figured I ought to actually give it a try to backup that assertion.

This small patch series is the result of that effort. I tested this series on Fedora 26 using 2.7.13 and Python 3.6.2.

To test with py3, I hacked config-host.mak to change the PYTHON variable to point to 'python3' binary, then compared the following generated content for the files:

qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c qmp-introspect.c qmp-introspect.h

Quoted 8 lines

with that generated under py2 to see they are identical.

It is possible there's still more bugs hiding that could impact on 2.6 or earlier versions of 3.x or 2.7.x, so this probably needs a bit wider testing, but I think the series illustrates the broad scope of the changes we can expect. Only the need to adapt to different module import locations adds to the line count, and that's fairly minimal.

Quoted 7 lines

This hasn't made it to the front of my review queue, but I got a quick question meanwhile.

I guess this was triggered by the discussion of David's "[PATCH]
scripts: Support building with Python 3". How does it differ from David's patch? Is it derived from it? Or is it an independently developed replacement?

I didn't notice David's existed. In terms of the qapi file changes, his patch appears equivelent to combining my patches 2->5.

He didn't seem to convert the print statement to a print function though (my patch 1), so I'm surprised his changes actually work with py3...

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 :|
#9 lore raw
Quoted 35 lines · Daniel P. Berrange
Quoted 26 lines · Markus Armbruster
Quoted 15 lines · Daniel P. Berrange

Since I claimed that supporting py2 & py3 in parallel would be easy for QEMU, I figured I ought to actually give it a try to backup that assertion.

This small patch series is the result of that effort. I tested this series on Fedora 26 using 2.7.13 and Python 3.6.2.

To test with py3, I hacked config-host.mak to change the PYTHON variable to point to 'python3' binary, then compared the following generated content for the files:

qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c qmp-introspect.c qmp-introspect.h

Quoted 8 lines

with that generated under py2 to see they are identical.

It is possible there's still more bugs hiding that could impact on 2.6 or earlier versions of 3.x or 2.7.x, so this probably needs a bit wider testing, but I think the series illustrates the broad scope of the changes we can expect. Only the need to adapt to different module import locations adds to the line count, and that's fairly minimal.

Quoted 7 lines

This hasn't made it to the front of my review queue, but I got a quick question meanwhile.

I guess this was triggered by the discussion of David's "[PATCH]
scripts: Support building with Python 3". How does it differ from David's patch? Is it derived from it? Or is it an independently developed replacement?

Quoted 6 lines

I didn't notice David's existed. In terms of the qapi file changes, his patch appears equivelent to combining my patches 2->5.

He didn't seem to convert the print statement to a print function though (my patch 1), so I'm surprised his changes actually work with py3...

It looks like all of those qapi.py print statements are for handling errors, so my builds must not have hit any of them. I had skimmed the output of pylint for errors, and it seems the print statements are only listed as warnings when printing to stderr, which I had overlooked.

    W:1945, 8: Expression "((print) >> (sys.stderr), ('%s: %s') %

((sys.argv[0], str(err))))" is assigned to nothing (expression-not-assigned

And instead of importing the print function, I just switched to sys.stderr.write etc. in qapi2texi.py.

Thanks.

David