[Spice-devel] [PATCH spice-common v2 2/2] meson: Do not build generated files twice

[Eduardo Lima (Etrunko)]

On 4/4/19 11:50 AM, Frediano Ziglio wrote:
>>
>> On 4/4/19 5:19 AM, Frediano Ziglio wrote:
>>>>
>>>> Hi,
>>>>
>>>> On Wed, Apr 03, 2019 at 05:20:36PM +0100, Frediano Ziglio wrote:
>>>>> spice-gtk and spice-server will use spice_common_client_dep
>>>>> and spice_common_server_dep as dependencies.
>>>>> However they will depend on both spice-common client/server
>>>>> libraries and their sources causing the sources to be compiled
>>>>> multiple times and causes linker errors on spice-gtk.
>>>>> The issue can be observed doing a "find -name \*generated\*.o"
>>>>> in Meson build directory.
>>>>>
>>>>> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
>>>>> ---
>>>>>  common/meson.build | 10 ++++++++--
>>>>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/common/meson.build b/common/meson.build
>>>>> index 2b7bef0..9eace62 100644
>>>>> --- a/common/meson.build
>>>>> +++ b/common/meson.build
>>>>> @@ -97,11 +97,13 @@ if spice_common_generate_client_code
>>>>>      'ssl_verify.h',
>>>>>    ]
>>>>>  
>>>>> +  target_headers = []
>>>>>    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 +
>>>>>                             ['client_marshallers.h'])
>>>>> +    target_headers += target[1]
>>>>>      spice_common_client_sources += target
>>>>>    endforeach
>>>>>  
>>>>> @@ -109,7 +111,7 @@ if spice_common_generate_client_code
>>>>>                                             install : false,
>>>>>                                             dependencies :
>>>>>                                             spice_common_dep)
>>>>>  
>>>>> -  spice_common_client_dep = declare_dependency(sources : target,
>>>>> +  spice_common_client_dep = declare_dependency(sources : target_headers,
>>>>>                                                 link_with :
>>>>>                                                 spice_common_client_lib,
>>>>>                                                 dependencies :
>>>>>                                                 spice_common_dep)
>>>>>  endif
>>>>> @@ -153,10 +155,14 @@ if spice_common_generate_server_code
>>>>>  
>>>>>    spice_common_server_sources = []
>>>>>  
>>>>> +  target_headers = []
>>>>>    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 +
>>>>>                             ['messages.h'])
>>>>> +    if t[2].length() > 1
>>>>> +      target_headers += target[1]
>>>>> +    endif
>>>>
>>>> Why this check?
>>>>
>>>
>>> One of the targets have no header, only a C module on output.
>>> It may seems odd to check t[2] (which is an array) and get
>>> target but each output produce a different entry in target.
>>> I'm not sure if having only one output target will be an
>>> array or not so the check on t[2].
>>>
>>
>> Because it is a list, we are sure that in the last iteration, 'target'
>> holds an item at index 2, so no need to check, just add it to declare
>> dependency directly.
>>
>>>>>      spice_common_server_sources += target
>>>>>    endforeach
>>>>>  
>>>>> @@ -164,7 +170,7 @@ if spice_common_generate_server_code
>>>>>                                             install : false,
>>>>>                                             dependencies :
>>>>>                                             spice_common_dep)
>>>>>  
>>>>> -  spice_common_server_dep = declare_dependency(sources : target,
>>>>> +  spice_common_server_dep = declare_dependency(sources : target_headers,
>>
>> Here --->                                                     target[2],
>>
> 
> Well, yes, unless you add another target, in that case you need to change
> everything.
> 
> Looks weird that you are doing the same things more or less for each
> target but one has a for, another not, another has a file as dependency,
> another a different one. But this a part of the fact that Meson has
> no functions so one way to reuse is copy&paste which blindly would
> exponentially increase the meson.build files. You and up, like here,
> to copy&paste, then simplify each copy ending up with lot of very
> similar lines.
> 

Well, that's the best we can do, because we have a chain of dependencies
of generated files. There is a possibility of using a generator() which
I haven't look at. This might be the case of using it.

http://mesonbuild.com/Reference-manual.html#generator-object

> Not that I have another better solution (using Meson).
> Maybe create a single "targets", using a "for" produce another
> "targets" containing the custom_target(s) and then split the array
> (which with no slicing is a bit ugly) into client and server one?
> 

Okay, I sent a new version of the patches squashed, plus one extra for
cosmetics, lets compare both series ;).


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