[RFC weston 4/4] WIP: libweston: rework versioning scheme

[Pekka Paalanen]

On Fri,  3 Jun 2016 14:27:39 +0100
Emil Velikov <emil.l.velikov at gmail.com> wrote:

> From: Emil Velikov <emil.velikov at collabora.com>
> 
> Use libweston-$major.so.0.$minor.$patch over the current scheme.

Hi,

is that really a commonly used versioning pattern?
Could you add a reference to the project you used as an example?
Is that project actually happy with it, or are they just stuck with it
because of compatiblity reasons?

> It allows for separation (distinction) of the backwards incompatible
> changes from forward compatible feature/bugfix ones.
> 
> TODO:
>  - Check if we need the - at LIBWESTON_VERSION_MAJOR@ headers changes.
>  - Check where do we want to use @foo@, ${foo} and $(foo).
> 
> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
> ---
>  Makefile.am  | 27 ++++++++++++++-------------
>  configure.ac |  8 ++++++--
>  2 files changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index d1d2178..99c5bfe 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -5,7 +5,7 @@ noinst_PROGRAMS =
>  libexec_PROGRAMS =
>  moduledir = $(libdir)/weston
>  module_LTLIBRARIES =
> -libweston_moduledir = $(libdir)/libweston-${LIBWESTON_ABI_VERSION}
> +libweston_moduledir = $(libdir)/libweston-$(LIBWESTON_VERSION_MAJOR)
>  libweston_module_LTLIBRARIES =
>  noinst_LTLIBRARIES =
>  BUILT_SOURCES =
> @@ -61,15 +61,16 @@ CLEANFILES = weston.ini				\
>  	internal-screenshot-00.png		\
>  	$(BUILT_SOURCES)
>  
> -lib_LTLIBRARIES = libweston.la
> -libweston_la_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON
> -libweston_la_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS)
> -libweston_la_LIBADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \
> +lib_LTLIBRARIES = libweston- at LIBWESTON_VERSION_MAJOR@.la
> +libweston_ at LIBWESTON_VERSION_MAJOR@_la_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON
> +libweston_ at LIBWESTON_VERSION_MAJOR@_la_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS)
> +libweston_ at LIBWESTON_VERSION_MAJOR@_la_LIBADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \
>  	$(DLOPEN_LIBS) -lm $(CLOCK_GETTIME_LIBS) \
>  	$(LIBINPUT_BACKEND_LIBS) libshared.la
> -libweston_la_LDFLAGS = -release ${LIBWESTON_ABI_VERSION}
> +libweston_ at LIBWESTON_VERSION_MAJOR@_la_LDFLAGS = -version-info 0:$(LIBWESTON_VERSION_MICRO):$(LIBWESTON_VERSION_PATCH)
>  

This looks suspiciously simple use of -version-info. Is it really correct?
Why not -avoid-version?

> -libweston_la_SOURCES =					\
> +
> +libweston_ at LIBWESTON_VERSION_MAJOR@_la_SOURCES =					\
>  	src/git-version.h				\
>  	src/log.c					\
>  	src/compositor.c				\
> @@ -119,7 +120,7 @@ systemd_notify_la_SOURCES =			\
>  	src/compositor.h
>  endif
>  
> -nodist_libweston_la_SOURCES =					\
> +nodist_libweston_ at LIBWESTON_VERSION_MAJOR@_la_SOURCES =					\
>  	protocol/weston-screenshooter-protocol.c			\
>  	protocol/weston-screenshooter-server-protocol.h			\
>  	protocol/text-cursor-position-protocol.c	\
> @@ -135,7 +136,7 @@ nodist_libweston_la_SOURCES =					\
>  	protocol/linux-dmabuf-unstable-v1-protocol.c	\
>  	protocol/linux-dmabuf-unstable-v1-server-protocol.h
>  
> -BUILT_SOURCES += $(nodist_libweston_la_SOURCES)
> +BUILT_SOURCES += $(nodist_libweston_ at LIBWESTON_VERSION_MAJOR@_la_SOURCES)
>  
>  bin_PROGRAMS += weston
>  
> @@ -145,7 +146,7 @@ weston_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON 		\
>  weston_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS)
>  weston_LDADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \
>  	$(DLOPEN_LIBS) $(LIBINPUT_BACKEND_LIBS) \
> -	-lm libshared.la libweston.la
> +	-lm libshared.la libweston- at LIBWESTON_VERSION_MAJOR@.la
>  
>  weston_SOURCES = 					\
>  	src/main.c					\
> @@ -220,13 +221,13 @@ endif
>  endif # BUILD_WESTON_LAUNCH
>  
>  pkgconfigdir = $(libdir)/pkgconfig
> -pkgconfig_DATA = src/weston.pc src/libweston-${LIBWESTON_ABI_VERSION}.pc
> +pkgconfig_DATA = src/weston.pc src/libweston- at LIBWESTON_VERSION_MAJOR@.pc
>  
>  wayland_sessiondir = $(datadir)/wayland-sessions
>  dist_wayland_session_DATA = src/weston.desktop
>  
> -libwestonincludedir = $(includedir)/libweston-${LIBWESTON_ABI_VERSION}
> -libwestoninclude_HEADERS =				\
> +libweston_ at LIBWESTON_VERSION_MAJOR@includedir = $(includedir)/libweston-$(LIBWESTON_VERSION_MAJOR)
> +libweston_ at LIBWESTON_VERSION_MAJOR@include_HEADERS =				\

These two renames probably aren't necessary?
We have libweston_moduledir also.

>  	src/version.h				\
>  	src/compositor.h			\
>  	src/compositor-drm.h			\
> diff --git a/configure.ac b/configure.ac
> index 2be2277..78457be 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -3,7 +3,9 @@ m4_define([weston_minor_version], [11])
>  m4_define([weston_micro_version], [90])
>  m4_define([weston_version],
>            [weston_major_version.weston_minor_version.weston_micro_version])
> -m4_define([libweston_abi_version], [0])
> +m4_define([libweston_major_version], [0])
> +m4_define([libweston_minor_version], [0])
> +m4_define([libweston_micro_version], [0])

Why do you add all these three, when we only need one?

>  
>  AC_PREREQ([2.64])
>  AC_INIT([weston],
> @@ -18,7 +20,9 @@ AC_SUBST([WESTON_VERSION_MAJOR], [weston_major_version])
>  AC_SUBST([WESTON_VERSION_MINOR], [weston_minor_version])
>  AC_SUBST([WESTON_VERSION_MICRO], [weston_micro_version])
>  AC_SUBST([WESTON_VERSION], [weston_version])
> -AC_SUBST([LIBWESTON_ABI_VERSION], [libweston_abi_version])
> +AC_SUBST([LIBWESTON_VERSION_MAJOR], [libweston_major_version])
> +AC_SUBST([LIBWESTON_VERSION_MINOR], [libweston_minor_version])
> +AC_SUBST([LIBWESTON_VERSION_MICRO], [libweston_micro_version])
>  
>  AC_CONFIG_AUX_DIR([build-aux])
>  AC_CONFIG_HEADERS([config.h])

I'm quite out of my depth with library versioning stuff. Can someone
claim to know this well enough to give a review, or should I start
learning?

I do agree that installing a libweston.so without the one number is not
good.


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/20160606/c57b6bf7/attachment.sig>