Re: [PATCH v1 1/1] Add support for meson building

[Ariel D'Alessandro <ariel@vanguardiasur.com.ar>]

Hi Kieran,

Sorry for the late reply.

On 6/18/20 10:57 AM, Kieran Bingham wrote:
> Hi Ariel,
> 
> Wow there's a lot of work there! That must have taken quite some effort
> between you and Ezequiel!
> 
> I've looked through, and about the only thing that stands out to me is
> the way you're joining strings.
> 
> Meson provides a join_paths() function specifically for that.
> Now we're "probably" not going to build this library on anything other
> than linux, but I think the function still has merit over the arbitrary
> strings which I mis-interpreted for 'divide' at first glance :S
> 
> Other than that, I expect we will have to run both build systems in
> parallel for some time to allow packaging and other builders to adapt.
> That might mean it's a bit more difficult to make sure both build
> systems are updated when adding new files or changing the build in anyway.
> 
> However, that said - I'm most certainly in favour of this change and
> would love to see more meson support, so theres an:
> 
> Acked-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> for the principle of this either way, but I don't maintain this library
> anyway ;-)
> 
> With join_paths() used instead of (I guess it's just plain string
> concatenation?):

So, this has already been discussed in the thread... skipping.

> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> But that's a whole lot of text, so I'm sure I've probably missed
> something. Lets see what more eyes find...
> 
> --
> Kieran
> 
> 
> 
> On 18/06/2020 14:33, Ariel D'Alessandro wrote:
>> Supports building libraries and tools found in contrib/, lib/ and
>> utils/ directories, along with the implemented gettext translations.
>>
>> Co-developed-by: Ezequiel Garcia <ezequiel@collabora.com>
>> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
>> Signed-off-by: Ariel D'Alessandro <ariel@vanguardiasur.com.ar>
>> ---
[snip]
>> diff --git a/contrib/gconv/meson.build b/contrib/gconv/meson.build
>> new file mode 100644
>> index 00000000..88abc46f
>> --- /dev/null
>> +++ b/contrib/gconv/meson.build
>> @@ -0,0 +1,42 @@
>> +arib_std_b24_sources = files(
>> +    'arib-std-b24.c',
>> +    'jis0201.h',
>> +    'jis0208.h',
>> +    'jisx0213.h',
>> +)
>> +
>> +arib_std_b24_deps = [
>> +    dep_jis,
>> +    dep_jisx0213,
>> +]
>> +
>> +arib_std_b24 = shared_module('ARIB-STD-B24',
>> +                             arib_std_b24_sources,
>> +                             name_prefix : '',
>> +                             dependencies : arib_std_b24_deps,
>> +                             install : true,
>> +                             install_dir : get_option('libdir') / 'gconv',
> 
> Looks like this should also be join_paths() (noticed below first, and
> skipping back to look for other occurences).
> 
> 
> 
>> +                             include_directories : v4l2_utils_incdir)
>> +
>> +dep_arib_std_b24 = declare_dependency(link_with : arib_std_b24)
>> +
>> +en300_468_tab00_sources = files(
>> +    'en300-468-tab00.c',
>> +)
>> +
>> +en300_468_tab00_deps = [
>> +    dep_jis,
>> +    dep_jisx0213,
>> +]
> 
> you could do
> 
> gconv_install = join_paths(get_option('libdir'), 'gconv')
> 
>> +
>> +en300_468_tab00 = shared_module('EN300-468-TAB00',
>> +                                en300_468_tab00_sources,
>> +                                name_prefix : '',
>> +                                dependencies : en300_468_tab00_deps,
>> +                                install : true,
>> +                                install_dir : get_option('libdir') / 'gconv',
> 
> 				   install_dir : gconv_install,
> 
> 
> 
>> +                                include_directories : v4l2_utils_incdir)
>> +
>> +dep_en300_468_tab00 = declare_dependency(link_with : en300_468_tab00)
>> +
>> +install_data('gconv-modules', install_dir : get_option('libdir') / 'gconv')
> 
> Then you can reuse the gconv_install variable here.
> 
> Same wherever approrpiate in other build files too.

Sounds good, I'll do that and reuse those variables whenever possible.