[libcamera-devel] [RFC PATCH 5/8] src: ipa: meson: Re-use existing system paths

[Kieran Bingham]

Hi Jacopo,

On 25/11/2020 08:48, Kieran Bingham wrote:
> Hi Jacopo,
> 
> On 24/11/2020 21:28, Jacopo Mondi wrote:
>> Hi Kieran
>>
>> On Mon, Nov 23, 2020 at 04:43:16PM +0000, Kieran Bingham wrote:
>>> Make use of the new system path variables and map the ipa paths upon the
>>> libcamera system paths.
>>>
>>> While we're at it, make use of the new shorter syntax for join_paths().
>>>
>>> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>> ---
>>>  src/ipa/meson.build | 16 +++++++---------
>>>  src/meson.build     |  2 ++
>>>  2 files changed, 9 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/src/ipa/meson.build b/src/ipa/meson.build
>>> index 5a5de267c147..c1dc1ce6fa90 100644
>>> --- a/src/ipa/meson.build
>>> +++ b/src/ipa/meson.build
>>> @@ -1,19 +1,17 @@
>>>  # SPDX-License-Identifier: CC0-1.0
>>>
>>> -ipa_install_dir = join_paths(get_option('libdir'), 'libcamera')
>>> -ipa_data_dir = join_paths(get_option('datadir'), 'libcamera', 'ipa')
>>> -ipa_sysconf_dir = join_paths(get_option('sysconfdir'), 'libcamera', 'ipa')
>>> -
>>>  ipa_includes = [
>>>      libcamera_includes,
>>>  ]
>>>
>>> -config_h.set('IPA_CONFIG_DIR',
>>> -             '"' + join_paths(get_option('prefix'), ipa_sysconf_dir) +
>>> -             ':' + join_paths(get_option('prefix'), ipa_data_dir) + '"')
>>> +ipa_install_dir = libcamera_libdir
>>> +ipa_data_dir = libcamera_datadir / 'ipa'
>>> +ipa_sysconf_dir = libcamera_sysconfdir / 'ipa'
>>> +
>>> +config_h.set('IPA_CONFIG_DIR', '"' + prefix / ipa_sysconf_dir +
>>> +                               ':' + prefix / ipa_data_dir + '"')
>>>
>>> -config_h.set('IPA_MODULE_DIR',
>>> -             '"' + join_paths(get_option('prefix'), ipa_install_dir) + '"')
>>> +config_h.set('IPA_MODULE_DIR', '"' + prefix / ipa_install_dir + '"')
>>
>> Same comment as in the previous patch, but what you have is good too
>>
>> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>>
> 
> Thanks, if the prefix can indeed be moved into the variables I will do
> so - but I'd like to keep all usages the same. So if one needs it split,
> then they'll all be split.
> 
> Fortunately, thanks to your prompting I think the one usage that doesn't
> use prefix is a bug and ... should be using it ;-)

It turns out that, no - it's not a bug.

So we do need to keep the prefix separated. Which is ... unfortunate.

The reason being is that the ipa-sign-install.sh script uses the
variable "MESON_INSTALL_DESTDIR_PREFIX" [0] which itself joins the
DESTDIR and PREFIX variables.

This means that the path we pass into that script from
src/ipa/meson.build needs to generate the module paths from the
ipa_install_dir *without* the prefix.

And because that means 'one' of these system variables can not have the
prefix, I feel like none of them should, to ensure that we are consistent.

Alternatively, we could have an exception and put the prefix in all of
them, and define an 'extra'? unprefixed ipa_install_dir, (which would be
an unprefixed "libcamera_libdir") so that we can use it in the
meson.add_install_script() ?

[0] https://mesonbuild.com/Reference-manual.html#meson-object

What do you think, leave it all separate? or make a special
"libcamera_libdir_noprefix"?


If we keep things as they are for this patch, I'm tempted to add:

# The ipa-sign-install.sh script which uses the ipa_names variable will
# itself prepend MESON_INSTALL_DESTDIR_PREFIX to each ipa module name,
# therefore we must not include the prefix string here.

to just before the construction of the ipa_names array in
src/ipa/meson.build.

--
Kieran

> 
> K
> 
>> Thanks
>>   j
>>
>>>
>>>  subdir('libipa')
>>>
>>> diff --git a/src/meson.build b/src/meson.build
>>> index b1b3514f6ed7..c7430805fd86 100644
>>> --- a/src/meson.build
>>> +++ b/src/meson.build
>>> @@ -2,10 +2,12 @@
>>>
>>>  # Handle system paths
>>>  datadir = get_option('datadir')
>>> +libdir = get_option('libdir')
>>>  prefix = get_option('prefix')
>>>  sysconfdir = get_option('sysconfdir')
>>>
>>>  libcamera_datadir = datadir / 'libcamera'
>>> +libcamera_libdir = libdir / 'libcamera'
>>>  libcamera_sysconfdir = sysconfdir / 'libcamera'
>>>
>>>  config_h.set('LIBCAMERA_SYSCONF_DIR', '"' + prefix / libcamera_sysconfdir + '"')
>>> --
>>> 2.25.1
>>>
>>> _______________________________________________
>>> libcamera-devel mailing list
>>> libcamera-devel at lists.libcamera.org
>>> https://lists.libcamera.org/listinfo/libcamera-devel
> 

-- 
Regards
--
Kieran