| Message ID | 20230503100140.1570024-1-kieran.bingham@ideasonboard.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series |
|
| Related | show |
Hello Kieran, On 5/3/23 12:01, Kieran Bingham wrote: > There may be parallel installations of libcamera with separate > instances of the v4l2-adaptation layer shared object. > > This shared library can be linked against a specific libcamera > soversion so provide a soversioned V4L2 compatibility layer. > > Suggested-by: Javier Martinez Canillas <javierm@redhat.com> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/v4l2/meson.build | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/v4l2/meson.build b/src/v4l2/meson.build > index f132103cb503..48f201c5c3a0 100644 > --- a/src/v4l2/meson.build > +++ b/src/v4l2/meson.build > @@ -30,6 +30,7 @@ v4l2_compat_cpp_args = [ > v4l2_compat = shared_library('v4l2-compat', > v4l2_compat_sources, > name_prefix : '', > + soversion : libcamera_soversion, > install : true, > dependencies : [libcamera_private, libdl], > cpp_args : v4l2_compat_cpp_args) Thanks a lot for doing this. I've just tested and it works correctly: Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Hi Kieran, Thank you for the patch. On Wed, May 03, 2023 at 11:01:40AM +0100, Kieran Bingham via libcamera-devel wrote: > There may be parallel installations of libcamera with separate > instances of the v4l2-adaptation layer shared object. For libcamera, absolutely, but for v4l2-compat.so, the use cases seem less clear to me. As discussed on IRC, versioning a library's SONAME is meant for two purposes as far as I understand: ABI stability for applications linking to the library, and installing multiple versions of the library side by side. The first purpose doesn't apply to v4l2-compat.so as nothing will link to it. The second purpose could be applicable, but it seems that the use cases would be a bit far-fetched. To support this properly, you would want to also support installing multiple versions of libcamerify side by side. This discussion and patch stem from a Fedora packaging rule that forbids unversioned shared object libraries in $libdir/ for non -devel packages. Maybe the patch you sent to move v4l2-compat.so to $libdir/libcamera could be enough to satisfy the packaging rules, without needing to version v4l2-compat.so ? Btw, Javier mentioned on IRC it should actually go to $libexecdir/libcamera, but let's discuss that in the review of your other patch. > This shared library can be linked against a specific libcamera > soversion so provide a soversioned V4L2 compatibility layer. > > Suggested-by: Javier Martinez Canillas <javierm@redhat.com> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/v4l2/meson.build | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/v4l2/meson.build b/src/v4l2/meson.build > index f132103cb503..48f201c5c3a0 100644 > --- a/src/v4l2/meson.build > +++ b/src/v4l2/meson.build > @@ -30,6 +30,7 @@ v4l2_compat_cpp_args = [ > v4l2_compat = shared_library('v4l2-compat', > v4l2_compat_sources, > name_prefix : '', > + soversion : libcamera_soversion, > install : true, > dependencies : [libcamera_private, libdl], > cpp_args : v4l2_compat_cpp_args)
Quoting Laurent Pinchart (2023-05-03 13:37:37) > Hi Kieran, > > Thank you for the patch. > > On Wed, May 03, 2023 at 11:01:40AM +0100, Kieran Bingham via libcamera-devel wrote: > > There may be parallel installations of libcamera with separate > > instances of the v4l2-adaptation layer shared object. > > For libcamera, absolutely, but for v4l2-compat.so, the use cases seem > less clear to me. > Because LD_PRELOAD='/usr/lib/x86_64-linux-gnu/v4l2-compat.so' firefox Will load the 'latest' ? most recent? version of libcamera, while: LD_PRELOAD='/usr/lib/x86_64-linux-gnu/v4l2-compat.so.0.0.4' firefox Will explicitly run firefox with libcamera-0.0.4, and LD_PRELOAD='/usr/lib/x86_64-linux-gnu/v4l2-compat.so.0.0.5' firefox will explicitly use libcamera 0.0.5. If a system has both 0.0.4 and 0.0.5 installed, I could certainly see reason to be able to test/validate against both. (And of course the version numbers could be further apart than a single release). Specifically because we update the soname of libcamera (on every release presently), it means each v4l2-compat.so is *only* compatible with the matching libcamera. > As discussed on IRC, versioning a library's SONAME is meant for two > purposes as far as I understand: ABI stability for applications linking > to the library, and installing multiple versions of the library side by > side. The first purpose doesn't apply to v4l2-compat.so as nothing will > link to it. > > The second purpose could be applicable, but it seems that the use cases > would be a bit far-fetched. To support this properly, you would want to > also support installing multiple versions of libcamerify side by side. 'libcamerify' would take the latest (Equivalent to LD_PRELOAD='/usr/lib/x86_64-linux-gnu/v4l2-compat.so') and if someone wants to use a specific version they can LD_PRELOAD it themselves. At that point it would be more likely they know what they're doing anyway. > This discussion and patch stem from a Fedora packaging rule that forbids > unversioned shared object libraries in $libdir/ for non -devel packages. > Maybe the patch you sent to move v4l2-compat.so to $libdir/libcamera > could be enough to satisfy the packaging rules, without needing to > version v4l2-compat.so ? Btw, Javier mentioned on IRC it should actually > go to $libexecdir/libcamera, but let's discuss that in the review of > your other patch. It sounds like you're on the Nack side. Do you actively object to this because you see harm in versioning the sofile? Even if we move it to libexec, or lib/libcamera/ or anywhere else, I can't see why it can not be versioned the same as libcamera. -- Kieran > > This shared library can be linked against a specific libcamera > > soversion so provide a soversioned V4L2 compatibility layer. > > > > Suggested-by: Javier Martinez Canillas <javierm@redhat.com> > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > --- > > src/v4l2/meson.build | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/src/v4l2/meson.build b/src/v4l2/meson.build > > index f132103cb503..48f201c5c3a0 100644 > > --- a/src/v4l2/meson.build > > +++ b/src/v4l2/meson.build > > @@ -30,6 +30,7 @@ v4l2_compat_cpp_args = [ > > v4l2_compat = shared_library('v4l2-compat', > > v4l2_compat_sources, > > name_prefix : '', > > + soversion : libcamera_soversion, > > install : true, > > dependencies : [libcamera_private, libdl], > > cpp_args : v4l2_compat_cpp_args) > > -- > Regards, > > Laurent Pinchart
Hi Kieran, On Thu, May 04, 2023 at 12:10:49AM +0100, Kieran Bingham wrote: > Quoting Laurent Pinchart (2023-05-03 13:37:37) > > On Wed, May 03, 2023 at 11:01:40AM +0100, Kieran Bingham via libcamera-devel wrote: > > > There may be parallel installations of libcamera with separate > > > instances of the v4l2-adaptation layer shared object. > > > > For libcamera, absolutely, but for v4l2-compat.so, the use cases seem > > less clear to me. > > Because > > LD_PRELOAD='/usr/lib/x86_64-linux-gnu/v4l2-compat.so' firefox > > Will load the 'latest' ? most recent? version of libcamera, while: > > LD_PRELOAD='/usr/lib/x86_64-linux-gnu/v4l2-compat.so.0.0.4' firefox > > Will explicitly run firefox with libcamera-0.0.4, and > > LD_PRELOAD='/usr/lib/x86_64-linux-gnu/v4l2-compat.so.0.0.5' firefox > > will explicitly use libcamera 0.0.5. > > If a system has both 0.0.4 and 0.0.5 installed, I could certainly see > reason to be able to test/validate against both. (And of course the > version numbers could be further apart than a single release). I can imagine that in theory, but in practice I'd be surprised if it mattered for v4l2-compat. > Specifically because we update the soname of libcamera (on every release > presently), it means each v4l2-compat.so is *only* compatible with the > matching libcamera. This brings us to the next question, how do you expect v4l2-compat to be packaged ? If you want multiple versions to be able to co-exist side-by-side, the file certainly need to be versioned, as you can't have multiple packages installing the same file. There is however a single libcamerify script, so that can't be part of the same package as v4l2-compat, otherwise you wouldn't be able to install multiple versions of v4l2-compat. If libcamerify is part of the same package as v4l2-compat.so then versioning the later has no value. The same holds true for cam and qcam by the way, which is why they can't be part of a libcamera package if we want to support installing multiple versions of the libraries. > > As discussed on IRC, versioning a library's SONAME is meant for two > > purposes as far as I understand: ABI stability for applications linking > > to the library, and installing multiple versions of the library side by > > side. The first purpose doesn't apply to v4l2-compat.so as nothing will > > link to it. > > > > The second purpose could be applicable, but it seems that the use cases > > would be a bit far-fetched. To support this properly, you would want to > > also support installing multiple versions of libcamerify side by side. > > 'libcamerify' would take the latest (Equivalent to > LD_PRELOAD='/usr/lib/x86_64-linux-gnu/v4l2-compat.so') and if someone > wants to use a specific version they can LD_PRELOAD it themselves. At > that point it would be more likely they know what they're doing anyway. Using /usr/lib/x86_64-linux-gnu/v4l2-compat.so means a package would need to provide this, and as explained above, that has to be a separate package from libcamera. It could possibly be the same package that contains libcamerify, but in that case, libcamerify could also target a specific version of v4l2-compat.so without requiring a symlink. > > This discussion and patch stem from a Fedora packaging rule that forbids > > unversioned shared object libraries in $libdir/ for non -devel packages. > > Maybe the patch you sent to move v4l2-compat.so to $libdir/libcamera > > could be enough to satisfy the packaging rules, without needing to > > version v4l2-compat.so ? Btw, Javier mentioned on IRC it should actually > > go to $libexecdir/libcamera, but let's discuss that in the review of > > your other patch. > > It sounds like you're on the Nack side. Do you actively object to this > because you see harm in versioning the sofile? > > Even if we move it to libexec, or lib/libcamera/ or anywhere else, I > can't see why it can not be versioned the same as libcamera. I'm not necessarily against it. All I want is to consider the big picture of how all this will be packaged, to make sure we implement something useful for distributions. > > > This shared library can be linked against a specific libcamera > > > soversion so provide a soversioned V4L2 compatibility layer. > > > > > > Suggested-by: Javier Martinez Canillas <javierm@redhat.com> > > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > --- > > > src/v4l2/meson.build | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/src/v4l2/meson.build b/src/v4l2/meson.build > > > index f132103cb503..48f201c5c3a0 100644 > > > --- a/src/v4l2/meson.build > > > +++ b/src/v4l2/meson.build > > > @@ -30,6 +30,7 @@ v4l2_compat_cpp_args = [ > > > v4l2_compat = shared_library('v4l2-compat', > > > v4l2_compat_sources, > > > name_prefix : '', > > > + soversion : libcamera_soversion, > > > install : true, > > > dependencies : [libcamera_private, libdl], > > > cpp_args : v4l2_compat_cpp_args)
Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: > On Thu, May 04, 2023 at 12:10:49AM +0100, Kieran Bingham wrote: [...] >> > This discussion and patch stem from a Fedora packaging rule that forbids >> > unversioned shared object libraries in $libdir/ for non -devel packages. >> > Maybe the patch you sent to move v4l2-compat.so to $libdir/libcamera >> > could be enough to satisfy the packaging rules, without needing to >> > version v4l2-compat.so ? Btw, Javier mentioned on IRC it should actually >> > go to $libexecdir/libcamera, but let's discuss that in the review of >> > your other patch. >> >> It sounds like you're on the Nack side. Do you actively object to this >> because you see harm in versioning the sofile? >> >> Even if we move it to libexec, or lib/libcamera/ or anywhere else, I >> can't see why it can not be versioned the same as libcamera. > > I'm not necessarily against it. All I want is to consider the big > picture of how all this will be packaged, to make sure we implement > something useful for distributions. > As another data point, none of the shared libraries I currently have installed in /usr/libexec/ have a versioned SONAME. If v4l2-compat.so is moved there, then IMO we should just drop this patch since the lib will only be used by libcamerify. And as Laurent mentioned, that isn't parallel installable anyways.
diff --git a/src/v4l2/meson.build b/src/v4l2/meson.build index f132103cb503..48f201c5c3a0 100644 --- a/src/v4l2/meson.build +++ b/src/v4l2/meson.build @@ -30,6 +30,7 @@ v4l2_compat_cpp_args = [ v4l2_compat = shared_library('v4l2-compat', v4l2_compat_sources, name_prefix : '', + soversion : libcamera_soversion, install : true, dependencies : [libcamera_private, libdl], cpp_args : v4l2_compat_cpp_args)
There may be parallel installations of libcamera with separate instances of the v4l2-adaptation layer shared object. This shared library can be linked against a specific libcamera soversion so provide a soversioned V4L2 compatibility layer. Suggested-by: Javier Martinez Canillas <javierm@redhat.com> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- src/v4l2/meson.build | 1 + 1 file changed, 1 insertion(+)