[Spice-devel] [PATCH spice-common v2 1/2] meson: Fix dependency of generated files

[Eduardo Lima (Etrunko)]

On 4/4/19 5:08 AM, Victor Toso wrote:
> Hi,
> 
> On Wed, Apr 03, 2019 at 05:20:35PM +0100, Frediano Ziglio wrote:
>> All generated file depends on generated_messages.h which is
>> generated too.
>> So add an explicit dependency from all generated file
>> (except generated_messages.h generator) to generated_messages.h
>> generator.
>> This fixes compiling SPICE server where generated_messages.h
>> was not generated at all.
>> Add dependency to the include header to make Meson generate the
>> proper include flag.
>>
>> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> 
> Acked-by: Victor Toso <victortoso at redhat.com>
> 
> You might want to wait a little while to see if Eduardo has any
> comments but tested both patches and works fine.
> 

I do have some comments, which I already dicussed with Frediano in
private. Sending them here as it is part of the review.

>> ---
>>  common/meson.build | 49 ++++++++++++++++++++++++++++++----------------
>>  1 file changed, 32 insertions(+), 17 deletions(-)
>>
>> diff --git a/common/meson.build b/common/meson.build
>> index 2d76d2b..2b7bef0 100644
>> --- a/common/meson.build
>> +++ b/common/meson.build
>> @@ -59,25 +59,34 @@ spice_common_dep = declare_dependency(link_with : spice_common_lib,
>>                                        include_directories : spice_common_include,
>>                                        dependencies : spice_common_deps)
>>  
>> +targets = [
>> +  ['client_demarshallers', spice_proto,
>> +    ['generated_client_demarshallers.c', 'generated_messages.h'],
>> +    ['--generate-demarshallers',
>> +      '--client',
>> +      '--include', 'common/messages.h',
>> +      '--generated-declaration-file', '@OUTPUT1@',
>> +      '@INPUT@', '@OUTPUT0@'
>> +    ]
>> +  ]
>> +]
>> +
>> +foreach t : targets
>> +  cmd = [python, spice_codegen] + t[3]
>> +  common_generated = custom_target(t[0], input : t[1], output : t[2], install : false, command : cmd,
>> +                                   depend_files : spice_codegen_files + ['messages.h'])
>> +endforeach
>> +

So now that there is only one item in the 'targets' list, the foreach
loop is not needed anymore.

>>  #
>>  # libspice-common-client
>>  #
>>  if spice_common_generate_client_code
>>    targets = [
>> -    ['client_demarshallers', spice_proto,
>> -      ['generated_client_demarshallers.c', 'generated_messages.h'],
>> -      ['--generate-demarshallers',
>> -        '--client',
>> -        '--include', 'common/messages.h',
>> -        '--generated-declaration-file', '@OUTPUT1@',
>> -        '@INPUT@', '@OUTPUT0@'
>> -      ]
>> -    ],
>> -    ['client_marshallers', spice_proto,
>> +    ['client_marshallers', [ spice_proto, common_generated ],
>>        ['generated_client_marshallers.c', 'generated_client_marshallers.h'],
>>        ['--generate-marshallers', '--generate-header',
>> -        '-P', '--client', '--include', 'client_marshallers.h',
>> -        '@INPUT@', '@OUTPUT0@'
>> +        '-P', '--client', '--include', 'common/client_marshallers.h',
>> +        '@INPUT0@', '@OUTPUT0@'
>>        ]
>>      ]
>>    ]
>> @@ -88,9 +97,11 @@ if spice_common_generate_client_code
>>      'ssl_verify.h',
>>    ]
>>  
>> +  spice_common_client_sources += common_generated
>>    foreach t : targets
>>      cmd = [python, spice_codegen] + t[3]
>> -    target = custom_target(t[0], input : t[1], output : t[2], install : false, command : cmd, depend_files : spice_codegen_files)
>> +    target = custom_target(t[0], input : t[1], output : t[2], install : false, command : cmd,
>> +                           depend_files : spice_codegen_files + ['client_marshallers.h'])
>>      spice_common_client_sources += target
>>    endforeach
>>  

Same as above.

>> @@ -128,11 +139,14 @@ if spice_common_generate_server_code
>>    ]
>>  
>>    targets = [
>> -    ['server_demarshallers', spice_proto, 'generated_server_demarshallers.c', ['--generate-demarshallers', '--server', '--include', 'common/messages.h', '@INPUT@', '@OUTPUT@']],
>> -    ['server_marshallers', spice_proto,
>> +    ['server_demarshallers', [ spice_proto, common_generated ],
>> +      ['generated_server_demarshallers.c'],
>> +      ['--generate-demarshallers', '--server', '--include', 'common/messages.h', '@INPUT0@', '@OUTPUT0@']
>> +    ],
>> +    ['server_marshallers', [ spice_proto, common_generated ],
>>        ['generated_server_marshallers.c', 'generated_server_marshallers.h'],
>>        ['--generate-marshallers', '--generate-header',
>> -        '--server'] + structs_args + ['--include', 'common/messages.h', '@INPUT@', '@OUTPUT0@'
>> +        '--server'] + structs_args + ['--include', 'common/messages.h', '@INPUT0@', '@OUTPUT0@'
>>        ]
>>      ],
>>    ]
>> @@ -141,7 +155,8 @@ if spice_common_generate_server_code
>>  
>>    foreach t : targets
>>      cmd = [python, spice_codegen] + t[3]
>> -    target = custom_target(t[0], input : t[1], output : t[2], install : false, command : cmd, depend_files : spice_codegen_files)
>> +    target = custom_target(t[0], input : t[1], output : t[2], install : false, command : cmd,
>> +                           depend_files : spice_codegen_files + ['messages.h'])
>>      spice_common_server_sources += target
>>    endforeach
>>  

Some minor adjustments here for sake of readability. I am sending a
fixup patch with the changes I did.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etrunko at redhat.com