[Spice-devel] [PATCH spice-common] Meson: make use of features in 0.47

[Eduardo Lima (Etrunko)]

On 25/07/18 13:08, Frediano Ziglio wrote:
>>
>> Spice-server and spice-gtk already require 0.47, and with this release,
>> there were some interesting additions we can make use:
>>
>>  - Options with 'feature' type:
>>
>>    This built-in type is used instead of the tri-state string combo
>>    (auto/true/false), simplifying the dependency checks.
>>
>>    http://mesonbuild.com/Build-options.html#features
>>
>>  - Dictionary:
>>
>>    Easier to iterate and improves readability of the code by replacing
>>    the use of nested lists.
>>
>>    http://mesonbuild.com/Reference-manual.html#dictionary-object
>>
>> Signed-off-by: Eduardo Lima (Etrunko) <etrunko at redhat.com>
>> ---
>>  meson.build       | 37 ++++++++++++++++---------------------
>>  meson_options.txt |  7 ++-----
>>  2 files changed, 18 insertions(+), 26 deletions(-)
>>
>> diff --git a/meson.build b/meson.build
>> index 226d9f4..6fa67dc 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -2,7 +2,7 @@
>>  # project definition
>>  #
>>  project('spice-common', 'c',
>> -         meson_version : '>= 0.45.0',
>> +         meson_version : '>= 0.47.0',
>>           license : 'LGPLv2.1')
>>  
>>  if not meson.is_subproject()
>> @@ -98,32 +98,27 @@ endif
>>  glib_version = '2.38'
>>  glib_version_info = '>= @0@'.format(glib_version)
>>  
>> -deps = [['spice-protocol', '>= 0.12.12'],
>> -        ['glib-2.0', glib_version_info],
>> -        ['gio-2.0', glib_version_info],
>> -        ['gthread-2.0', glib_version_info],
>> -        ['pixman-1', '>= 0.17.7'],
>> -        ['openssl', '>= 1.0.0']]
>> +deps = {'spice-protocol' : '>= 0.12.12',
>> +        'glib-2.0'       : glib_version_info,
>> +        'gio-2.0'        : glib_version_info,
>> +        'gthread-2.0'    : glib_version_info,
>> +        'pixman-1'       : '>= 0.17.7',
>> +        'openssl'        : '>= 1.0.0'}
>>  
>> -foreach dep : deps
>> -  spice_common_deps += dependency(dep[0], version : dep[1])
>> +foreach dep, version : deps
>> +  spice_common_deps += dependency(dep, version : version)
>>  endforeach
>>  
> 
> surely more readable, note that dictionary does not keep
> the order of the items while arrays does, but is not a problem
> with current code.
> 
>>  #
>>  # Non-mandatory/optional dependencies
>>  #
>> -optional_deps = [
>> -                    ['celt051', '>= 0.5.1.1'],
>> -                    ['opus', '>= 0.9.14'],
>> -                ]
>> -foreach dep : optional_deps
>> -  option_value = get_option(dep[0])
>> -  if option_value != 'false'
>> -    d = dependency(dep[0], required: (option_value == 'true'), version :
>> dep[1])
>> -    if d.found()
>> -      spice_common_deps += d
>> -
>> spice_common_config_data.set('HAVE_ at 0@'.format(dep[0].underscorify().to_upper()),
>> '1')
>> -    endif
>> +optional_deps = {'celt051' : '>= 0.5.1.1',
>> +                 'opus'    : '>= 0.9.14'}
>> +foreach dep, version : optional_deps
>> +  d = dependency(dep, required : get_option(dep), version : version)
>> +  if d.found()
>> +    spice_common_deps += d
>> +
>> spice_common_config_data.set('HAVE_ at 0@'.format(dep.underscorify().to_upper()),
>> '1')
>>    endif
>>  endforeach
>>  
>> diff --git a/meson_options.txt b/meson_options.txt
>> index b88f209..1b80257 100644
>> --- a/meson_options.txt
>> +++ b/meson_options.txt
>> @@ -11,15 +11,12 @@ option('extra-checks',
>>      description : 'Enable extra checks on code')
>>  
>>  option('celt051',
>> -    type : 'combo',
>> -    choices : ['true', 'false', 'auto'],
>> -    value : 'auto',
>> +    type : 'feature',
>>      yield : true,
>>      description: 'Enable celt051 audio codec')
>>  
>>  option('opus',
>> -    type : 'combo',
>> -    choices : ['true', 'false', 'auto'],
>> +    type : 'feature',
>>      yield : true,
>>      description: 'Enable Opus audio codec')
>>  
> 
> I would say yes.
> 
> What worry me is that this Meson looks like a moving target.
> The fact that they don't want to add function requires them to
> to write new and new modules/functions and have new releases.
> The main concern for us is that is hard to predict when we could
> remove autoconf stuff having to continue to maintain 2 build systems.
> Will be hard to have Meson 0.47 in next RHEL7 which will mean we
> need at least to wait other 6 months/1 year.
> 

I agree that changing requirements for every new release is far from
ideal, but we are in early stages and I do not see any more strong
reasons for requiring a 0.48 for example (whenever that version is out).

IMO a good time for that would be when they finally fix the equivalent
of 'make dist' to include the generated files:
https://github.com/mesonbuild/meson/issues/2166.

That said, I think I could have split this patch in two, one for the
feature option and the other one for replacing the lists with
dictionaries where appropriate.

> Maybe for older distros we could package Meson in a tarball and
> use it inside higher packager (like spec file) to use a newer
> meson without the needs to have it installed (I looked and there
> are mainly python files so seems doable, just requires a recent
> python3 installed in the system).
> 

Not sure it is worth the hassle, but it is easily doable, it is just a
matter of setting the PYTHONPATH environment variable and you can use
meson from a local directory.

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