Re: [Qemu-devel] [PATCH 4/7] qapi: Fix code generation for sub-modules in other directories

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> On 3/1/19 9:40 AM, Markus Armbruster wrote:
>> The #include directives to pull in sub-modules use file names relative
>> to the main module.  Works only when all modules are in the same
>> directory, or the main module's output directory is in the compiler's
>> include path.  Use relative file names instead.
>> 
>> The dummy variable we generate to avoid empty .o files has an invalid
>> name for sub-modules in other directories.  Fix that.
>> 
>> Both messed up in commit 252dc3105fc "qapi: Generate separate .h, .c
>> for each module".  Escaped testing because tests/qapi-schema-test.json
>> doesn't cover sub-modules in other directories, only
>> tests/qapi-schema/include-relpath.json does, and we generate and
>> compile C code only for the former, not the latter.  Fold the latter
>> into the former.  This would have caught the mistakes fixed in this
>> commit.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>
>> +++ b/tests/.gitignore
>> @@ -11,9 +11,17 @@ test-*
>>  !test-*.c
>>  !docker/test-*
>>  test-qapi-commands.[ch]
>> +include/test-qapi-commands-sub-module.[ch]
>> +test-qapi-commands-sub-sub-module.[ch]
>>  test-qapi-events.[ch]
>> +include/test-qapi-events-sub-module.[ch]
>> +test-qapi-events-sub-sub-module.[ch]
>>  test-qapi-types.[ch]
>> +include/test-qapi-types-sub-module.[ch]
>> +test-qapi-types-sub-sub-module.[ch]
>>  test-qapi-visit.[ch]
>> +include/test-qapi-visit-sub-module.[ch]
>> +test-qapi-visit-sub-sub-module.[ch]
>>  test-qapi-introspect.[ch]
>>  *-test
>
> Messes with sorting, and makes me wonder if any globs could compress it
> further, but I can live with it (as I don't have any concrete
> suggestions on how better to write it).

I do: support only out-of-tree builds ;-P

>> @@ -505,8 +504,18 @@ qapi-schema += unknown-expr-key.json
>>  
>>  check-qapi-schema-y := $(addprefix tests/qapi-schema/, $(qapi-schema))
>>  
>> -GENERATED_FILES += tests/test-qapi-types.h tests/test-qapi-visit.h \
>> -	tests/test-qapi-commands.h tests/test-qapi-events.h \
>> +GENERATED_FILES += tests/test-qapi-types.h \
>> +	tests/include/test-qapi-types-sub-module.h \
>> +	tests/test-qapi-types-sub-sub-module.h \
>> +	tests/test-qapi-visit.h \
>> +	tests/include/test-qapi-visit-sub-module.h \
>> +	tests/test-qapi-visit-sub-sub-module.h \
>> +	tests/test-qapi-commands.h \
>> +	tests/include/test-qapi-commands-sub-module.h \
>> +	tests/test-qapi-commands-sub-sub-module.h \
>> +	tests/test-qapi-events.h \
>> +	tests/include/test-qapi-events-sub-module.h \
>> +	tests/test-qapi-events-sub-sub-module.h \
>>  	tests/test-qapi-introspect.h
>
> Worth declaring a good portion of this list in a separate variable, then
> appending that variable to GENERATED FILES as well as...
>
>
>> @@ -609,12 +623,32 @@ tests/test-replication$(EXESUF): tests/test-replication.o $(test-util-obj-y) \
>>  	$(test-block-obj-y)
>>  
>>  tests/test-qapi-types.c tests/test-qapi-types.h \
>> +tests/include/test-qapi-types-sub-module.c \
>> +tests/include/test-qapi-types-sub-module.h \
>> +tests/test-qapi-types-sub-sub-module.c \
>> +tests/test-qapi-types-sub-sub-module.h \
>>  tests/test-qapi-visit.c tests/test-qapi-visit.h \
>> +tests/include/test-qapi-visit-sub-module.c \
>> +tests/include/test-qapi-visit-sub-module.h \
>> +tests/test-qapi-visit-sub-sub-module.c \
>> +tests/test-qapi-visit-sub-sub-module.h \
>>  tests/test-qapi-commands.h tests/test-qapi-commands.c \
>> +tests/include/test-qapi-commands-sub-module.h \
>> +tests/include/test-qapi-commands-sub-module.c \
>> +tests/test-qapi-commands-sub-sub-module.h \
>> +tests/test-qapi-commands-sub-sub-module.c \
>>  tests/test-qapi-events.c tests/test-qapi-events.h \
>> +tests/include/test-qapi-events-sub-module.c \
>> +tests/include/test-qapi-events-sub-module.h \
>> +tests/test-qapi-events-sub-sub-module.c \
>> +tests/test-qapi-events-sub-sub-module.h \
>>  tests/test-qapi-introspect.c tests/test-qapi-introspect.h: \
>
> ...reusing it here for less duplication?

We compute stuff from ${QAPI_MODULES} for qapi/qapi-schema.json.
Reduces verbosity somewhat, for a price in complexity.  Verbosity
increases with number of modules, complexity stays flat.
qapi/qapi-schema.json has 20 sub-modules,
tests/qapi-schema/qapi-schema-test.json has two.  I'd prefer to stick
with stupid for now.

> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!