[PATCH] Build a libweston library

Quentin Glidic sardemff7+wayland at sardemff7.net
Fri May 13 08:55:30 UTC 2016


On 11/05/2016 21:31, Giulio Camuffo wrote:
> This patch creates a libweston-N.so library, where N is the "libweston_abi_version"
> defined in configure.ac. Almost all the code that was previously built in the
> weston binary is now in libweston.sp, except main.c and log.c.
> Possibly other files will need to be moved to weston but it can be done when we
> identify which files are they.
> When the API/ABI of libweston changes in an incompatible way, the abi version
> will be bumped so that multiple versions can live together.
> Also a libweston-N.pc file is created and installed together with weston.pc.
>
> Signed-off-by: Giulio Camuffo <giuliocamuffo at gmail.com>
> ---
>  Makefile.am         | 39 ++++++++++++++++++++++++---------------
>  configure.ac        |  3 +++
>  src/input.c         |  4 ++--
>  src/libweston.pc.in | 11 +++++++++++
>  4 files changed, 40 insertions(+), 17 deletions(-)
>  create mode 100644 src/libweston.pc.in
>
> diff --git a/Makefile.am b/Makefile.am
> index 0b7aa35..fbc509d 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -3,7 +3,7 @@ ACLOCAL_AMFLAGS = -I m4
>  bin_PROGRAMS =
>  noinst_PROGRAMS =
>  libexec_PROGRAMS =
> -moduledir = $(libdir)/weston
> +moduledir = $(libdir)/weston-${LIBWESTON_ABI_VERSION}

As Benoit said, you must split what is weston and what is libweston here.
Let’s introduce libweston*dir. I prefer …/libweston-${abi}, because 
…/weston-${abi} is confusing and can look like a leftover or wrong rename.
Also see an unrelated (to libweston) comment at the end of this email.

>  module_LTLIBRARIES =
>  noinst_LTLIBRARIES =
>  BUILT_SOURCES =
> @@ -59,17 +59,15 @@ CLEANFILES = weston.ini				\
>  	internal-screenshot-00.png		\
>  	$(BUILT_SOURCES)
>
> -bin_PROGRAMS += weston
> +lib_LTLIBRARIES = libweston.la
> +libweston_la_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON
> +libweston_la_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS)

You are missing $(AM_CFLAGS) here.
If adding it breaks something, then fix AM_CFLAGS.


> +libweston_la_LIBADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \
> +        $(DLOPEN_LIBS) -lm libshared.la
> +libweston_la_LDFLAGS = -release ${LIBWESTON_ABI_VERSION}
>
> -weston_LDFLAGS = -export-dynamic
> -weston_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON
> -weston_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS)
> -weston_LDADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \
> -	$(DLOPEN_LIBS) -lm $(CLOCK_GETTIME_LIBS) libshared.la
> -
> -weston_SOURCES =					\
> +libweston_la_SOURCES =					\
>  	src/git-version.h				\
> -	src/log.c					\
>  	src/compositor.c				\
>  	src/compositor.h				\
>  	src/compositor-drm.h				\
> @@ -92,7 +90,6 @@ weston_SOURCES =					\
>  	src/timeline.c					\
>  	src/timeline.h					\
>  	src/timeline-object.h				\
> -	src/main.c					\
>  	src/linux-dmabuf.c				\
>  	src/linux-dmabuf.h				\
>  	shared/helpers.h				\
> @@ -119,7 +116,7 @@ systemd_notify_la_SOURCES =			\
>  	src/compositor.h
>  endif
>
> -nodist_weston_SOURCES =					\
> +nodist_libweston_la_SOURCES =					\
>  	protocol/weston-screenshooter-protocol.c			\
>  	protocol/weston-screenshooter-server-protocol.h			\
>  	protocol/text-cursor-position-protocol.c	\
> @@ -135,7 +132,19 @@ nodist_weston_SOURCES =					\
>  	protocol/linux-dmabuf-unstable-v1-protocol.c	\
>  	protocol/linux-dmabuf-unstable-v1-server-protocol.h
>
> -BUILT_SOURCES += $(nodist_weston_SOURCES)
> +BUILT_SOURCES += $(nodist_libweston_la_SOURCES)
> +
> +bin_PROGRAMS += weston
> +
> +weston_LDFLAGS = -export-dynamic
> +weston_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON
> +weston_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS)
> +weston_LDADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \
> +	$(DLOPEN_LIBS) -lm libshared.la libweston.la
> +
> +weston_SOURCES = 				\
> +	src/main.c				\
> +	src/log.c
>
>  # Track this dependency explicitly instead of using BUILT_SOURCES.  We
>  # add BUILT_SOURCES to CLEANFILES, but we want to keep git-version.h
> @@ -205,12 +214,12 @@ endif
>  endif # BUILD_WESTON_LAUNCH
>
>  pkgconfigdir = $(libdir)/pkgconfig
> -pkgconfig_DATA = src/weston.pc
> +pkgconfig_DATA = src/weston.pc src/libweston-${LIBWESTON_ABI_VERSION}.pc
>
>  wayland_sessiondir = $(datadir)/wayland-sessions
>  dist_wayland_session_DATA = src/weston.desktop
>
> -westonincludedir = $(includedir)/weston
> +westonincludedir = $(includedir)/weston-${LIBWESTON_ABI_VERSION}

