Eric Blake <ebl...@redhat.com> writes:
> On 02/11/2018 03:35 AM, Markus Armbruster wrote:
>> Whenever qapi-schema.json changes, we run six programs eleven times to
>> update eleven files. Similar for qga/qapi-schema.json. This is
>> silly. Replace the six programs by a single program that spits out
>> all eleven files.
>>
>> The programs become modules in new Python package qapi, along with the
>> helper library. This requires moving them to scripts/qapi/.
>>
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
>> ---
>> .gitignore | 2 +
>> Makefile | 86 +++++++++----------
>> docs/devel/qapi-code-gen.txt | 97
>> ++++++++++------------
>> monitor.c | 2 +-
>> qapi-schema.json | 2 +-
>> scripts/qapi-gen.py | 41 +++++++++
>> scripts/qapi/__init__.py | 0
>> scripts/{qapi-commands.py => qapi/commands.py} | 23 ++---
>> scripts/{qapi.py => qapi/common.py} | 18 +---
>> 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 | 4 +-
>> 16 files changed, 193 insertions(+), 290 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} (99%)
>> rename scripts/{qapi2texi.py => qapi/doc.py} (92%)
>> mode change 100755 => 100644
>
> Still forgot mention that the mode bit change was intentional, but not
> worth a respin for just that.
>
>> 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%)
>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
>
>> +++ b/docs/devel/qapi-code-gen.txt
>
>> - $ python scripts/qapi-event.py --output-dir="qapi-generated"
>> - --prefix="example-" example-schema.json
>> $ cat qapi-generated/example-qapi-event.h
>> [Uninteresting stuff omitted...]
>> @@ -1302,23 +1296,22 @@ Example:
>> }
>> const char *const example_QAPIEvent_lookup[] = {
>> - [EXAMPLE_QAPI_EVENT_MY_EVENT] = "MY_EVENT",
>> +
>> +[EXAMPLE_QAPI_EVENT_MY_EVENT] = "MY_EVENT",
>> [EXAMPLE_QAPI_EVENT__MAX] = NULL,
>> };
>
> Looks like our generated code indentation has slightly regressed from
> what we would write by hand, but it's still okay for generated code
> (and the commit message in 5/29 did call that out)
I'd prefer to have this tidied up.
>> +++ b/scripts/qapi-gen.py
>
>> +++ b/scripts/qapi/commands.py
>> @@ -13,7 +13,7 @@ This work is licensed under the terms of the GNU GPL,
>> version 2.
>> See the COPYING file in the top-level directory.
>> """
>> -from qapi import *
>> +from qapi.common import *
>> def gen_command_decl(name, arg_type, boxed, ret_type):
>> @@ -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'
>
> We discussed whether to make the assignment to blurb be a one-liner in
> an earlier patch, but I'm also fine with the churn you have over the
> course of the series.
I ran out of time, and decided to leave this one as is.
