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

[Jacopo Mondi]

Hi Kieran,

On Tue, Dec 01, 2020 at 04:33:52PM +0000, Kieran Bingham wrote:
> 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"?

I think a special case is more confusing than helpful.

>
>
> 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.
>

That would be nice!

Thanks
   j

> --
> 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