Same as moduledir.


>  westoninclude_HEADERS =				\
>  	src/version.h				\
>  	src/compositor.h			\
> diff --git a/configure.ac b/configure.ac
> index 2ca1f4e..94aad57 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -3,6 +3,7 @@ m4_define([weston_minor_version], [10])
>  m4_define([weston_micro_version], [91])
>  m4_define([weston_version],
>            [weston_major_version.weston_minor_version.weston_micro_version])
> +m4_define([libweston_abi_version], [0])
>
>  AC_PREREQ([2.64])
>  AC_INIT([weston],
> @@ -15,6 +16,7 @@ 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_CONFIG_AUX_DIR([build-aux])
>  AC_CONFIG_HEADERS([config.h])
> @@ -643,6 +645,7 @@ if test "x$enable_systemd_notify" = "xyes"; then
>  fi
>
>  AC_CONFIG_FILES([Makefile src/version.h src/weston.pc])
> +AC_CONFIG_FILES([src/libweston-${LIBWESTON_ABI_VERSION}.pc:src/libweston.pc.in])
>
>  AM_CONDITIONAL([HAVE_GIT_REPO], [test -f $srcdir/.git/logs/HEAD])
>
> diff --git a/src/input.c b/src/input.c
> index 8fe898c..c6cdedd 100644
> --- a/src/input.c
> +++ b/src/input.c
> @@ -2277,7 +2277,7 @@ bind_seat(struct wl_client *client, void *data, uint32_t version, uint32_t id)
>  }
>
>  #ifdef ENABLE_XKBCOMMON
> -int
> +WL_EXPORT int

I would do that in a preparation/cleanup commit.


>  weston_compositor_xkb_init(struct weston_compositor *ec,
>  			   struct xkb_rule_names *names)
>  {
> @@ -2442,7 +2442,7 @@ weston_compositor_build_global_keymap(struct weston_compositor *ec)
>  	return 0;
>  }
>  #else
> -int
> +WL_EXPORT int
>  weston_compositor_xkb_init(struct weston_compositor *ec,
>  			   struct xkb_rule_names *names)
>  {
> diff --git a/src/libweston.pc.in b/src/libweston.pc.in
> new file mode 100644
> index 0000000..3f66997
> --- /dev/null
> +++ b/src/libweston.pc.in
> @@ -0,0 +1,11 @@
> +prefix=@prefix@
> +exec_prefix=@exec_prefix@
> +libdir=@libdir@
> +includedir=@includedir@
> +libexecdir=@libexecdir@
> +pkglibexecdir=${libexecdir}/@PACKAGE@

libexecdir is useless here.


> +
> +Name: libweston API
> +Description: Header files for libweston compositors development
> +Version: @WESTON_VERSION@
> +Cflags: -I${includedir} -lweston- at LIBWESTON_ABI_VERSION@

libweston is taking over compositor.h, right? In that case, please see 
commit 63225cf2527ff9f54fc111c030c8e293748ec47d for weston.pc, you are 
missing three Requires.private here.

Again, as Benoit said, -l is for Libs:.

I would also argue that since the include directory now has 
${abi_version} in it, you should put ${pkgincludedir} in Cflags: instead 
of ${includedir}, so users won’t have to #include 
<libweston-${abi_version}/whatever.h>



Directories definitions un-libweston-related comment:
I am in favour of defining all directories in a single place[1], at the 
top of configure.ac (before dependencies checking), so we can AC_SUBST 
directly the .pc files without manually adding @PACKAGE@ and friends. 
That effectively means redefining pkg*dir variables.
This is mostly cosmetic though.

[1] 
<https://github.com/sardemff7/eventd/blob/f24a0e95a3bf8aa654db76da11b1b9dd30f67897/configure.ac#L93>


Cheers,

-- 

Quentin “Sardem FF7” Glidic


More information about the wayland-devel mailing list