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

[Emil Velikov]

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.

> 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.

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

Thanks
Emil