[PATCH weston 2/6] Split the modules and include files between weston and libweston
Benoit Gschwind
gschwind at gnu-log.net
Fri May 27 12:05:13 UTC 2016
Hello Giulio,
The idea of the patch is fine, but the patch doesn't work. See following
comments. (@Quentin: no it's does not work as it is ;) )
On 24/05/2016 18:59, Giulio Camuffo wrote:
> The backends are now installed in lib/libweston-0, and the include
> files that will be used by libweston in include/libweston-0. The other
> modules and weston-specific include files are kept in the old paths.
> A new load_weston_module() is added to load plugins in the old path,
> which is not part of libweston, but weston only and defined in main.c.
> To allow that to be used by out of tree weston plugins, the function
> is declared in a new weston.h, installed in include/weston.
>
> The -0 in the paths is the abi version of libweston, and it will be
> used by the libweston .so too. When the abi change the number will
> be increased.
>
> Signed-off-by: Giulio Camuffo <giuliocamuffo at gmail.com>
> ---
> Makefile.am | 28 +++++++++++++++++-----------
> configure.ac | 2 ++
> ivi-shell/ivi-layout.c | 3 ++-
> src/compositor.c | 2 +-
> src/main.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
> src/weston.h | 40 ++++++++++++++++++++++++++++++++++++++++
> src/weston.pc.in | 2 +-
> 7 files changed, 109 insertions(+), 15 deletions(-)
> create mode 100644 src/weston.h
>
> diff --git a/Makefile.am b/Makefile.am
> index 00b74e5..da37526 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -5,6 +5,8 @@ noinst_PROGRAMS =
> libexec_PROGRAMS =
> moduledir = $(libdir)/weston
> module_LTLIBRARIES =
> +libweston_moduledir = $(libdir)/libweston-${LIBWESTON_ABI_VERSION}
> +libweston_module_LTLIBRARIES =
> noinst_LTLIBRARIES =
> BUILT_SOURCES =
>
> @@ -49,7 +51,7 @@ AM_CPPFLAGS = \
> -I$(top_srcdir)/shared \
> -I$(top_builddir)/protocol \
> -DDATADIR='"$(datadir)"' \
> - -DMODULEDIR='"$(moduledir)"' \
Here you replace MODULESDIR as already commented by Quentin. But you
need MODULEDIR to build weston-compositor. I fixed the build issue by
adding MODULEDIR to weston-compositor C(PP)FLAGS as follow:
diff --git a/Makefile.am b/Makefile.am
index da37526..2554e1f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -64,8 +64,10 @@ CLEANFILES = weston.ini \
bin_PROGRAMS += weston
weston_LDFLAGS = -export-dynamic
-weston_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON
-weston_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS)
+weston_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON \
+ -DMODULEDIR='"$(moduledir)"'
+weston_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS) \
+ -DMODULEDIR='"$(moduledir)"'
weston_LDADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \
$(DLOPEN_LIBS) -lm $(CLOCK_GETTIME_LIBS) libshared.la
After this change weston build and run.
> + -DLIBWESTON_MODULEDIR='"$(libweston_moduledir)"' \
> -DLIBEXECDIR='"$(libexecdir)"' \
> -DBINDIR='"$(bindir)"'
>
> @@ -95,6 +97,7 @@ weston_SOURCES = \
> src/main.c \
> src/linux-dmabuf.c \
> src/linux-dmabuf.h \
> + src/weston.h \
> shared/helpers.h \
> shared/matrix.c \
> shared/matrix.h \
> @@ -104,7 +107,7 @@ weston_SOURCES = \
> src/weston-egl-ext.h
>
> if SYSTEMD_NOTIFY_SUPPORT
> -module_LTLIBRARIES += systemd-notify.la
> +libweston_module_LTLIBRARIES += systemd-notify.la
> systemd_notify_la_LDFLAGS = -module -avoid-version
> systemd_notify_la_LIBADD = $(SYSTEMD_DAEMON_LIBS)
> systemd_notify_la_CFLAGS = \
> @@ -210,8 +213,8 @@ pkgconfig_DATA = src/weston.pc
> wayland_sessiondir = $(datadir)/wayland-sessions
> dist_wayland_session_DATA = src/weston.desktop
>
> -westonincludedir = $(includedir)/weston
> -westoninclude_HEADERS = \
> +libwestonincludedir = $(includedir)/libweston-${LIBWESTON_ABI_VERSION}
> +libwestoninclude_HEADERS = \
> src/version.h \
> src/compositor.h \
> src/compositor-drm.h \
> @@ -226,13 +229,16 @@ westoninclude_HEADERS = \
> shared/zalloc.h \
> shared/platform.h
>
> +westonincludedir = $(includedir)/weston
> +westoninclude_HEADERS = src/weston.h
> +
> if ENABLE_IVI_SHELL
> westoninclude_HEADERS += \
> ivi-shell/ivi-layout-export.h
> endif
>
> if ENABLE_EGL
> -module_LTLIBRARIES += gl-renderer.la
> +libweston_module_LTLIBRARIES += gl-renderer.la
> gl_renderer_la_LDFLAGS = -module -avoid-version
> gl_renderer_la_LIBADD = $(COMPOSITOR_LIBS) $(EGL_LIBS)
> gl_renderer_la_CFLAGS = \
> @@ -249,7 +255,7 @@ gl_renderer_la_SOURCES = \
> endif
>
> if ENABLE_X11_COMPOSITOR
> -module_LTLIBRARIES += x11-backend.la
> +libweston_module_LTLIBRARIES += x11-backend.la
> x11_backend_la_LDFLAGS = -module -avoid-version
> x11_backend_la_LIBADD = $(COMPOSITOR_LIBS) $(X11_COMPOSITOR_LIBS) \
> libshared-cairo.la
> @@ -275,7 +281,7 @@ INPUT_BACKEND_SOURCES = \
> shared/helpers.h
>
> if ENABLE_DRM_COMPOSITOR
> -module_LTLIBRARIES += drm-backend.la
> +libweston_module_LTLIBRARIES += drm-backend.la
> drm_backend_la_LDFLAGS = -module -avoid-version
> drm_backend_la_LIBADD = \
> $(COMPOSITOR_LIBS) \
> @@ -306,7 +312,7 @@ endif
> endif
>
> if ENABLE_WAYLAND_COMPOSITOR
> -module_LTLIBRARIES += wayland-backend.la
> +libweston_module_LTLIBRARIES += wayland-backend.la
> wayland_backend_la_LDFLAGS = -module -avoid-version
> wayland_backend_la_LIBADD = \
> $(COMPOSITOR_LIBS) \
> @@ -363,7 +369,7 @@ endif
> endif
>
> if ENABLE_HEADLESS_COMPOSITOR
> -module_LTLIBRARIES += headless-backend.la
> +libweston_module_LTLIBRARIES += headless-backend.la
> headless_backend_la_LDFLAGS = -module -avoid-version
> headless_backend_la_LIBADD = $(COMPOSITOR_LIBS) libshared.la
> headless_backend_la_CFLAGS = $(COMPOSITOR_CFLAGS) $(AM_CFLAGS)
> @@ -374,7 +380,7 @@ headless_backend_la_SOURCES = \
> endif
>
> if ENABLE_FBDEV_COMPOSITOR
> -module_LTLIBRARIES += fbdev-backend.la
> +libweston_module_LTLIBRARIES += fbdev-backend.la
> fbdev_backend_la_LDFLAGS = -module -avoid-version
> fbdev_backend_la_LIBADD = \
> $(COMPOSITOR_LIBS) \
> @@ -396,7 +402,7 @@ fbdev_backend_la_SOURCES = \
> endif
>
> if ENABLE_RDP_COMPOSITOR
> -module_LTLIBRARIES += rdp-backend.la
> +libweston_module_LTLIBRARIES += rdp-backend.la
> rdp_backend_la_LDFLAGS = -module -avoid-version
> rdp_backend_la_LIBADD = $(COMPOSITOR_LIBS) \
> $(RDP_COMPOSITOR_LIBS) \
> diff --git a/configure.ac b/configure.ac
> index e1300b4..282b05f 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -3,6 +3,7 @@ m4_define([weston_minor_version], [10])
> m4_define([weston_micro_version], [92])
> 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])
> diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
> index 7fa8b33..d91b8c8 100644
> --- a/ivi-shell/ivi-layout.c
> +++ b/ivi-shell/ivi-layout.c
> @@ -60,6 +60,7 @@
> #include <string.h>
> #include <assert.h>
>
> +#include "weston.h"
> #include "compositor.h"
> #include "ivi-shell.h"
> #include "ivi-layout-export.h"
> @@ -2070,7 +2071,7 @@ load_controller_modules(struct weston_compositor *compositor, const char *module
> end = strchrnul(p, ',');
> snprintf(buffer, sizeof buffer, "%.*s", (int)(end - p), p);
>
> - controller_module_init = weston_load_module(buffer, "controller_module_init");
> + controller_module_init = load_weston_module(buffer, "controller_module_init");
> if (!controller_module_init)
> return -1;
>
> diff --git a/src/compositor.c b/src/compositor.c
> index b6ef7f3..5a52d86 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -4935,7 +4935,7 @@ weston_load_module(const char *name, const char *entrypoint)
> if (builddir)
> snprintf(path, sizeof path, "%s/.libs/%s", builddir, name);
> else
> - snprintf(path, sizeof path, "%s/%s", MODULEDIR, name);
> + snprintf(path, sizeof path, "%s/%s", LIBWESTON_MODULEDIR, name);
> } else {
> snprintf(path, sizeof path, "%s", name);
> }
> diff --git a/src/main.c b/src/main.c
> index 3279ac6..7b52910 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -37,12 +37,14 @@
> #include <sys/utsname.h>
> #include <sys/stat.h>
> #include <sys/wait.h>
> +#include <linux/limits.h>
>
> #ifdef HAVE_LIBUNWIND
> #define UNW_LOCAL_ONLY
> #include <libunwind.h>
> #endif
>
> +#include "weston.h"
> #include "compositor.h"
> #include "../shared/os-compatibility.h"
> #include "../shared/helpers.h"
> @@ -473,6 +475,49 @@ weston_create_listening_socket(struct wl_display *display, const char *socket_na
> return 0;
> }
>
the following function name "load_weston_module" is error prone in
regards of current weston_load_module function name. Moreover as they do
similar job in different context ;)
I suggest to rename load_weston_module or/and weston_load_module. To be
constructive here is my brainstorm:
weston_load_module could be:
- weston_load_submodule
- weston_load_internal_module
- weston_load_backend_module
- libweston_load_module // in preparation of libweston new prefix ?
- someprefix_load_module // in preparation of someprefix new prefix ?
load_weston_module could be:
- weston_compositor_load_module // We could use weston_compositor as new
prefix.
- weston_load_plugin
- weston_load_frontend_module
any proposal that avoid similar name are welcome :)
> +WL_EXPORT void *
> +load_weston_module(const char *name, const char *entrypoint)
> +{
> + const char *builddir = getenv("WESTON_BUILD_DIR");
> + char path[PATH_MAX];
> + void *module, *init;
> +
> + if (name == NULL)
> + return NULL;
> +
> + if (name[0] != '/') {
> + if (builddir)
> + snprintf(path, sizeof path, "%s/.libs/%s", builddir, name);
> + else
> + snprintf(path, sizeof path, "%s/%s", MODULEDIR, name);
> + } else {
> + snprintf(path, sizeof path, "%s", name);
> + }
> +
> + module = dlopen(path, RTLD_NOW | RTLD_NOLOAD);
> + if (module) {
> + weston_log("Module '%s' already loaded\n", path);
> + dlclose(module);
> + return NULL;
> + }
> +
> + weston_log("Loading module '%s'\n", path);
> + module = dlopen(path, RTLD_NOW);
> + if (!module) {
> + weston_log("Failed to load module: %s\n", dlerror());
> + return NULL;
> + }
> +
> + init = dlsym(module, entrypoint);
> + if (!init) {
> + weston_log("Failed to lookup init function: %s\n", dlerror());
> + dlclose(module);
> + return NULL;
> + }
> +
> + return init;
> +}
> +
> static int
> load_modules(struct weston_compositor *ec, const char *modules,
> int *argc, char *argv[])
> @@ -489,7 +534,7 @@ load_modules(struct weston_compositor *ec, const char *modules,
> while (*p) {
> end = strchrnul(p, ',');
> snprintf(buffer, sizeof buffer, "%.*s", (int) (end - p), p);
> - module_init = weston_load_module(buffer, "module_init");
> + module_init = load_weston_module(buffer, "module_init");
> if (!module_init)
> return -1;
> if (module_init(ec, argc, argv) < 0)
> diff --git a/src/weston.h b/src/weston.h
> new file mode 100644
> index 0000000..63f47cd
> --- /dev/null
> +++ b/src/weston.h
> @@ -0,0 +1,40 @@
> +/*
> + * Copyright © 2016 Giulio Camuffo
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining
> + * a copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sublicense, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial
> + * portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#ifndef WESTON_H
> +#define WESTON_H
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +void *
> +load_weston_module(const char *name, const char *entrypoint);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif
> diff --git a/src/weston.pc.in b/src/weston.pc.in
> index c560eb3..f2ffc9e 100644
> --- a/src/weston.pc.in
> +++ b/src/weston.pc.in
> @@ -9,4 +9,4 @@ Name: Weston Plugin API
> Description: Header files for Weston plugin development
> Version: @WESTON_VERSION@
> Requires.private: wayland-server pixman-1 xkbcommon
> -Cflags: -I${includedir}
> +Cflags: -I${includedir}/weston -I{includedir}/libweston- at LIBWESTON_ABI_VERSION@
>
Best regard
--
Benoit Gschwind
More information about the wayland-devel
mailing list