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

[Victor Toso <victortoso@redhat.com>]

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

Hi,

On Mon, Jun 27, 2022 at 05:29:26PM +0200, Markus Armbruster wrote:
> 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/

I'm thinking on the tests in QEMU side. Perhaps adding something
with Avocado that generates the qapi-go and communicates with a
running QEMU with that generated Go module?

One thing that I try to keep in mind is to not add Go
dependencies in QEMU and this Golang work is not internal to QEMU
itself.

> Since you leave the front end alone, you don't need the first
> kind.
>
> The other two kinds are less clear.

I'm open for suggestions. I thought that, if we have a qapi-go Go
module in Gitlab's qemu-project namespace, we could leverage most
of the tests to the consumer of the actual generated code but I
agree that it is necessary to have something in qemu too.

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

Me too. I'll add documentation for the next iteration, thanks for
pointing it out.

Cheers,
Victor

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]