[Qemu-devel] [PATCH v2 0/5] qapi: support py2 & py3 in parallel

[Qemu-devel] [PATCH v2 0/5] qapi: support py2 & py3 in parallel

[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

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(-)

-- 
2.13.5

[Qemu-devel] [PATCH v2 1/5] qapi: convert to use python print function instead of statement

[Daniel P. Berrange]

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

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 scripts/qapi.py      | 12 ++++++------
 scripts/qapi2texi.py |  9 +++++----
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 8aa2775f12..a60b79c126 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -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]
 
diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index a317526e51..9c57b01bb4 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -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__':
-- 
2.13.5

[Qemu-devel] [PATCH v2 2/5] qapi: use items()/values() intead of iteritems()/itervalues()

[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

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 scripts/qapi.py      | 12 ++++++------
 scripts/qapi2texi.py |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index a60b79c126..d89af7d6c6 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -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,
diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index 9c57b01bb4..bc43edabd7 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -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))
-- 
2.13.5

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

[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.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 scripts/qapi.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index d89af7d6c6..05cb1ee38c 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -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',
-- 
2.13.5

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1103 bytes --]

On 08/31/2017 09:24 AM, Daniel P. Berrange wrote:
> 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?

> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  scripts/qapi.py | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index d89af7d6c6..05cb1ee38c 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -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',
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

[Qemu-devel] [PATCH v2 4/5] qapi: adapt to moved location of StringIO module in py3

[Daniel P. Berrange]

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 scripts/qapi.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 05cb1ee38c..3c06711e2c 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -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')
-- 
2.13.5

[Qemu-devel] [PATCH v2 5/5] qapi: Adapt to moved location of 'maketrans' function in py3

[Daniel P. Berrange]

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 scripts/qapi.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 3c06711e2c..853622badd 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -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.
-- 
2.13.5

Re: [Qemu-devel] [PATCH v2 0/5] qapi: support py2 & py3 in parallel

[Markus Armbruster]

"Daniel P. Berrange" <berrange@redhat.com> writes:

> 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.

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?

Re: [Qemu-devel] [PATCH v2 0/5] qapi: support py2 & py3 in parallel

[Daniel P. Berrange]

On Fri, Sep 08, 2017 at 11:33:01AM +0200, Markus Armbruster wrote:
> "Daniel P. Berrange" <berrange@redhat.com> writes:
> 
> > 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.
> 
> 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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 0/5] qapi: support py2 & py3 in parallel

[David Michael]

On Fri, Sep 8, 2017 at 2:40 AM, Daniel P. Berrange <berrange@redhat.com> wrote:
> On Fri, Sep 08, 2017 at 11:33:01AM +0200, Markus Armbruster wrote:
>> "Daniel P. Berrange" <berrange@redhat.com> writes:
>>
>> > 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.
>>
>> 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...

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