| Message ID | 20201123164319.152742-6-kieran.bingham@ideasonboard.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series |
|
| Related | show |
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@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@jmondi.org> 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@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
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@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@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 ;-) 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@lists.libcamera.org >> https://lists.libcamera.org/listinfo/libcamera-devel
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@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@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@lists.libcamera.org >>> https://lists.libcamera.org/listinfo/libcamera-devel >
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@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@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@lists.libcamera.org > >>> https://lists.libcamera.org/listinfo/libcamera-devel > > > > -- > Regards > -- > Kieran
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 + '"') 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 + '"')
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@ideasonboard.com> --- src/ipa/meson.build | 16 +++++++--------- src/meson.build | 2 ++ 2 files changed, 9 insertions(+), 9 deletions(-)