[PATCH weston 3/6] README: Document versioning scheme, forward compatibility

Thu Jul 7 09:05:14 UTC 2016

On Wed, 6 Jul 2016 13:57:48 +0100
Emil Velikov <emil.l.velikov at gmail.com> wrote:

> On 5 July 2016 at 15:46, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> > On Mon,  4 Jul 2016 15:23:51 +0100
> > Emil Velikov <emil.l.velikov at gmail.com> wrote:
> >  
> >> From: Emil Velikov <emil.velikov at collabora.com>
> >>
> >> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> >> ---
> >> Pekka,
> >>
> >> There's a couple of things to 'break' - forward and backward
> >> compatibility.
> >>
> >> Latter implies changing (removing) certain existing API, while the
> >> former is used in reference to functionality introduced with minor
> >> bumps.
> >>
> >> Since people don't always know when the new API is introduced, let alone
> >> bump the version accordingly in configure (and thus package runtime
> >> dependency), things end up badly.
> >>
> >> In some subtle cases (the autogenerated headers in wayland) not only is
> >> the new API available, but you end up using it without knowing. And yes,
> >> I fully agree that approach used in wayland is good, but it can cause
> >> subtle breakage.
> >>
> >> If weston devs don't want this approach (i.e. adding the ifdef guards
> >> prove too annoying and/or other), then one can just stick with only with
> >> MAJOR. Then the number will ramp quite fast and user will have no way
> >> of knowing/detecting bugfix (patch versions).
> >>
> >> From my experience, using the LIBWESTON_API_VERSION alike macros does
> >> not get in the way of development. Yet it's up-to you guys to make the
> >> call.
> >>
> >> -Emil
> >> ---
> >>  README | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 46 insertions(+)
> >>
> >> diff --git a/README b/README
> >> index 126df4d..72e8c7c 100644
> >> --- a/README
> >> +++ b/README
> >> @@ -70,6 +70,52 @@ For more information about parallel installability, see
> >>  http://ometer.com/parallel.html
> >>
> >>
> >> +Versioning scheme
> >> +-----------------
> >> +
> >> +In order to provide consistent, easy to use versioning, libweston
> >> +follows the rules in the Apache Portable Runtime Project
> >> +http://apr.apache.org/versioning.html.  
> >
> > Hi Emil,
> >
> > I read that doc, and I fully agree with it now.
> >  
> >> +
> >> +The document provides the full details, with the gist summed below:
> >> + - Major - backward incompatible changes.
> >> + - Minor - new backward compatible features.
> >> + - Patch - internal (implementation specific) fixes.
> >> +
> >> +
> >> +Forward compatibility
> >> +---------------------
> >> +
> >> +In order to ensure prevent subtle breaks with a simple recompile  
> >
> > Strike "ensure" perhaps?
> >  
> Yes, well spotted.
> 
> >> +(against a newer version), features introduced with minor versions are is
> >> +guarded with a LIBWESTON_API_VERSION guard.
> >> +
> >> +For example:
> >> +Libweston v1.1.0 introduces a new entry point weston_ham_sandwich().
> >> +As such it will be annotated as below in the relevant header(s).
> >> +
> >> +#if LIBWESTON_API_VERSION >= 0x0101
> >> +
> >> +bool
> >> +weston_ham_sandwich(void);
> >> +
> >> +#endif
> >> +
> >> +As the user requires the said symbol, they must explicitly set the
> >> +LIBWESTON_API_VERSION macro. By doing so they explicitly state "yes I
> >> +want to use the said version of the library", at which point they should
> >> +also bump the version check in their configure (or equivalent) script.
> >> +
> >> +The LIBWESTON_API_VERSION is of the format 0x$MAJOR$MINOR and does not
> >> +include PATCH version. As mentioned in the Versioning scheme section,
> >> +PATCH does not reflect any user visible API changes, thus should be not
> >> +considered part of the API VERSION.
> >> +
> >> +Similar approach is used by ATK, QT and KDE programs/libraries,
> >> +libjpeg-turbo, GTK, NetworkManager (in a more complex/compherensive
> >> +manner), js17, lz4 and many others.  
> >
> > This is interesting and the very first time I hear about such a scheme.
> > Obviously I have never used any library that required me to declare
> > which version I am expecting to use of it at the CPP level.
> >
> > Also the APR versioning doc says nothing about it,  
> Neither have I found any reference/docs on the topic sadly.
> 
> > and I couldn't find
> > any GTK documentation of a similar mechanism. libjpeg-turbo seems to
> > have something of the sort indeed, however it also defines
> > JPEG_LIB_VERSION itself, not the user of the library. I suspect there
> > is some misunderstanding here. I didn't check the others.
> >  
> On the libjpeg-turbo front, JPEG_LIB_VERSION is _optionally_ defined
> and controls only the structs' layout.
> 
> On the GTK side I've made a typo - it should have been GDK.
> Here is a snippet from /usr/include/gtk-3.0/gdk/gdkversionmacros.h
> 
> /**
>  * GDK_VERSION_MAX_ALLOWED:
>  *
>  * A macro that should be defined by the user prior to including
>  * the gdk.h header.
>  * The definition should be one of the predefined GDK version
>  * macros: %GDK_VERSION_3_0, %GDK_VERSION_3_2,...
>  *
>  * This macro defines the upper bound for the GDK API to use.
>  *
>  * If a function has been introduced in a newer version of GDK,
>  * it is possible to use this symbol to get compiler warnings when
>  * trying to use that function.
>  *
>  * Since: 3.4
>  */
> 
> Obviously the exact method (define upper/lower/other limit) and
> effects (warn, hide, error) differ between libraries but the overall
> idea is there.
> 
> That said, I could have misunderstood something.

Hi Emil,

oh cool. I picked the two not best examples to look at. :-)

> > We might need this kind of scheme for deprecating things in libwayland,
> > too.
> >  
> Yes, I was hoping to get to that at some point.
> 
> > So, let's see how it works for libweston. I'm in favour of this.
> >
> > I think we should also have an example for how do this in a
> > configure.ac nicely, so that you define the version you want to use
> > just once, and from that it computes:
> > - which libweston-$N.pc to use, and the version check with it for
> >   pkg-config
> > - the generation of #define LIBWESTON_API_VERSION into config.h
> >  
> Ack. I'll add a snippet of how the configure changes should look.
> 
> > Btw. is it usual for the *users* to define LIBWESTON_API_VERSION? To me
> > it sounds like something libweston would define. Users might define
> > something like REQUIRE_LIBWESTON_API_VERSION, no?
> >  
> The name I chose is causing some confusion.
> REQUIRE_LIBWESTON_API_VERSION sounds a lot better.

Now that you mentioned the semantics could be of upper or lower
limit, the name should imply the meaning. I only thought of using
it as both lower limit (as in pkg-config check) *and* upper limit
(do not declare API that was added after the given version or
remove API that was deprecated after the given version).

However, I also realized something more.

If you do like I suggested for configure.ac, you would have trouble
using pre-releases of libweston.

Pre-releases of, say 1.13.0, will be released as 1.12.9x. That
would be MINOR=12, so if you use the same numbers for both
pkg-config check and for the CPP macro for API, you cannot test the
13 API until it has been finally released.

So maybe those two need to be separate anyway?

> 
> >> +
> >> +
> >>  Libweston design goals
> >>  ----------------------
> >>  
> >
> > The one typo(?) and the bikeshedding about the name aside:
> > Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> >  

Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160707/36d1338e/attachment.sig>