Re: [Qemu-devel] [PATCH RFC 06/21] qapi-gen: New common driver for code and doc generators

2018-02-08 Thread Markus Armbruster 
Markus Armbruster  writes:

> Marc-Andre Lureau  writes:
>
>> On Fri, Feb 2, 2018 at 2:03 PM, Markus Armbruster  wrote:
[...]
>>> diff --git a/scripts/qapi2texi.py b/scripts/qapi/doc.py
>>> old mode 100755
>>> new mode 100644
>>> similarity index 92%
>>> rename from scripts/qapi2texi.py
>>> rename to scripts/qapi/doc.py
>>> index 924b374cd3..1f57f6e1c2
>>> --- a/scripts/qapi2texi.py
>>> +++ b/scripts/qapi/doc.py
>>> @@ -4,10 +4,9 @@
>>>  # 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"""
>>> +
>>>  import re
>>> -import sys
>>> -
>>> -import qapi
>>> +import qapi.common
>>>
>>>  MSG_FMT = """
>>>  @deftypefn {type} {{}} {name}
>>> @@ -196,7 +195,7 @@ def texi_entity(doc, what, base=None, variants=None,
>>>  + texi_sections(doc))
>>>
>>>
>>> -class QAPISchemaGenDocVisitor(qapi.QAPISchemaVisitor):
>>> +class QAPISchemaGenDocVisitor(qapi.common.QAPISchemaVisitor):
>>
>> Would be a bit easier to read and more consitent with a top-level
>> "from qapi.common import QAPISchemaVisitor"
>
> Can do.

The obvious patch (appended) doesn't work, because doc_required is
always False in gen_doc().  WTF?!?

[...]


diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 4027722032..919e77b79e 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -7,7 +7,7 @@
 
 from __future__ import print_function
 import re
-import qapi.common
+from qapi.common import doc_required, QAPIGenDoc, QAPISchemaVisitor
 
 MSG_FMT = """
 @deftypefn {type} {{}} {name}
@@ -196,7 +196,7 @@ def texi_entity(doc, what, base=None, variants=None,
 + texi_sections(doc))
 
 
-class QAPISchemaGenDocVisitor(qapi.common.QAPISchemaVisitor):
+class QAPISchemaGenDocVisitor(QAPISchemaVisitor):
 def __init__(self):
 self.out = None
 self.cur_doc = None
@@ -272,7 +272,7 @@ def texi_schema(schema):
 
 
 def gen_doc(schema, output_dir, prefix):
-if qapi.common.doc_required:
-gen = qapi.common.QAPIGenDoc()
+if doc_required:
+gen = QAPIGenDoc()
 gen.add(texi_schema(schema))
 gen.write(output_dir, prefix + 'qapi.texi')

Re: [Qemu-devel] [PATCH RFC 06/21] qapi-gen: New common driver for code and doc generators

2018-02-06 Thread Eric Blake 

On 02/06/2018 01:45 AM, Markus Armbruster wrote:


-def main(argv):
-(input_file, output_dir, do_c, do_h, prefix, opts) = parse_command_line()
-
-blurb = '''
- * Schema-defined QAPI/QMP commands
-'''
-
+def gen_commands(schema, output_dir, prefix):
+blurb = ' * Schema-defined QAPI/QMP commands'


Some churn on the definition of blurb; worth tidying this in the
introduction earlier in the series?


Doesn't seem worth a separate patch, but I can try to reshuffle things
to reduce churn.


Yeah, my question was more about the conversion between multiline
'''...''' to single-line '...'; why not just use the single-line
construct earlier in the series when introducing the blurb variable.


Introduced in PATCH 01:

 -c_comment = '''
 -/*
 - * schema-defined QMP->QAPI command dispatch
 +blurb = '''
 + * Schema-defined QAPI/QMP commands
   *
   * Copyright IBM, Corp. 2011


Multiline made sense here.


Shortened in PATCH 02:

  blurb = '''
   * Schema-defined QAPI/QMP commands
 - *
 - * Copyright IBM, Corp. 2011
 - *
 - * Authors:
 - *  Anthony Liguori   

   '''




Where it is now a single line...



Variable eliminated there in PATCH 17:

 -blurb = ' * Schema-defined QAPI types'
 -self._genc = QAPIGenC(blurb, __doc__)
 -self._genh = QAPIGenH(blurb, __doc__)
 +self._module = {}
 +self._add_module(None, ' * Built-in QAPI types')


Oh, I didn't even notice that!



I could delay the reformatting until PATCH 16, or do it in PATCH 02.
Feels like a wash to me, but if you have a preference...


I guess my preference is to reformat to single line in PATCH 02, then 
patch 6 doesn't have to touch it, and patch 16 can still get rid of it. 
But, as it disappears by the end of the series anyways, I'm also okay if 
you don't bother (churn is not necessarily bad, if it is still easy to 
review).




The commit message talks only about the QAPI/QMP schema.  It's confusing
because our poor taste in file names creates ambiguity: does
qapi-schema.json refer to ./qapi-schema.json (i.e. the QAPI/QMP one),
qga/qapi-schema.json, or both?

