[Spice-devel] [PATCH spice-common] fixup! meson build

Fri May 25 13:48:53 UTC 2018

On 25/05/18 09:35, Frediano Ziglio wrote:
>>
>> On Thu, May 24, 2018 at 02:15:17PM -0300, Eduardo Lima (Etrunko) wrote:
>>> On 24/05/18 12:53, Jonathon Jongsma wrote:
>>>> On Thu, 2018-05-24 at 12:41 -0300, Eduardo Lima (Etrunko) wrote:
>>>>> On 24/05/18 12:31, Jonathon Jongsma wrote:
>>>>>> On Wed, 2018-05-23 at 15:23 -0300, Eduardo Lima (Etrunko) wrote:
>>>>>>> On 23/05/18 11:43, Frediano Ziglio wrote:
>>>>>>>>>
>>>>>>>>> Signed-off-by: Eduardo Lima (Etrunko) <etrunko at redhat.com>
>>>>>>>>> ---
>>>>>>>>>  meson.build       | 14 ++++++++++----
>>>>>>>>>  meson_options.txt |  5 +++++
>>>>>>>>>  2 files changed, 15 insertions(+), 4 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/meson.build b/meson.build
>>>>>>>>> index 9d44604..cd75c51 100644
>>>>>>>>> --- a/meson.build
>>>>>>>>> +++ b/meson.build
>>>>>>>>> @@ -90,11 +90,14 @@ endforeach
>>>>>>>>>  #
>>>>>>>>>  # check for mandatory dependencies
>>>>>>>>>  #
>>>>>>>>> -glib_version_info = '>= 2.46'
>>>>>>>>> -glib_encoded_version = 'GLIB_VERSION_2_46'
>>>>>>>>>  spice_protocol_version = '>=
>>>>>>>>> @0@'.format(get_option('protocol-
>>>>>>>>> version'))
>>>>>>>>>  
>>>>>>>>> -deps = [['spice-protocol', spice_protocol_version],
>>>>>>>>> +glib_version = get_option('glib-version')
>>>>>>>>> +glib_major_minor = glib_version.split('.')
>>>>>>>>> +glib_version_info = '>= @0 at .@1@'.format(glib_major_minor[0],
>>>>>>>>> glib_major_minor[1])
>>>>>>>>> +glib_encoded_version = 'GLIB_VERSION_ at 0@_ at 1@'.format(glib_ma
>>>>>>>>> jor_
>>>>>>>>> minor[0],
>>>>>>>>> glib_major_minor[1])
>>>>>>>>> +
>>>>>>>>> +deps = [['spice-protocol', '>=
>>>>>>>>> @0@'.format(get_option('protocol-
>>>>>>>>> version'))],
>>>>>>>>>          ['glib-2.0', glib_version_info],
>>>>>>>>>          ['gobject-2.0', glib_version_info],
>>>>>>>>>          ['gio-2.0', glib_version_info],
>>>>>>>>> @@ -115,8 +118,11 @@ spice_common_global_cflags +=
>>>>>>>>> spice_common_glib_cflags
>>>>>>>>>  # Non-mandatory/optional dependencies
>>>>>>>>>  #
>>>>>>>>>  deps = [['opus', '>= 0.9.14', 'HAVE_OPUS'],]
>>>>>>>>> -optional_deps = [['celt051', '>= 0.5.1.1', 'HAVE_CELT051'],]
>>>>>>>>>  
>>>>>>>>> +# Check deps which are optional but enabled by default. This
>>>>>>>>> foreach block
>>>>>>>>> only
>>>>>>>>> +# checks the option, and adds the package to the deps list,
>>>>>>>>> while the real
>>>>>>>>> check
>>>>>>>>> +# for the dependency is done in the foeach block below.
>>>>>>>>> +optional_deps = [['celt051', '>= 0.5.1.1', 'HAVE_CELT051'],]
>>>>>>>>>  foreach dep : optional_deps
>>>>>>>>>    if get_option(dep[0])
>>>>>>>>>      deps += [dep]
>>>>>>>>> diff --git a/meson_options.txt b/meson_options.txt
>>>>>>>>> index 8e27cbf..b2a38e7 100644
>>>>>>>>> --- a/meson_options.txt
>>>>>>>>> +++ b/meson_options.txt
>>>>>>>>> @@ -26,6 +26,11 @@ option('manual',
>>>>>>>>>      yield : true,
>>>>>>>>>      description : 'Build SPICE manual (default=true)')
>>>>>>>>>  
>>>>>>>>> +option('glib-version',
>>>>>>>>> +    type : 'string',
>>>>>>>>> +    value : '2.38',
>>>>>>>>> +    description : 'Glib version required (default=2.38)')
>>>>>>>>> +
>>>>>>>>>  option('protocol-version',
>>>>>>>>>      type : 'string',
>>>>>>>>>      value : '0.12.12',
>>>>>>>>
>>>>>>>> Both glib-version and protocol-version, not clear why we don't
>>>>>>>> need this for autoconf but this is needed for Meson.
>>>>>>>> I think there were discussions about removing protocol-version
>>>>>>>> option too.
>>>>>>>
>>>>>>> The reason is that server and client both require different
>>>>>>> versions
>>>>>>> of
>>>>>>> glib, and the check is only done here in spice-common. So we need
>>>>>>> some
>>>>>>> way to tell which version we require. If none specified, fallback
>>>>>>> to
>>>>>>> the
>>>>>>> default value.
>>>>>>
>>>>>> But why can't we just check for glib in the other projects as well?
>>>>>> This option seems a bit odd to me.
>>>>>
>>>>> We can for sure, but that would mean we will be checking for it
>>>>> twice,
>>>>> and it could be a bit messy, for instance, the configure phase will
>>>>> check for a glib version for spice-common that satisfies the
>>>>> requirement
>>>>> but when the check is done again for server or gtk, the version
>>>>> required
>>>>> is higher and it will fail. So, why not fail straight away?
>>>>>
>>>>> Doing the check twice, will also mean that the dependencies will be
>>>>> added twice, and the command line will add up a lot (not sure if
>>>>> meson
>>>>> is smart to not add the same flags twice).
>>>>
>>>> Would be interesting to know.
>>>>
>>>>>
>>>>> Finally, if this is done for glib case, for the sake of consistency,
>>>>> I
>>>>> think we should do the same for all other dependencies that are
>>>>> required
>>>>> for both server and client.
>>>>
>>>> Well, to me it seems like the right thing to do would be to specify the
>>>> dependencies needed within each project.
>>>
>>> The way I see it, this can turn into a maintenance problem in the
>>> future, and for this reason I wanted to keep all common checks in a
>>> single place, instead of replicated all over the projects. We already
>>> have this problem with autotools, just look at the gstreamer check for
>>> instance.
>>

(Replying to both Christophe and Frediano in this email, sorry for the
lazyness.)

>> Imo it's good if each project check by itself for the dependencies it
>> needs, so that they are as self-contained as possible. If some
>> additional dependencies in order to use spice-common headers, then
>> it's good that we can get them directly from the spice-common submodule.
>> If on the contrary it's spice-gtk or spice code which needs some
>> dependency in order to compile, then it makes more sense that it checks
>> itself if this dependency is available, rather than being tied to
>> spice-common.
> 

What do you mean, even if you have the same dependency in both server
and common, we should replicate the code across the projects or make use
of the dependency already defined in common?

If it is the former, then oh boy, I will have to go back and review each
dependency to make sure where it belongs.

> I agree subprojects should be self-contained as possible.
> Although spice-common is a bit particular in this case as targets
> are limited to spice-server and spice-gtk to avoid some code duplication
> between the two.

This is the way I see it, the reason of spice-common existing in first
place is to share code between both server and gtk. So they are all
somewhat interdependent and not really much self-contained.

> One question that I have is: does this code is a Meson limitation
> or something that has to be done differently?

The only other way is to replicate the checks in all projects, as I
explained in a previous mail.

> On autoconf you can reuse m4 macros, for instance we reuse
> SPICE_CHECK_SMARTCARD from spice-common in spice-gtk and spice-server.
> We don't reuse SPICE_CHECK_GLIB2 but we check manually on spice-server
> and spice-gtk, maybe with an additional version parameter this function
> could be reused too.
> So, are these Meson build option a way to reuse the check in spice-common
> build files due to some Meson limitation or another way of doing things?

Exactly, it is not possible to reuse the checks, because there is no
support for defining functions in the language. What we can do is to
have the parent project accessing the variables/definitions in the
subproject.

> autoconf allows to pass any --enable-/--with- options so they can be
> reused by subprojects. Is something like this implemented by Meson?
> Seems not possible, options need to be manually copied to the parent
> projects in order to work. Maybe there is another way to achieve this with
> Meson.
> 
> Had a look at http://mesonbuild.com/Subprojects.html but didn't find
> anything.
> 

About options, you have to define the same in both subproject and
superproject. The subproject may inherit the value automatically if the
'yield' keyword is used.

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