Re: [PATCH 3/6] qapi: Generate command registration stuff into separate files

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 11/20/19 12:25 PM, Markus Armbruster wrote:
>> Having to include qapi-commands.h just for qmp_init_marshal() is
>> suboptimal.  Generate it into separate files.  This lets
>> monitor/misc.c, qga/main.c, and the generated qapi-commands-FOO.h
>> include less.
>>
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>
>> +++ b/docs/devel/qapi-code-gen.txt
>> @@ -1493,6 +1493,10 @@ $(prefix)qapi-commands.c: Command marshal/dispatch 
>> functions for each
>>   $(prefix)qapi-commands.h: Function prototypes for the QMP commands
>>                             specified in the schema
>>   +$(prefix)qapi-init-commands.h - Command initialization prototype
>> +
>> +$(prefix)qapi-init-commands.h - Command initialization code
>
> Looks like you meant s/h/c/

I did.  Thanks!

>> +    #endif /* EXAMPLE_QAPI_INIT_COMMANDS_H */
>> +    $ cat qapi-generated/example-qapi-init-commands.
>> +[Uninteresting stuff omitted...]
>
> missing a 'c'

Yes.

>> +++ b/Makefile
>
>>   -QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h
>> qga-qapi-visit.h qga-qapi-commands.h)
>> +QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h 
>> qga-qapi-visit.h qga-qapi-commands.h qga-qapi-init-commands.h)
>
> Worth using \ for line-wrapping?

Hmm, it's used only in the following line, could just as well inline
there and line-wrap that:

    $(qga-obj-y): $(QGALIB_GEN)

Hmm^2, why do we even have that line?  The compiler-generated .d should
take care of dependencies.  Dig, dig, ...

    commit 016c77ad62a8ad607dd4349d8cb8ad1365bab831
    Author: Michael Roth <address@hidden>
    Date:   Tue Jul 26 11:39:24 2011 -0500

        Makefile: add missing deps on $(GENERATED_HEADERS)

        This fixes a build issue with make -j6+ due to qapi-generated files
        being built before $(GENERATED_HEADERS) have been created.

        Tested-by: Stefan Berger <address@hidden>
        Signed-off-by: Michael Roth <address@hidden>
        Signed-off-by: Stefan Hajnoczi <address@hidden>

Yes, but the same issue exists for all the other generated headers, and
we solve it differently: put them into $(generated-files-y), have
Makefile depend on them.

I'll clean this up on top.  No need to beautify lines now that will go
away then.

> With those addressed,
> Reviewed-by: Eric Blake <address@hidden>

Thanks!