Perhaps I should rename qapi-schema.json to qapi/schema.json.


I'd be in favor of such a change.  We'd also rename qga/qapi-schema.json 
to qga/schema.json?




The commit message needs a note along the lines of "same for
qga/qapi-schema.json".



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

Re: [Qemu-devel] [PATCH RFC 06/21] qapi-gen: New common driver for code and doc generators

2018-02-05 Thread Markus Armbruster 
Eric Blake  writes:

> On 02/03/2018 03:03 AM, Markus Armbruster wrote:
>> Eric Blake  writes:
>> 
>>> On 02/02/2018 07:03 AM, Markus Armbruster wrote:
 Whenever qapi-schema.json changes, we run six programs eleven times to
 update eleven files.  This is silly.  Replace the six programs by a
 single program that spits out all eleven files.
>>>
>>> Yay, about time!
>>>
>>> One program, but still invoked multiple times:
>>>
>
  rename scripts/{qapi2texi.py => qapi/doc.py} (92%)
  mode change 100755 => 100644
>>>
>>> Unintinentional?  We're inconsistent on which of our *.py files are
>>> executable in git.  Does it matter whether a file that is designed for
>>> use as a module is marked executable (if so, perhaps 5/21 should be
>>> adding the x attribute on other files, rather than this patch removing
>>> it on the doc generator).
>> 
>> qapi2texi.py is no longer runnable as a standalone program after this
>> patch.
>> 
>> So are qapi-{commands,events,introspect,types,visit}.py, but they never
>> had the executable bit set.
>
> Okay, so dropping the bit consistently makes sense; still, a mention in
> the commit message wouldn't hurt.

Can do.

 +++ b/Makefile
>>>
 +qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h \
 +qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h \
 +qga/qapi-generated/qga-qmp-commands.h 
 qga/qapi-generated/qga-qmp-marshal.c \
 +qga/qapi-generated/qga-qapi.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) $(SRC_PATH)/scripts/qapi-gen.py \
 +  -o qga/qapi-generated -p "qga-" $<, \
 +  "GEN","$(@:%-timestamp=%)")
 +  @>$@
>>>
>>> once for qga,
>> 
>> That's the QAPI/QGA schema.  The commit message talks about the QAPI/QMP
>> schema.  I wish the two weren't named the same.
>
> 7 files here,...
>
>> 
>> Modularization might make fusing them possible.  Whether that's a good
>> idea I don't know.
>> 
 +qapi-types.c qapi-types.h \
 +qapi-visit.c qapi-visit.h \
 +qmp-commands.h qmp-marshal.c \
 +qapi-event.c qapi-event.h \
 +qmp-introspect.h qmp-introspect.c \
 +qapi.texi: \
 +qapi-gen-timestamp ;
 +qapi-gen-timestamp: $(qapi-modules) $(qapi-py)
 +  $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-gen.py \
 +  -o "." -b $<, \
 +  "GEN","$(@:%-timestamp=%)")
 +  @>$@
>>>
>>> and again for the main level.  Still, a definite improvement!
>
> 11 files here,...
>
>> 
>> Perhaps I can find a way to clarify the commit message.
>> 
>
> [1]
>
>
 -def main(argv):
 -(input_file, output_dir, do_c, do_h, prefix, opts) = 
 parse_command_line()
 -
 -blurb = '''
 - * Schema-defined QAPI/QMP commands
 -'''
 -
 +def gen_commands(schema, output_dir, prefix):
 +blurb = ' * Schema-defined QAPI/QMP commands'
>>>
>>> Some churn on the definition of blurb; worth tidying this in the
>>> introduction earlier in the series?
>> 
>> Doesn't seem worth a separate patch, but I can try to reshuffle things
>> to reduce churn.
>
> Yeah, my question was more about the conversion between multiline
> '''...''' to single-line '...'; why not just use the single-line
> construct earlier in the series when introducing the blurb variable.

Introduced in PATCH 01:

-c_comment = '''
-/*
- * schema-defined QMP->QAPI command dispatch
+blurb = '''
+ * Schema-defined QAPI/QMP commands
  *
  * Copyright IBM, Corp. 2011
  *
  * Authors:
  *  Anthony Liguori   
- *
- * This work is licensed under the terms of the GNU LGPL, version 2.1 or 
later.
- * See the COPYING.LIB file in the top-level directory.
- *
- */
-'''

Shortened in PATCH 02:

 blurb = '''
  * Schema-defined QAPI/QMP commands
