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

[Frediano Ziglio]

> 
> 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.

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?

> >>>                                                 link_with :
> >>>                                                 spice_common_server_lib,
> >>>                                                 dependencies :
> >>>                                                 spice_common_dep)
> >>>  endif
> > 

Frediano