[libcamera-devel] [PATCH 1/4] meson: Shared Object version handling
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat Oct 1 02:07:08 CEST 2022
On Sat, Oct 01, 2022 at 12:44:01AM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2022-10-01 00:29:06)
> > On Fri, Sep 30, 2022 at 11:03:27PM +0100, Kieran Bingham wrote:
> > > Quoting Laurent Pinchart (2022-09-30 21:50:44)
> > > > Hi Kieran,
> > > >
> > > > (CC'ing Javier)
> > > >
> > > > Final lap before a release (even if it's just a minor one), that's nice
> > > > :-)
> > > >
> > > > On Fri, Sep 30, 2022 at 04:44:33PM +0100, Kieran Bingham via libcamera-devel wrote:
> > > > > Quoting Jacopo Mondi (2022-09-30 14:54:32)
> > > > > > On Thu, Sep 29, 2022 at 03:36:23PM +0100, Kieran Bingham via libcamera-devel wrote:
> > > > > > > The libcamera project is not yet ready to declare ABI nor API stability,
> > > > > > > but it will benefit the community to be able to provide more regular
> > > > > > > release cycles to determine 'versioned' points of history.
> > > > > > >
> > > > > > > Ideally, these releases will be made at any ABI breakage, but can be
> > > > > > > made at arbitary time based points along the way.
> > > > > > >
> > > > > > > To support releases which may not be ABI stable, declare the soversion
> > > > > > > of both the libcamera and libcamera-base library to be dependant upon
> > > > > > > both the major and minor component of the project version.
> > > > > > >
> > > > > > > As part of this, introduce a new 'Versions' summary section to highlight
> > > > > > > the different version components that may become apparent within any
> > > > > > > given build.
> > > > > > >
> > > > > > > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > > >
> > > > I tried the script and it seemed to work, but compiling libcamera then
> > > > gave me the following warnings:
> > > >
> > > > warning: source './x86-gcc-11.3.0/src/libcamera/libcamera.so.0.0' is not a readable file or directory... skipping.
> > > > warning: source './x86-gcc-11.3.0/src/libcamera/libcamera.so.0' is not a readable file or directory... skipping.
> > > > warning: source './x86-gcc-11.3.0/src/libcamera/libcamera.so' is not a readable file or directory... skipping.
> > > > warning: source './x86-gcc-11.3.0/src/libcamera/base/libcamera-base.so.0' is not a readable file or directory... skipping
> > >
> > > Hrm ... looks like that might be doxygen?
> >
> > I think you're right.
> >
> > meson generates the symlinks for the shared libraries at configure time,
> > so if Doxygen runs before the binary is built, the links will be broken.
> > Furthermore, meson fails to remove old links when the soname is bumped,
> > so broken links will remain in place. We thus can't fix the problem by
> > just adding a dependency on libcamera and libcamera-base to the Doxygen
> > target like I initially thought.
> >
> > > This fixes it.
> > >
> > > diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
> > > index 88dfcddaebf6..761807005294 100644
> > > --- a/Documentation/Doxyfile.in
> > > +++ b/Documentation/Doxyfile.in
> > > @@ -46,7 +46,9 @@ EXCLUDE = @TOP_SRCDIR@/include/libcamera/base/span.h \
> > > @TOP_BUILDDIR@/include/libcamera/internal/tracepoints.h \
> > > @TOP_BUILDDIR@/src/libcamera/proxy/
> > >
> > > -EXCLUDE_PATTERNS = @TOP_BUILDDIR@/include/libcamera/ipa/*_serializer.h \
> > > +EXCLUDE_PATTERNS = @TOP_BUILDDIR@/src/libcamera/libcamera.so* \
> > > + @TOP_BUILDDIR@/src/libcamera/base/libcamera-base.so* \
> > > + @TOP_BUILDDIR@/include/libcamera/ipa/*_serializer.h \
> > > @TOP_BUILDDIR@/include/libcamera/ipa/*_proxy.h \
> > > @TOP_BUILDDIR@/include/libcamera/ipa/ipu3_*.h \
> > > @TOP_BUILDDIR@/include/libcamera/ipa/raspberrypi_*.h \
> >
> > I can't think of a better option :-S
> >
> > > > > > > ---
> > > > > > > meson.build | 15 +++++++++++++++
> > > > > > > src/libcamera/base/meson.build | 1 +
> > > > > > > src/libcamera/meson.build | 1 +
> > > > > > > 3 files changed, 17 insertions(+)
> > > > > > >
> > > > > > > diff --git a/meson.build b/meson.build
> > > > > > > index 72919102ad55..9bbfd0e9c784 100644
> > > > > > > --- a/meson.build
> > > > > > > +++ b/meson.build
> > > > > > > @@ -26,6 +26,21 @@ endif
> > > > > > >
> > > > > > > libcamera_version = libcamera_git_version.split('+')[0]
> > > > > > >
> > > > > > > +# Enforce Major and Minor as part of the soversion. Until we make a first major
> > > > > > > +# release, and remain on version 0.x each release may denote ABI instabilty.
> > > > > >
> > > > > > Missing a , after 0.x ?
> > > > >
> > > > > Yes, looks like the , after release should move to after 0.x,
> > > >
> > > > That's more readable indeed.
> > > >
> > > > > > nit apart:
> > > > > > Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> > > > >
> > > > > Thanks.
> > > > >
> > > > > > > +# We can continue to consider that a patch level increment should be
> > > > > > > +# compatible.
> > > > > > > +project_version = meson.project_version().split('.')
> > > > > > > +soversion = project_version[0] + '.' + project_version[1]
> > > >
> > > > As we have libcamera_version and not just version, I'd have named the
> > > > variable libcamera_soversion.
> > > >
> > > > Why do you use two components of the version only ? As we don't
> > > > guarantee ABI stability, could we use all three components for now ?
> > > > This could then be shortened to major.minor when we'll add ABI change
> > > > detection (but still without ABI stability), and finally to just major
> > > > once ABI stability will be guaranteed. Javier, do you have any opinion ?
> > >
> > > I believe we don't define ABI stability in either of major or minor. But
> > > we 'do' (or can) in .patch. (We're also unlikely to create a .patch
> > > relase, unless we find a specific need to fix something for a
> > > distributions release, but distributions might still make patch
> > > releases...).
> >
> > Why wouldn't we start with patch releases ?
>
> Becaause that prohibits anyone else adding fixes to that release point.
>
> If Ubuntu take libcamera-0.5 ... but there's a bug .. they can fix it
> (or we can) in libcamera-0.5.1.
>
> If we do patch release that's not possible.
This is an area where Javier may shed some light. I see lots of
libraries that make use of major, minor and patch themselves, it doesn't
seem like the patch number is meant to be reserved for distributions.
At some point in the future I'm quite sure we'll have multiple
maintained stable branches, with bug fix releases. We're not there yet,
so I'd rather start by doing 0.0.x releases to start with, and see where
it leads.
> > > So I don't see specific benefit in including it in the sovesion.
> > >
> > > > > > > +
> > > > > > > +summary({
> > > > > > > + 'Project': meson.project_version(),
> > > > > > > + 'Sources': libcamera_git_version,
> > > > > > > + 'libcamera': libcamera_version,
> > > >
> > > > Is there ever a case where libcamera_version should be different than
> > > > meson.project_version() ?
> > >
> > > *should be* ... no. *can be* yes.
> > >
> > > I think this can occur if you clone a repo without fetching tags.
> > >
> > > That's why it's split to highlight ;-)
> > >
> > >
> > > On a local build that has had a 'release', but all tags removed
> > >
> > > libcamera 0.1.0
> > >
> > > Versions
> > > Project : 0.1.0
> > > Sources : 0.0.0+3954-a64a132e
> > > libcamera : 0.0.0
> > > Shared Object : 0.1
> >
> > Is this something we really have to handle ? :-) If so, there's more to
> > fix, as the "sources" version is what libcamera will print in its log,
> > and it won't match the soname or project version. The "libcamera"
> > version is what will end up in version.h, which could also confuse
> > applications. I see three options, we could consider that:
> >
> > - This really shouldn't happen, and let people who clone the git tree
> > without tags suffer from their mistake if they decide to shoot
> > themselves in the foot.
>
> Yup. I'm here right now.
>
> > - This could happen by mistake and we should fail loudly at configure
> > time, to be kind to people's feet.
>
> I'm not sure failing to build because of a version string is friendly.
> But not having the right 'version' isn't nice either.
If it's really not supposed to happen, I think this would be more
user-friendly than letting it go through silently, and having the user
experience subtle and hard to debug issues. The mismatch in the versions
printed at configure time will likely be ignored, if noticed at all. And
if users complain about this problem, then we'll get to learn about
valid use cases, in which case we would switch to the strategy below.
TL;DR: I would fail at configure time if we detect a problem, and lower
the number of versions we print to remove duplicates.
> But I think we're trying to fight corner cases here.
>
> > - This is a valid case, and the project version from meson.build should
> > then be use to construct the other versions if tags are missing.
>
> That gets difficult, (or ... more difficult) as you won't be able to
> determine 'what' point the specific release was made at, so you can't
> generate any additional information. We'd end up losing helpful
> information.
Yes, it's not nice.
> If you want to update utils/gen-version.sh ... we can do that on top.
On top, and later, if we realize there are valid use cases.
> > > > > > > + 'Shared Object': soversion,
> > > >
> > > > I would have named this 'soname' (or 'SONAME'), that's the official
> > > > term. 'Shared Object' is ambiguous.
> > >
> > > It's in the 'version' section, and is the Shared Object Version. I.e.
> > > the soversion.
> > >
> > > It can be soname though.
> >
> > I'd still prefer soname, it's more explicit.
> >
> > > > > > > + },
> > > > > > > + section : 'Versions')
> > > > > > > +
> > > > > > > # This script gererates the .tarball-version file on a 'meson dist' command.
> > > >
> > > > While at it you could fix the "gererates" typo.
> > > >
> > > > > > > meson.add_dist_script('utils/run-dist.sh')
> > > > > > >
> > > > > > > diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build
> > > > > > > index 3b9d74efe935..51719f112d26 100644
> > > > > > > --- a/src/libcamera/base/meson.build
> > > > > > > +++ b/src/libcamera/base/meson.build
> > > > > > > @@ -51,6 +51,7 @@ libcamera_base_args = [ '-DLIBCAMERA_BASE_PRIVATE' ]
> > > > > > > libcamera_base_lib = shared_library('libcamera-base',
> > > > > > > [libcamera_base_sources, libcamera_base_headers],
> > > > > > > version : libcamera_version,
> > > > > > > + soversion : soversion,
> > > > > > > name_prefix : '',
> > > > > > > install : true,
> > > > > > > cpp_args : libcamera_base_args,
> > > > > > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > > > > > > index 63b47b177fd2..3aa7f32067f8 100644
> > > > > > > --- a/src/libcamera/meson.build
> > > > > > > +++ b/src/libcamera/meson.build
> > > > > > > @@ -160,6 +160,7 @@ libcamera_deps = [
> > > > > > > libcamera = shared_library('libcamera',
> > > > > > > libcamera_sources,
> > > > > > > version : libcamera_version,
> > > > > > > + soversion : soversion,
> > > > > > > name_prefix : '',
> > > > > > > install : true,
> > > > > > > include_directories : includes,
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list