On 4 July 2016 at 15:45, Quentin Glidic <sardemff7+wayl...@sardemff7.net> wrote:
> On 04/07/2016 16:23, Emil Velikov wrote:
>>
>> Use the documented libweston-$major.so.0.$minor.$patch scheme.
>>
>> An (almost) identical one is used by GLIB, GTK{2,3}, QT5, json-glib and
>> others.
>>
>> v2:
>> - Use shorter variable names LIBWESTON_{MAJOR,MINOR...}
>> - Correctly use -version-info.
>> - Drop unneeded @LIBWESTON_VERSION_MAJOR@ additions.
>>
>> Signed-off-by: Emil Velikov <emil.l.veli...@collabora.com>
>> ---
>>
>> XXX:
>> - Should we rename libweston{,-0}.pc.in ?
>> - Drop the s/LIBWESTON_ABI_VERSION/LIBWESTON_MAJOR/ hunk ?
>> - Keep the configure libweston_*_version variables shorter ?
>> - Yes, the LT_VERSION_INFO hunk looks a bit nasty, yet this is what GTK
>> is doing, once you unraver the 2-3 layers of variables. It works as
>> expected though, and I'd imagine others are doing a similar trick.
>>
>> fixup! libweston: use new versioning scheme
>
>
> Looks good. A bit hard to read but I thing you at least build-tested it. :-)
>
> I think you can drop the "v2" part in the commit message, btw.
>
I've noticed that other commits keep their revision history above the
--- line, and thus is present in git log. So I take that it's a nice
to have, but here it's just useless ?
> One last comment inline to make it perfect.
>
>
>
>> ---
>> Makefile.am | 24 ++++++++++++------------
>> compositor/weston.pc.in | 2 +-
>> configure.ac | 12 +++++++++---
>> 3 files changed, 22 insertions(+), 16 deletions(-)
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index b050c60..1c3d553 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_MAJOR)
>> libweston_module_LTLIBRARIES =
>> noinst_LTLIBRARIES =
>> BUILT_SOURCES =
>> @@ -61,15 +61,15 @@ 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-@LIBWESTON_MAJOR@.la
>> +libweston_@LIBWESTON_MAJOR@_la_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON
>> +libweston_@LIBWESTON_MAJOR@_la_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS)
>> $(LIBUNWIND_CFLAGS)
>> +libweston_@LIBWESTON_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_@LIBWESTON_MAJOR@_la_LDFLAGS = -version-info $(LT_VERSION_INFO)
>>
>> -libweston_la_SOURCES = \
>> +libweston_@LIBWESTON_MAJOR@la_SOURCES = \
>> libweston/git-version.h \
>> libweston/log.c \
>> libweston/compositor.c \
>> @@ -121,7 +121,7 @@ systemd_notify_la_SOURCES = \
>> libweston/compositor.h
>> endif
>>
>> -nodist_libweston_la_SOURCES = \
>> +nodist_libweston_@LIBWESTON_MAJOR@_la_SOURCES =
>> \
>> protocol/weston-screenshooter-protocol.c \
>> protocol/weston-screenshooter-server-protocol.h \
>> protocol/text-cursor-position-protocol.c \
>> @@ -137,7 +137,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_@LIBWESTON_MAJOR@_la_SOURCES)
>>
>> bin_PROGRAMS += weston
>>
>> @@ -148,7 +148,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-@LIBWESTON_MAJOR@.la
>>
>> weston_SOURCES = \
>> compositor/main.c \
>> @@ -225,12 +225,12 @@ endif
>> endif # BUILD_WESTON_LAUNCH
>>
>> pkgconfigdir = $(libdir)/pkgconfig
>> -pkgconfig_DATA = compositor/weston.pc
>> libweston/libweston-${LIBWESTON_ABI_VERSION}.pc
>> +pkgconfig_DATA = compositor/weston.pc
>> libweston/libweston-${LIBWESTON_MAJOR}.pc
>>
>> wayland_sessiondir = $(datadir)/wayland-sessions
>> dist_wayland_session_DATA = compositor/weston.desktop
>>
>> -libwestonincludedir = $(includedir)/libweston-${LIBWESTON_ABI_VERSION}
>> +libwestonincludedir = $(includedir)/libweston-${LIBWESTON_MAJOR}
>> libwestoninclude_HEADERS = \
>> libweston/version.h \
>> libweston/compositor.h \
>> diff --git a/compositor/weston.pc.in b/compositor/weston.pc.in
>> index 09e8580..6890a77 100644
>> --- a/compositor/weston.pc.in
>> +++ b/compositor/weston.pc.in
>> @@ -8,5 +8,5 @@ pkglibexecdir=${libexecdir}/@PACKAGE@
>> Name: Weston Plugin API
>> Description: Header files for Weston plugin development
>> Version: @WESTON_VERSION@
>> -Requires.private: libweston-@LIBWESTON_ABI_VERSION@
>> +Requires.private: libweston-@LIBWESTON_MAJOR@
>> Cflags: -I${includedir}/weston
>> diff --git a/configure.ac b/configure.ac
>> index 85a475a..be40f10 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_patch_version], [0])
>>
>> AC_PREREQ([2.64])
>> AC_INIT([weston],
>> @@ -18,7 +20,11 @@ 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_MAJOR], [libweston_major_version])
>> +m4_define([lt_current], [libweston_minor_version])
>> +m4_define([lt_revision], [libweston_patch_version])
>> +m4_define([lt_age], [libweston_minor_version])
>> +AC_SUBST([LT_VERSION_INFO], [lt_current:lt_revision:lt_age])
>
>
> I had to do the whole version updating script twice to fully understand
> that.
> Please add a comment saying:
> “We use minor as current and age since on ABI/API break/removal we bump
> major, so minor will be reset to 0.”
> or anything clear.
I'm afraid that the whole -version-info still makes little sense over
here, so anything that you/others suggest will go in as-is. If you
prefer we could fully opt the GTK route, making it clearer from
libtool POV. I'm fearful that will end up more confusing for the
maintainer/release person - even the GTK people get it wrong every now
and then.
> The confusing thing here is that current and age are not supposed to be in
> sync, and current is supposed to be incremented forever.
> Except that here, we have a “new lib” on major bump, so current is fine to
> be 0 again.
>
As per above - any suggestions how to make it less confusing are be
appreciated - more inline comments, alternative commit message, etc.
Thanks
Emil
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
