Re: [RFC PATCH v2 0/8] qapi: add generator for Golang interface

[Markus Armbruster <armbru@redhat.com>]

Victor Toso <victortoso@redhat.com> writes:

> Hi Markus,
>
> On Mon, Jun 27, 2022 at 09:15:53AM +0200, Markus Armbruster wrote:
>> Victor Toso <victortoso@redhat.com> writes:
>> 
>> > Hi,
>> >
>> > This is the second iteration of RFC v1:
>> >   https://lists.gnu.org/archive/html/qemu-devel/2022-04/msg00226.html
>> >
>> >
>> > # What this is about?
>> >
>> > To generate a simple Golang interface that could communicate with QEMU
>> > over QMP. The Go code that is generated is meant to be used as the bare
>> > bones to exchange QMP messages.
>> >
>> > The goal is to have this as a Go module in QEMU gitlab namespace,
>> > similar to what have been done to pyhon-qemu-qmp
>> >   https://gitlab.com/qemu-project/python-qemu-qmp
>> 
>> Aspects of review:
>> 
>> (1) Impact on common code, if any
>> 
>>     I care, because any messes made there are likely to affect me down
>>     the road.
>
> For the first version of the Go generated interface, my goal is
> to have something that works and can be considered alpha to other
> Go projects.
>
> With this first version, I don't want to bring huge changes to
> the python library or to the QAPI spec and its usage in QEMU.
> Unless someone finds that a necessity.
>
> So I hope (1) is simple :)
>
>> (2) The generated Go code
>> 
>>     Is it (close to) what we want long term?  If not, is it good enough
>>     short term, and how could we make necessary improvements?
>> 
>>     I'd prefer to leave this to folks who actually know their Go.
>> (3) General Python sanity
>> 
>>     We need eyes, but not necessarily mine.  Any takers?
>> 
>> [...]
>> 
>> >  scripts/qapi/golang.py | 765 +++++++++++++++++++++++++++++++++++++++++
>> >  scripts/qapi/main.py   |   2 +
>> >  2 files changed, 767 insertions(+)
>> >  create mode 100644 scripts/qapi/golang.py
>> 
>> This adds a new generator and calls it from generate(), i.e.
>> review aspect (1) is empty.  "Empty" is a quick & easy way to
>> get my ACK!
>> 
>> No tests?
>
> I've added tests but on the qapi-go module, those are the files
> with _test.go prefix on them. Example for commands:
>
>     https://gitlab.com/victortoso/qapi-go/-/blob/main/pkg/qapi/commands_test.go
>
> Should the generator itself have tests or offloading that to the
> qapi-go seems reasonable?

Offloading may be reasonable, but how am I to run the tests then?
Documentation should tell me.

We have roughly three kinds of tests so far:

1. Front end tests in tests/qapi-schema

2. Unit tests in tests/unit/

   To find them:

        $ git-grep '#include ".*qapi-.*\.h"' tests/unit/

3. Many tests talking QMP in tests/qtest/

Since you leave the front end alone, you don't need the first kind.

The other two kinds are less clear.

>> No documentation?
>
> Yes, this iteration removed the documentation of the generated
> types. I'm a bit sad about that. I want to consume the
> documentation in the QAPI files to provide the latest info from
> types/fields but we can't 'copy' it, the only solution is 'refer'
> to it with hyperlink, which I haven't done yet.

Two kinds of documentation: generated documentation for the generated Go
code, and documentation about the generator.  I was thinking of the
latter.  Specifically, docs/devel/qapi-code-gen.rst section "Code
generation".  Opinions on its usefulness differ.  I like it.