- *
- * Copyright IBM, Corp. 2011
- *
- * Authors:
- *  Anthony Liguori   
 '''

Reformatted in PATCH 06 (see above).

Moved in PATCH 16 to visitor's __init__ for types and visits (the rest
aren't implemented, yet):

 class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
-def __init__(self, opt_builtins):
+def __init__(self, opt_builtins, prefix):
 self._opt_builtins = opt_builtins
-self.decl = None
-self.defn = None
-self._fwdecl = None
-self._btin = None
+self._prefix = prefix
+blurb = ' * Schema-defined QAPI types'
+self._genc = QAPIGenC(blurb, __doc__)
+self._genh = QAPIGenH(blurb, __doc__)

Variable eliminated there in PATCH 17:

-blurb = ' * Schema-defined QAPI types'
-self._genc = QAPIGenC(blurb, __doc__)
-self._genh = QAPIGenH(blurb, __doc__)
+

Re: [Qemu-devel] [PATCH RFC 06/21] qapi-gen: New common driver for code and doc generators

2018-02-05 Thread Eric Blake 
On 02/03/2018 03:03 AM, Markus Armbruster wrote:
> Eric Blake  writes:
> 
>> On 02/02/2018 07:03 AM, Markus Armbruster wrote:
>>> Whenever qapi-schema.json changes, we run six programs eleven times to
>>> update eleven files.  This is silly.  Replace the six programs by a
>>> single program that spits out all eleven files.
>>
>> Yay, about time!
>>
>> One program, but still invoked multiple times:
>>

>>>  rename scripts/{qapi2texi.py => qapi/doc.py} (92%)
>>>  mode change 100755 => 100644
>>
>> Unintinentional?  We're inconsistent on which of our *.py files are
>> executable in git.  Does it matter whether a file that is designed for
>> use as a module is marked executable (if so, perhaps 5/21 should be
>> adding the x attribute on other files, rather than this patch removing
>> it on the doc generator).
> 
> qapi2texi.py is no longer runnable as a standalone program after this
> patch.
> 
> So are qapi-{commands,events,introspect,types,visit}.py, but they never
> had the executable bit set.

Okay, so dropping the bit consistently makes sense; still, a mention in
the commit message wouldn't hurt.

> 
>>> +++ b/Makefile
>>
>>> +qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h \
>>> +qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h \
>>> +qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c 
>>> \
>>> +qga/qapi-generated/qga-qapi.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) $(SRC_PATH)/scripts/qapi-gen.py \
>>> +   -o qga/qapi-generated -p "qga-" $<, \
>>> +   "GEN","$(@:%-timestamp=%)")
>>> +   @>$@
>>
>> once for qga,
> 
> That's the QAPI/QGA schema.  The commit message talks about the QAPI/QMP
> schema.  I wish the two weren't named the same.

7 files here,...

> 
> Modularization might make fusing them possible.  Whether that's a good
> idea I don't know.
> 
>>> +qapi-types.c qapi-types.h \
>>> +qapi-visit.c qapi-visit.h \
>>> +qmp-commands.h qmp-marshal.c \
>>> +qapi-event.c qapi-event.h \
>>> +qmp-introspect.h qmp-introspect.c \
>>> +qapi.texi: \
>>> +qapi-gen-timestamp ;
>>> +qapi-gen-timestamp: $(qapi-modules) $(qapi-py)
>>> +   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-gen.py \
>>> +   -o "." -b $<, \
>>> +   "GEN","$(@:%-timestamp=%)")
>>> +   @>$@
>>
>> and again for the main level.  Still, a definite improvement!

11 files here,...

> 
> Perhaps I can find a way to clarify the commit message.
> 

[1]


>>> -def main(argv):
>>> -(input_file, output_dir, do_c, do_h, prefix, opts) = 
>>> parse_command_line()
>>> -
>>> -blurb = '''
>>> - * Schema-defined QAPI/QMP commands
>>> -'''
>>> -
>>> +def gen_commands(schema, output_dir, prefix):
>>> +blurb = ' * Schema-defined QAPI/QMP commands'
>>
>> Some churn on the definition of blurb; worth tidying this in the
>> introduction earlier in the series?
> 
> Doesn't seem worth a separate patch, but I can try to reshuffle things
> to reduce churn.

Yeah, my question was more about the conversion between multiline
'''...''' to single-line '...'; why not just use the single-line
construct earlier in the series when introducing the blurb variable.
You are right that creating blurb didn't need a separate patch, just
less churn over the series.

>>>  
>>> -qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py
>>> +# TODO don't duplicate $(SRC_PATH)/Makefile's qapi-py here
>>> +qapi-py = $(SRC_PATH)/scripts/qapi/commands.py \
>>> +$(SRC_PATH)/scripts/qapi/events.py \
>>> +$(SRC_PATH)/scripts/qapi/introspect.py \
>>> +$(SRC_PATH)/scripts/qapi/types.py \
>>> +$(SRC_PATH)/scripts/qapi/visit.py \
>>> +$(SRC_PATH)/scripts/qapi/common.py \
>>> +$(SRC_PATH)/scripts/qapi/doc.py \
>>> +$(SRC_PATH)/scripts/ordereddict.py \
>>> +$(SRC_PATH)/scripts/qapi-gen.py
>>
>> So, were you counting these in the 11 generated files mentioned in the
>> commit message? :)
> 
> I'm not sure I understand what you mean here...

[1] and 9 more files.  So, the commit message only mentioned the 11 QMP
files, rather than all 27 (if I counted right) QAPI generated files.  My
comments were trying to point out that you simplified more than just the
QMP code generation into fewer script invocations, and the counts looked
off since out of the three spots converted, only one of the spots had 11
files.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH RFC 06/21] qapi-gen: New common driver for code and doc generators

2018-02-05 Thread Markus Armbruster 
Marc-Andre Lureau  writes:

> On Fri, Feb 2, 2018 at 2:03 PM, Markus Armbruster  wrote:
>> Whenever qapi-schema.json changes, we run six programs eleven times to
>> update eleven files.  This is silly.  Replace the six programs by a
>> single program that spits out all eleven files.
>>
>
> Now we will need documentation update.

Absolutely!  One reason this is RFC.

> Also greping for the renamed files:
>
> git grep -E 
> 'qapi-commands|qapi2texi|qapi-event.py|qapi-intros|qapi-types.py|qapi-visit.py'
> docs/devel/qapi-code-gen.txt:=== scripts/qapi-types.py ===
> docs/devel/qapi-code-gen.txt:$ python scripts/qapi-types.py
> --output-dir="qapi-generated" \
> docs/devel/qapi-code-gen.txt:=== scripts/qapi-visit.py ===
> docs/devel/qapi-code-gen.txt:$ python scripts/qapi-visit.py
> --output-dir="qapi-generated"
> docs/devel/qapi-code-gen.txt:=== scripts/qapi-commands.py ===
> docs/devel/qapi-code-gen.txt:generated by
> qapi-visit.py are used to
> docs/devel/qapi-code-gen.txt:$ python scripts/qapi-commands.py
> --output-dir="qapi-generated"
> docs/devel/qapi-code-gen.txt:=== scripts/qapi-event.py ===
> docs/devel/qapi-code-gen.txt:$ python scripts/qapi-event.py
> --output-dir="qapi-generated"
> docs/devel/qapi-code-gen.txt:=== scripts/qapi-introspect.py ===
> docs/devel/qapi-code-gen.txt:$ python scripts/qapi-introspect.py
> --output-dir="qapi-generated"
> monitor.c: * qapi-introspect.py's output actually conforms to the schema.
> qapi-schema.json:# Documentation generated with qapi2texi.py is in
> source order, with
>
>> Signed-off-by: Markus Armbruster 
>> ---
>>  Makefile   | 86 
>> ++
>>  scripts/qapi-gen.py| 41 +++
>>  scripts/qapi/__init__.py   |  0
>>  scripts/{qapi-commands.py => qapi/commands.py} | 23 ++
>>  scripts/{qapi.py => qapi/common.py}|  0
>>  scripts/{qapi2texi.py => qapi/doc.py}  | 29 ++--
>>  scripts/{qapi-event.py => qapi/events.py}  | 23 ++
>>  scripts/{qapi-introspect.py => qapi/introspect.py} | 32 ++--
>>  scripts/{qapi-types.py => qapi/types.py}   | 34 ++---
>>  scripts/{qapi-visit.py => qapi/visit.py}   | 34 ++---
>>  tests/Makefile.include | 56 +++---
>>  tests/qapi-schema/test-qapi.py |  2 +-
>>  12 files changed, 140 insertions(+), 220 deletions(-)
>>  create mode 100755 scripts/qapi-gen.py
>>  create mode 100644 scripts/qapi/__init__.py
>>  rename scripts/{qapi-commands.py => qapi/commands.py} (94%)
>>  rename scripts/{qapi.py => qapi/common.py} (100%)
>>  rename scripts/{qapi2texi.py => qapi/doc.py} (92%)
>>  mode change 100755 => 100644
>>  rename scripts/{qapi-event.py => qapi/events.py} (92%)
>>  rename scripts/{qapi-introspect.py => qapi/introspect.py} (90%)
>>  rename scripts/{qapi-types.py => qapi/types.py} (90%)
>>  rename scripts/{qapi-visit.py => qapi/visit.py} (92%)
>>
>> diff --git a/Makefile b/Makefile
>> index af31e8981f..e02f0c13ef 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -92,6 +92,7 @@ GENERATED_FILES += qmp-commands.h qapi-types.h 
>> qapi-visit.h qapi-event.h
>>  GENERATED_FILES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c
>>  GENERATED_FILES += qmp-introspect.h
>>  GENERATED_FILES += qmp-introspect.c
>> +GENERATED_FILES += qapi.texi
>>
>>  GENERATED_FILES += trace/generated-tcg-tracers.h
>>
>> @@ -477,25 +478,26 @@ qemu-ga$(EXESUF): QEMU_CFLAGS += -I qga/qapi-generated
>>  qemu-keymap$(EXESUF): LIBS += $(XKBCOMMON_LIBS)
>>  qemu-keymap$(EXESUF): QEMU_CFLAGS += $(XKBCOMMON_CFLAGS)
>>
>> -gen-out-type = $(subst .,-,$(suffix $@))
>> +qapi-py = $(SRC_PATH)/scripts/qapi/commands.py \
>> +$(SRC_PATH)/scripts/qapi/events.py \
>> +$(SRC_PATH)/scripts/qapi/introspect.py \
>> +$(SRC_PATH)/scripts/qapi/types.py \
>> +$(SRC_PATH)/scripts/qapi/visit.py \
>> +$(SRC_PATH)/scripts/qapi/common.py \
>> +$(SRC_PATH)/scripts/qapi/doc.py \
>> +$(SRC_PATH)/scripts/ordereddict.py \
>> +$(SRC_PATH)/scripts/qapi-gen.py
>>
>> -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 \
>> -   $(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 \
>> -   $(gen-out-type) -o qga/qapi-generated -p "qga-" $<, \
>> -   "GEN","$@")
>>

Re: [Qemu-devel] [PATCH RFC 06/21] qapi-gen: New common driver for code and doc generators

2018-02-05 Thread Marc-Andre Lureau 
On Fri, Feb 2, 2018 at 2:03 PM, Markus Armbruster  wrote:
> Whenever qapi-schema.json changes, we run six programs eleven times to
> update eleven files.  This is silly.  Replace the six programs by a
> single program that spits out all eleven files.
>

Now we will need documentation update.

Also greping for the renamed files:

git grep -E 
'qapi-commands|qapi2texi|qapi-event.py|qapi-intros|qapi-types.py|qapi-visit.py'
docs/devel/qapi-code-gen.txt:=== scripts/qapi-types.py ===
docs/devel/qapi-code-gen.txt:$ python scripts/qapi-types.py
--output-dir="qapi-generated" \
docs/devel/qapi-code-gen.txt:=== scripts/qapi-visit.py ===
docs/devel/qapi-code-gen.txt:$ python scripts/qapi-visit.py
--output-dir="qapi-generated"
docs/devel/qapi-code-gen.txt:=== scripts/qapi-commands.py ===
docs/devel/qapi-code-gen.txt:generated by
qapi-visit.py are used to
docs/devel/qapi-code-gen.txt:$ python scripts/qapi-commands.py
--output-dir="qapi-generated"
docs/devel/qapi-code-gen.txt:=== scripts/qapi-event.py ===
docs/devel/qapi-code-gen.txt:$ python scripts/qapi-event.py
--output-dir="qapi-generated"
docs/devel/qapi-code-gen.txt:=== scripts/qapi-introspect.py ===
docs/devel/qapi-code-gen.txt:$ python scripts/qapi-introspect.py
--output-dir="qapi-generated"
monitor.c: * qapi-introspect.py's output actually conforms to the schema.
qapi-schema.json:# Documentation generated with qapi2texi.py is in
source order, with

> Signed-off-by: Markus Armbruster 
> ---
>  Makefile   | 86 
> ++
>  scripts/qapi-gen.py| 41 +++
>  scripts/qapi/__init__.py   |  0
>  scripts/{qapi-commands.py => qapi/commands.py} | 23 ++
>  scripts/{qapi.py => qapi/common.py}|  0
>  scripts/{qapi2texi.py => qapi/doc.py}  | 29 ++--
>  scripts/{qapi-event.py => qapi/events.py}  | 23 ++
>  scripts/{qapi-introspect.py => qapi/introspect.py} | 32 ++--
>  scripts/{qapi-types.py => qapi/types.py}   | 34 ++---
>  scripts/{qapi-visit.py => qapi/visit.py}   | 34 ++---
>  tests/Makefile.include | 56 +++---
>  tests/qapi-schema/test-qapi.py |  2 +-
>  12 files changed, 140 insertions(+), 220 deletions(-)
>  create mode 100755 scripts/qapi-gen.py
>  create mode 100644 scripts/qapi/__init__.py
>  rename scripts/{qapi-commands.py => qapi/commands.py} (94%)
>  rename scripts/{qapi.py => qapi/common.py} (100%)
>  rename scripts/{qapi2texi.py => qapi/doc.py} (92%)
>  mode change 100755 => 100644
>  rename scripts/{qapi-event.py => qapi/events.py} (92%)
>  rename scripts/{qapi-introspect.py => qapi/introspect.py} (90%)
>  rename scripts/{qapi-types.py => qapi/types.py} (90%)
>  rename scripts/{qapi-visit.py => qapi/visit.py} (92%)
>
> diff --git a/Makefile b/Makefile
> index af31e8981f..e02f0c13ef 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -92,6 +92,7 @@ GENERATED_FILES += qmp-commands.h qapi-types.h qapi-visit.h 
> qapi-event.h
>  GENERATED_FILES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c
>  GENERATED_FILES += qmp-introspect.h
>  GENERATED_FILES += qmp-introspect.c
> +GENERATED_FILES += qapi.texi
>
>  GENERATED_FILES += trace/generated-tcg-tracers.h
>
> @@ -477,25 +478,26 @@ qemu-ga$(EXESUF): QEMU_CFLAGS += -I qga/qapi-generated
>  qemu-keymap$(EXESUF): LIBS += $(XKBCOMMON_LIBS)
>  qemu-keymap$(EXESUF): QEMU_CFLAGS += $(XKBCOMMON_CFLAGS)
>
> -gen-out-type = $(subst .,-,$(suffix $@))
> +qapi-py = $(SRC_PATH)/scripts/qapi/commands.py \
> +$(SRC_PATH)/scripts/qapi/events.py \
> +$(SRC_PATH)/scripts/qapi/introspect.py \
> +$(SRC_PATH)/scripts/qapi/types.py \
> +$(SRC_PATH)/scripts/qapi/visit.py \
> +$(SRC_PATH)/scripts/qapi/common.py \
> +$(SRC_PATH)/scripts/qapi/doc.py \
> +$(SRC_PATH)/scripts/ordereddict.py \
> +$(SRC_PATH)/scripts/qapi-gen.py
>
> -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 \
> -   $(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 \
> -   $(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 \
>

Re: [Qemu-devel] [PATCH RFC 06/21] qapi-gen: New common driver for code and doc generators

2018-02-03 Thread Markus Armbruster 
Eric Blake  writes:

> On 02/02/2018 07:03 AM, Markus Armbruster wrote:
>> Whenever qapi-schema.json changes, we run six programs eleven times to
>> update eleven files.  This is silly.  Replace the six programs by a
>> single program that spits out all eleven files.
>
> Yay, about time!
>
> One program, but still invoked multiple times:
>
>> 
>> Signed-off-by: Markus Armbruster 
>> ---
>>  Makefile   | 86 
>> ++
>>  scripts/qapi-gen.py| 41 +++
>>  scripts/qapi/__init__.py   |  0
>>  scripts/{qapi-commands.py => qapi/commands.py} | 23 ++
>>  scripts/{qapi.py => qapi/common.py}|  0
>>  scripts/{qapi2texi.py => qapi/doc.py}  | 29 ++--
>>  scripts/{qapi-event.py => qapi/events.py}  | 23 ++
>>  scripts/{qapi-introspect.py => qapi/introspect.py} | 32 ++--
>>  scripts/{qapi-types.py => qapi/types.py}   | 34 ++---
>>  scripts/{qapi-visit.py => qapi/visit.py}   | 34 ++---
>
> Maybe the commit message should mention:
>
> This requires moving some files around for proper use in python.

Yes, I should mention that I wrap the QAPI modules in a Python package.

>>  tests/Makefile.include | 56 +++---
>>  tests/qapi-schema/test-qapi.py |  2 +-
>>  12 files changed, 140 insertions(+), 220 deletions(-)
>>  create mode 100755 scripts/qapi-gen.py
>>  create mode 100644 scripts/qapi/__init__.py
>>  rename scripts/{qapi-commands.py => qapi/commands.py} (94%)
>>  rename scripts/{qapi.py => qapi/common.py} (100%)
>>  rename scripts/{qapi2texi.py => qapi/doc.py} (92%)
>>  mode change 100755 => 100644
>
> Unintinentional?  We're inconsistent on which of our *.py files are
> executable in git.  Does it matter whether a file that is designed for
> use as a module is marked executable (if so, perhaps 5/21 should be
> adding the x attribute on other files, rather than this patch removing
> it on the doc generator).

qapi2texi.py is no longer runnable as a standalone program after this
patch.

So are qapi-{commands,events,introspect,types,visit}.py, but they never
had the executable bit set.

>> +++ b/Makefile
>
>> +qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h \
>> +qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h \
>> +qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c \
>> +qga/qapi-generated/qga-qapi.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) $(SRC_PATH)/scripts/qapi-gen.py \
>> +-o qga/qapi-generated -p "qga-" $<, \
>> +"GEN","$(@:%-timestamp=%)")
>> +@>$@
>
> once for qga,

That's the QAPI/QGA schema.  The commit message talks about the QAPI/QMP
schema.  I wish the two weren't named the same.

Modularization might make fusing them possible.  Whether that's a good
idea I don't know.

>> +qapi-types.c qapi-types.h \
>> +qapi-visit.c qapi-visit.h \
>> +qmp-commands.h qmp-marshal.c \
>> +qapi-event.c qapi-event.h \
>> +qmp-introspect.h qmp-introspect.c \
>> +qapi.texi: \
>> +qapi-gen-timestamp ;
>> +qapi-gen-timestamp: $(qapi-modules) $(qapi-py)
>> +$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-gen.py \
>> +-o "." -b $<, \
>> +"GEN","$(@:%-timestamp=%)")
>> +@>$@
>
> and again for the main level.  Still, a definite improvement!

Perhaps I can find a way to clarify the commit message.

>> +++ b/scripts/qapi-gen.py
>
>> +def main(argv):
>> +(input_file, output_dir, do_c, do_h, prefix, opts) = \
>> +parse_command_line('bu', ['builtins', 'unmask-non-abi-names'])
>> +
>> +opt_builtins = False
>> +opt_unmask = False
>> +
>> +for o, a in opts:
>> +if o in ('-b', '--builtins'):
>> +opt_builtins = True
>> +if o in ('-u', '--unmask-non-abi-names'):
>> +opt_unmask = True
>> +
>> +schema = QAPISchema(input_file)
>> +
>> +gen_types(schema, output_dir, prefix, opt_builtins)
>> +gen_visit(schema, output_dir, prefix, opt_builtins)
>> +gen_commands(schema, output_dir, prefix)
>> +gen_events(schema, output_dir, prefix)
>> +gen_introspect(schema, output_dir, prefix, opt_unmask)
>> +gen_doc(schema, output_dir, prefix)
>
> Rather simple, but definitely nicer all in one python file than as a
> series of make rules.
>
>> @@ -255,13 +255,8 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor):
>>  self._regy += gen_register_command(name, success_response)
>>  
>>  
>> -def main(argv):
>> -(input_file, output_dir, do_c, do_h, prefix, opts) = 
>> parse_command_line()
>> -
>> -blurb = '''
>> - * Schema-defined QAPI/QMP commands
>> -'''
>> -
>> +def gen_commands(schema, output_dir,

Re: [Qemu-devel] [PATCH RFC 06/21] qapi-gen: New common driver for code and doc generators

2018-02-02 Thread Eric Blake 
On 02/02/2018 07:03 AM, Markus Armbruster wrote:
> Whenever qapi-schema.json changes, we run six programs eleven times to
> update eleven files.  This is silly.  Replace the six programs by a
> single program that spits out all eleven files.

Yay, about time!

One program, but still invoked multiple times:

> 
> Signed-off-by: Markus Armbruster 
> ---
>  Makefile   | 86 
> ++
>  scripts/qapi-gen.py| 41 +++
>  scripts/qapi/__init__.py   |  0
>  scripts/{qapi-commands.py => qapi/commands.py} | 23 ++
>  scripts/{qapi.py => qapi/common.py}|  0
>  scripts/{qapi2texi.py => qapi/doc.py}  | 29 ++--
>  scripts/{qapi-event.py => qapi/events.py}  | 23 ++
>  scripts/{qapi-introspect.py => qapi/introspect.py} | 32 ++--
>  scripts/{qapi-types.py => qapi/types.py}   | 34 ++---
>  scripts/{qapi-visit.py => qapi/visit.py}   | 34 ++---

Maybe the commit message should mention:

This requires moving some files around for proper use in python.

>  tests/Makefile.include | 56 +++---
>  tests/qapi-schema/test-qapi.py |  2 +-
>  12 files changed, 140 insertions(+), 220 deletions(-)
>  create mode 100755 scripts/qapi-gen.py
>  create mode 100644 scripts/qapi/__init__.py
>  rename scripts/{qapi-commands.py => qapi/commands.py} (94%)
>  rename scripts/{qapi.py => qapi/common.py} (100%)
>  rename scripts/{qapi2texi.py => qapi/doc.py} (92%)
>  mode change 100755 => 100644

Unintinentional?  We're inconsistent on which of our *.py files are
executable in git.  Does it matter whether a file that is designed for
use as a module is marked executable (if so, perhaps 5/21 should be
adding the x attribute on other files, rather than this patch removing
it on the doc generator).

> +++ b/Makefile

> +qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h \
> +qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h \
> +qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c \
> +qga/qapi-generated/qga-qapi.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) $(SRC_PATH)/scripts/qapi-gen.py \
> + -o qga/qapi-generated -p "qga-" $<, \
> + "GEN","$(@:%-timestamp=%)")
> + @>$@

once for qga,


> +qapi-types.c qapi-types.h \
> +qapi-visit.c qapi-visit.h \
> +qmp-commands.h qmp-marshal.c \
> +qapi-event.c qapi-event.h \
> +qmp-introspect.h qmp-introspect.c \
> +qapi.texi: \
> +qapi-gen-timestamp ;
> +qapi-gen-timestamp: $(qapi-modules) $(qapi-py)
> + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-gen.py \
> + -o "." -b $<, \
> + "GEN","$(@:%-timestamp=%)")
> + @>$@

and again for the main level.  Still, a definite improvement!

> +++ b/scripts/qapi-gen.py

> +def main(argv):
> +(input_file, output_dir, do_c, do_h, prefix, opts) = \
> +parse_command_line('bu', ['builtins', 'unmask-non-abi-names'])
> +
> +opt_builtins = False
> +opt_unmask = False
> +
> +for o, a in opts:
> +if o in ('-b', '--builtins'):
> +opt_builtins = True
> +if o in ('-u', '--unmask-non-abi-names'):
> +opt_unmask = True
> +
> +schema = QAPISchema(input_file)
> +
> +gen_types(schema, output_dir, prefix, opt_builtins)
> +gen_visit(schema, output_dir, prefix, opt_builtins)
> +gen_commands(schema, output_dir, prefix)
> +gen_events(schema, output_dir, prefix)
> +gen_introspect(schema, output_dir, prefix, opt_unmask)
> +gen_doc(schema, output_dir, prefix)

Rather simple, but definitely nicer all in one python file than as a
series of make rules.

> @@ -255,13 +255,8 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor):
>  self._regy += gen_register_command(name, success_response)
>  
>  
> -def main(argv):
> -(input_file, output_dir, do_c, do_h, prefix, opts) = parse_command_line()
> -
> -blurb = '''
> - * Schema-defined QAPI/QMP commands
> -'''
> -
> +def gen_commands(schema, output_dir, prefix):
> +blurb = ' * Schema-defined QAPI/QMP commands'

Some churn on the definition of blurb; worth tidying this in the
introduction earlier in the series?

> rename to scripts/qapi/introspect.py
> index 09e7d1f140..2153060f2c 100644
> --- a/scripts/qapi-introspect.py
> +++ b/scripts/qapi/introspect.py
> @@ -10,7 +10,7 @@ This work is licensed under the terms of the GNU GPL, 
> version 2.
>  See the COPYING file in the top-level directory.
>  """
>  
> -from qapi import *
> +from qapi.common import *
>  
>  
>  # Caveman's json.dumps() replacement (we're stuck at Python 2.4)
> @@ -168,22 +168,8 @@ const char %(c_name)s[] = %(c_string)s;
>

[Qemu-devel] [PATCH RFC 06/21] qapi-gen: New common driver for code and doc generators

2018-02-02 Thread Markus Armbruster 
Whenever qapi-schema.json changes, we run six programs eleven times to
update eleven files.  This is silly.  Replace the six programs by a
single program that spits out all eleven files.

Signed-off-by: Markus Armbruster 
---
 Makefile   | 86 ++
 scripts/qapi-gen.py| 41 +++
 scripts/qapi/__init__.py   |  0
 scripts/{qapi-commands.py => qapi/commands.py} | 23 ++
 scripts/{qapi.py => qapi/common.py}|  0
 scripts/{qapi2texi.py => qapi/doc.py}  | 29 ++--
 scripts/{qapi-event.py => qapi/events.py}  | 23 ++
 scripts/{qapi-introspect.py => qapi/introspect.py} | 32 ++--
 scripts/{qapi-types.py => qapi/types.py}   | 34 ++---
 scripts/{qapi-visit.py => qapi/visit.py}   | 34 ++---
 tests/Makefile.include | 56 +++---
 tests/qapi-schema/test-qapi.py |  2 +-
 12 files changed, 140 insertions(+), 220 deletions(-)
 create mode 100755 scripts/qapi-gen.py
 create mode 100644 scripts/qapi/__init__.py
 rename scripts/{qapi-commands.py => qapi/commands.py} (94%)
 rename scripts/{qapi.py => qapi/common.py} (100%)
 rename scripts/{qapi2texi.py => qapi/doc.py} (92%)
 mode change 100755 => 100644
 rename scripts/{qapi-event.py => qapi/events.py} (92%)
 rename scripts/{qapi-introspect.py => qapi/introspect.py} (90%)
 rename scripts/{qapi-types.py => qapi/types.py} (90%)
 rename scripts/{qapi-visit.py => qapi/visit.py} (92%)

diff --git a/Makefile b/Makefile
index af31e8981f..e02f0c13ef 100644
--- a/Makefile
+++ b/Makefile
@@ -92,6 +92,7 @@ GENERATED_FILES += qmp-commands.h qapi-types.h qapi-visit.h 
qapi-event.h
 GENERATED_FILES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c
 GENERATED_FILES += qmp-introspect.h
 GENERATED_FILES += qmp-introspect.c
+GENERATED_FILES += qapi.texi
 
 GENERATED_FILES += trace/generated-tcg-tracers.h
 
@@ -477,25 +478,26 @@ qemu-ga$(EXESUF): QEMU_CFLAGS += -I qga/qapi-generated
 qemu-keymap$(EXESUF): LIBS += $(XKBCOMMON_LIBS)
 qemu-keymap$(EXESUF): QEMU_CFLAGS += $(XKBCOMMON_CFLAGS)
 
-gen-out-type = $(subst .,-,$(suffix $@))
+qapi-py = $(SRC_PATH)/scripts/qapi/commands.py \
+$(SRC_PATH)/scripts/qapi/events.py \
+$(SRC_PATH)/scripts/qapi/introspect.py \
+$(SRC_PATH)/scripts/qapi/types.py \
+$(SRC_PATH)/scripts/qapi/visit.py \
+$(SRC_PATH)/scripts/qapi/common.py \
+$(SRC_PATH)/scripts/qapi/doc.py \
+$(SRC_PATH)/scripts/ordereddict.py \
+$(SRC_PATH)/scripts/qapi-gen.py
 
-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 \
-   $(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 \
-   $(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 \
-   $(gen-out-type) -o qga/qapi-generated -p "qga-" $<, \
-   "GEN","$@")
+qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h \
+qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h \
+qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c \
+qga/qapi-generated/qga-qapi.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) $(SRC_PATH)/scripts/qapi-gen.py \
+   -o qga/qapi-generated -p "qga-" $<, \
+   "GEN","$(@:%-timestamp=%)")
+   @>$@
 
 qapi-modules = $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qapi/common.json \
$(SRC_PATH)/qapi/block.json $(SRC_PATH)/qapi/block-core.json \
@@ -512,31 +514,18 @@ qapi-modules = $(SRC_PATH)/qapi-schema.json 
$(SRC_PATH)/qapi/common.json \
$(SRC_PATH)/qapi/transaction.json \
$(SRC_PATH)/qapi/ui.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 \
-   $(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)