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

[Markus Armbruster <armbru@redhat.com>]

Victor Toso <victortoso@redhat.com> writes:

> Hi,
>
> On Tue, Jul 05, 2022 at 08:46:34AM -0700, Andrea Bolognani wrote:
>> I've commented in detail to the single patches, just a couple of
>> additional points.
>>
>> On Fri, Jun 17, 2022 at 02:19:24PM +0200, Victor Toso wrote:
>> > * 7) Flat structs by removing embed types. Discussion with Andrea
>> >      Thread: https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg01590.html
>> >
>> >      No one required it but I decided to give it a try. Major issue that
>> >      I see with this approach is to have generated a few 'Base' structs
>> >      that are now useless. Overall, less nested structs seems better to
>> >      me. Opnions?
>> >
>> >      Example:
>> >       | /* This is now useless, should be removed? */
>> >       | type InetSocketAddressBase struct {
>> >       |     Host string `json:"host"`
>> >       |     Port string `json:"port"`
>> >       | }
>>
>> Can we somehow keep track, in the generator, of types that are
>> only used as building blocks for other types, and prevent them
>> from showing up in the generated code?
>
> I'm not 100% sure it is good to remove them from generated code
> because technically it is a valid qapi type. If all @base types
> are embed types and they don't show in other way or form, sure we
> can remove them from generated code... I'm not sure if it is
> possible to guarantee this.
>
> But yes, if possible, I'd like to remove what seems useless type
> definitions.

The existing C generators have to generate all the types, because the
generated code is for QEMU's own use, where we need all the types.

The existing introspection generator generates only the types visible in
QAPI/QMP introspection.

The former generate for internal use (where we want all the types), and
the latter for external use (where only the types visible in the
external interface are actually useful).

>> Finally, looking at the repository containing the generated
>> code I see that the generated type are sorted by kind, e.g. all
>> unions are in a file, all events in another one and so on. I
>> believe the structure should match more closely that of the
>> QAPI schema, so e.g.  block-related types should all go in one
>> file, net-related types in another one and so on.
>
> That's something I don't mind adding but some hardcoded mapping
> is needed. If you look into git history of qapi/ folder, .json
> files can come and go, types be moved around, etc. So, we need to
> proper map types in a way that the generated code would be kept
> stable even if qapi files would have been rearranged. What I
> proposed was only the simplest solution.
>
> Also, the generator takes a qapi-schema.json as input. We are
> more focused in qemu/qapi/qapi-schema.json generated coded but
> would not hurt to think we could even use it for qemu-guest-agent
> from qemu/qga/qapi-schema.json -- this to say that the hardcoded
> mapping needs to take into account non qemu qapi schemas too.

In the beginning, the QAPI schema was monolithic.  qga/qapi-schema.json
still is.

When keeping everything in a single qapi-schema.json became unwieldy, we
split it into "modules" tied together with a simple include directive.
Generated code remained monolithic.

When monolithic generated code became too annoying (touch schema,
recompile everything), we made it match the module structure: code for
FOO.json goes into *-FOO.c and *-FOO.h, where the *-FOO.h #include the
generated headers for the .json modules FOO.json includes.

Schema code motion hasn't been much of a problem.  Moving from FOO.json
to one of the modules it includes is transparent.  Non-transparent moves
are relatively rare as long as the split into modules actually makes
sense.

>> Looking forward to the next iteration :)
>
> Me too, thanks again!
>
> Cheers,
> Victor