| Message ID | 20230501155507.116039-3-kieran.bingham@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hello Kieran, Thanks for your patch. On 5/1/23 17:55, Kieran Bingham wrote: > Move the v4l2-compat.so shared library installation to the libcamera > library directory. This is the same location that IPA modules live and > will allow the 'generically sounding' v4l2-compat.so to be more clearly > identified as a libcamera compatibility layer. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > We could also rename this to libcamera-v4l2-compat.so, and/or we could > also re-consider whether the libcamera-hal.so file should also live > under the libcamera/ libdir by default. > > src/v4l2/meson.build | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/v4l2/meson.build b/src/v4l2/meson.build > index f132103cb503..114fddf7302a 100644 > --- a/src/v4l2/meson.build > +++ b/src/v4l2/meson.build > @@ -31,6 +31,7 @@ v4l2_compat = shared_library('v4l2-compat', > v4l2_compat_sources, > name_prefix : '', > install : true, > + install_dir : libcamera_libdir, > dependencies : [libcamera_private, libdl], > cpp_args : v4l2_compat_cpp_args) > > @@ -38,7 +39,7 @@ v4l2_compat = shared_library('v4l2-compat', > # adaptation layer. > > cdata = configuration_data() > -cdata.set('LIBCAMERA_V4L2_SO', get_option('prefix') / get_option('libdir') / 'v4l2-compat.so') > +cdata.set('LIBCAMERA_V4L2_SO', get_option('prefix') / libcamera_libdir / 'v4l2-compat.so') > As Laurent mentioned in another thread, I think that: +cdata.set('LIBCAMERA_V4L2_SO', get_option('libexecdir') / 'libcamera' / 'v4l2-compat.so') would be better and more consistent with how other projects install shared objects that are not meant to be public. That would also solve the issue I had when trying to enable the v4l2 compatibility layer and ship it in a separate libcamera-v4l2 sub-package. If you post a v2 with that change, feel free to add my: Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Hi Javier and Kieran, On Wed, May 03, 2023 at 02:51:38PM +0200, Javier Martinez Canillas via libcamera-devel wrote: > On 5/1/23 17:55, Kieran Bingham wrote: > > Move the v4l2-compat.so shared library installation to the libcamera > > library directory. This is the same location that IPA modules live and > > will allow the 'generically sounding' v4l2-compat.so to be more clearly > > identified as a libcamera compatibility layer. > > > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > --- > > We could also rename this to libcamera-v4l2-compat.so, and/or we could I think libcamera/v4l2-compat.so is enough from a namespacing point of view. > > also re-consider whether the libcamera-hal.so file should also live > > under the libcamera/ libdir by default. That's a question specific to Android (and Chrome OS), I don't recall what the Android rules are. > > src/v4l2/meson.build | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/src/v4l2/meson.build b/src/v4l2/meson.build > > index f132103cb503..114fddf7302a 100644 > > --- a/src/v4l2/meson.build > > +++ b/src/v4l2/meson.build > > @@ -31,6 +31,7 @@ v4l2_compat = shared_library('v4l2-compat', > > v4l2_compat_sources, > > name_prefix : '', > > install : true, > > + install_dir : libcamera_libdir, > > dependencies : [libcamera_private, libdl], > > cpp_args : v4l2_compat_cpp_args) > > > > @@ -38,7 +39,7 @@ v4l2_compat = shared_library('v4l2-compat', > > # adaptation layer. > > > > cdata = configuration_data() > > -cdata.set('LIBCAMERA_V4L2_SO', get_option('prefix') / get_option('libdir') / 'v4l2-compat.so') > > +cdata.set('LIBCAMERA_V4L2_SO', get_option('prefix') / libcamera_libdir / 'v4l2-compat.so') > > As Laurent mentioned in another thread, I think that: > > +cdata.set('LIBCAMERA_V4L2_SO', get_option('libexecdir') / 'libcamera' / 'v4l2-compat.so') I think you need get_option('prefix') / get_option('libexecdir') / 'libcamera' / 'v4l2-compat.so' > would be better and more consistent with how other projects install > shared objects that are not meant to be public. According to https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04s07.html, "/usr/libexec includes internal binaries that are not intended to be executed directly by users or shell scripts. Applications may use a single subdirectory under /usr/libexec." This being said, valgrind installs LD_PRELOAD-able .so files in libexec, and there are some other .so from different packages there too. I thus think it makes sense to store v4l2-compat.so in libexec. If we go for libexec, the first patch in this series could also be dropped. We could keep it for additional safety, but until we store other files in that directory, it's a bit pointless. > That would also solve the issue I had when trying to enable the v4l2 > compatibility layer and ship it in a separate libcamera-v4l2 sub-package. > > If you post a v2 with that change, feel free to add my: > > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: > Hi Javier and Kieran, > > On Wed, May 03, 2023 at 02:51:38PM +0200, Javier Martinez Canillas via libcamera-devel wrote: >> On 5/1/23 17:55, Kieran Bingham wrote: >> > Move the v4l2-compat.so shared library installation to the libcamera >> > library directory. This is the same location that IPA modules live and >> > will allow the 'generically sounding' v4l2-compat.so to be more clearly >> > identified as a libcamera compatibility layer. >> > >> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> > --- >> > We could also rename this to libcamera-v4l2-compat.so, and/or we could > > I think libcamera/v4l2-compat.so is enough from a namespacing point of > view. > Agreed. >> > also re-consider whether the libcamera-hal.so file should also live >> > under the libcamera/ libdir by default. > > That's a question specific to Android (and Chrome OS), I don't recall > what the Android rules are. > >> > src/v4l2/meson.build | 3 ++- >> > 1 file changed, 2 insertions(+), 1 deletion(-) >> > >> > diff --git a/src/v4l2/meson.build b/src/v4l2/meson.build >> > index f132103cb503..114fddf7302a 100644 >> > --- a/src/v4l2/meson.build >> > +++ b/src/v4l2/meson.build >> > @@ -31,6 +31,7 @@ v4l2_compat = shared_library('v4l2-compat', >> > v4l2_compat_sources, >> > name_prefix : '', >> > install : true, >> > + install_dir : libcamera_libdir, >> > dependencies : [libcamera_private, libdl], >> > cpp_args : v4l2_compat_cpp_args) >> > >> > @@ -38,7 +39,7 @@ v4l2_compat = shared_library('v4l2-compat', >> > # adaptation layer. >> > >> > cdata = configuration_data() >> > -cdata.set('LIBCAMERA_V4L2_SO', get_option('prefix') / get_option('libdir') / 'v4l2-compat.so') >> > +cdata.set('LIBCAMERA_V4L2_SO', get_option('prefix') / libcamera_libdir / 'v4l2-compat.so') >> >> As Laurent mentioned in another thread, I think that: >> >> +cdata.set('LIBCAMERA_V4L2_SO', get_option('libexecdir') / 'libcamera' / 'v4l2-compat.so') > > I think you need > > get_option('prefix') / get_option('libexecdir') / 'libcamera' / 'v4l2-compat.so' > Right, sorry I missed the 'prefix' when typed the above. >> would be better and more consistent with how other projects install >> shared objects that are not meant to be public. > > According to > https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04s07.html, > > "/usr/libexec includes internal binaries that are not intended to be > executed directly by users or shell scripts. Applications may use a > single subdirectory under /usr/libexec." > > This being said, valgrind installs LD_PRELOAD-able .so files in libexec, > and there are some other .so from different packages there too. I thus > think it makes sense to store v4l2-compat.so in libexec. > > If we go for libexec, the first patch in this series could also be > dropped. We could keep it for additional safety, but until we store > other files in that directory, it's a bit pointless. > Agreed.
diff --git a/src/v4l2/meson.build b/src/v4l2/meson.build index f132103cb503..114fddf7302a 100644 --- a/src/v4l2/meson.build +++ b/src/v4l2/meson.build @@ -31,6 +31,7 @@ v4l2_compat = shared_library('v4l2-compat', v4l2_compat_sources, name_prefix : '', install : true, + install_dir : libcamera_libdir, dependencies : [libcamera_private, libdl], cpp_args : v4l2_compat_cpp_args) @@ -38,7 +39,7 @@ v4l2_compat = shared_library('v4l2-compat', # adaptation layer. cdata = configuration_data() -cdata.set('LIBCAMERA_V4L2_SO', get_option('prefix') / get_option('libdir') / 'v4l2-compat.so') +cdata.set('LIBCAMERA_V4L2_SO', get_option('prefix') / libcamera_libdir / 'v4l2-compat.so') configure_file(input : 'libcamerify.in', output : 'libcamerify',
Move the v4l2-compat.so shared library installation to the libcamera library directory. This is the same location that IPA modules live and will allow the 'generically sounding' v4l2-compat.so to be more clearly identified as a libcamera compatibility layer. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- We could also rename this to libcamera-v4l2-compat.so, and/or we could also re-consider whether the libcamera-hal.so file should also live under the libcamera/ libdir by default. src/v4l2/meson.build | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)