[libcamera-devel] [PATCH 3/4] src: meson: Define system paths

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Mon, Dec 07, 2020 at 03:07:36PM +0000, Kieran Bingham wrote:
> Define libcamera specific system paths for use within the library code
> base. These can be used to identify system configuration files and
> shared data.
> 
> Also, make use of the new system path variables and map the ipa paths
> upon the libcamera system path, making use of the new shorter syntax for
> join_paths().
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> ---
> v2:
>  - Add comments explaining why we can't use prefix throughout
>  - Squash src/meson and src/ipa/meson changes together
> 
>  src/ipa/meson.build | 19 ++++++++++---------
>  src/meson.build     | 14 ++++++++++++++
>  2 files changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/src/ipa/meson.build b/src/ipa/meson.build
> index 5a5de267c147..41625ed471bc 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 + '"')
>  subdir('libipa')
>  
> @@ -22,6 +20,9 @@ ipa_sign = files('ipa-sign.sh')
>  ipas = ['raspberrypi', 'rkisp1', 'vimc']
>  ipa_names = []
>  
> +# 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.
>  foreach pipeline : get_option('pipelines')
>      if ipas.contains(pipeline)
>          subdir(pipeline)
> diff --git a/src/meson.build b/src/meson.build
> index 4b75f05878bc..f98fcede203b 100644
> --- a/src/meson.build
> +++ b/src/meson.build
> @@ -1,5 +1,19 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
> +# 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'

We could write this

libcamera_datadir = get_option('datadir') / 'libcamera'
libcamera_libdir = get_option('libdir') / 'libcamera'
libcamera_sysconfdir = get_option('sysconfdir') / 'libcamera'

to avoid the datadir, libdir and sysconfdir variables that are fairly
generic and could result in namespace clashes later.

> +
> +config_h.set('LIBCAMERA_DATA_DIR', '"' + prefix / libcamera_datadir + '"')
> +config_h.set('LIBCAMERA_SYSCONF_DIR', '"' + prefix / libcamera_sysconfdir + '"')

Those two variables are not used in this patch series, I believe they
were introduced to support the configuration file parser. Should they be
left out for now ?

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> +
> +# Module Signing
>  openssl = find_program('openssl', required : true)
>  if openssl.found()
>      ipa_priv_key = custom_target('ipa-priv-key',

-- 
Regards,

Laurent Pinchart