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@xxxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxx>
>> Signed-off-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
>> Signed-off-by: Ariel D'Alessandro <ariel@xxxxxxxxxxxxxxxxxxxx>
>> ---
[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.
