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

Thu Jul 7 16:11:35 UTC 2016

On 7 July 2016 at 10:05, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> 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).
>
Ack. Considering I already know how it would/should be used, I'm a bit
short on unbiased name. Your earlier suggestion sounds great imho, but
if you have something else in mind please share it :-)

> 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?
>
The example will cover this confusion, but the gist is that:
 - No released version of software X should rely on development
version of libweston/libfoo.
 - If there's a serious need to void the above, one should set the
REQUIRED_API to the number it would be in an official/final version.

Example:
libweston (devel. process):
- git tag 1.12.90
- Add API X, guarded behind version 0x010d version check (one could
keep the numbers decimal and/or other)
- other...

User:
- Bump required version to 1.12.90
- Use "-DREQUIRED_API_VERSION=0x010d"
Personally I'm leaning towards adding the define only in the
respective places that need the newer API, but having it in configure
(global scale) would be nice as well.

Thanks
Emil