On 08/09/2016 16:45, Krzysztof Konopko wrote:
This commit adds ability to load external back-end implementations which is
required if they are not (and can not be) maintained within 'weston' tree.
libweston-backend is introduced to provide common back-end functionality to
external implementations. Also few required functions are exported as
well and libweston-backend.so library is introduced.
Signed-off-by: Krzysztof Konopko <[email protected]>
Hi,
I think this is not a bad idea to have support for more backends.
However, it must be clear which type of backend is needed, and why it
cannot be in Weston repository. (It is not a requirement for the commit
message, but as an informal addition for ML archiving.)
A second point against this patch is that backend have a big rework
coming soon, with Armin’s work during GSoC. We are waiting for the next
release cycle to start merging his patches, as we are currently in
Release Candidate state.
This leads to the third point: this patch seems to have been written
against the pre-libweston code. You are re-introducing "struct
weston_config" in the backend init function, but it was removed on
purpose. If you want such custom backends, use the exact same init
function (which I hope we will rename on the next cycle, to properly
namespace it).
Now, a few inline comments for your next version (which should wait for
Armin’s patches, or be built on top of them).
---
Makefile.am | 63 +++++++++++++++++++++++++----------------------
compositor/main.c | 7 +++---
libweston/compositor.c | 16 ++++++++++++
libweston/compositor.h | 10 ++++++++
libweston/libinput-seat.c | 10 ++++----
libweston/libweston.pc.in | 2 +-
shared/config-parser.c | 2 +-
shared/option-parser.c | 4 ++-
8 files changed, 73 insertions(+), 41 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 1e63a58..d65646e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -205,30 +205,6 @@ weston_SOURCES = \
compositor/main.c : $(top_builddir)/libweston/git-version.h
libweston/compositor.c : $(top_builddir)/libweston/git-version.h
-noinst_LTLIBRARIES += \
- libsession-helper.la
-
-libsession_helper_la_SOURCES = \
- libweston/launcher-util.c \
- libweston/launcher-util.h \
- libweston/launcher-impl.h \
- libweston/weston-launch.h \
- libweston/launcher-weston-launch.c \
- libweston/launcher-direct.c
-libsession_helper_la_CFLAGS = $(AM_CFLAGS) $(LIBDRM_CFLAGS) $(PIXMAN_CFLAGS)
$(COMPOSITOR_CFLAGS)
-libsession_helper_la_LIBADD = $(LIBDRM_LIBS)
-
-if ENABLE_DBUS
-if HAVE_SYSTEMD_LOGIN
-libsession_helper_la_SOURCES += \
- libweston/dbus.h \
- libweston/dbus.c \
- libweston/launcher-logind.c
-libsession_helper_la_CFLAGS += $(SYSTEMD_LOGIN_CFLAGS) $(DBUS_CFLAGS)
-libsession_helper_la_LIBADD += $(SYSTEMD_LOGIN_LIBS) $(DBUS_LIBS)
-endif
-endif
-
if HAVE_GIT_REPO
libweston/git-version.h : $(top_srcdir)/.git/logs/HEAD
$(AM_V_GEN)echo "#define BUILD_ID \"$(shell git --git-dir=$(top_srcdir)/.git
describe --always --dirty) $(shell git --git-dir=$(top_srcdir)/.git log -1 --format='%s
(%ci)')\"" > $@
@@ -285,8 +261,11 @@ libwestoninclude_HEADERS = \
libweston/compositor-rdp.h \
libweston/compositor-wayland.h \
libweston/compositor-x11.h \
+ libweston/launcher-util.h \
+ libweston/libinput-seat.h \
libweston/plugin-registry.h \
libweston/timeline-object.h \
+ protocol/presentation-time-server-protocol.h \
Why this header? It should not be installed. And it should be safe to
use any version of this header with any version of libweston, thanks to
libwayland design.
shared/matrix.h \
shared/config-parser.h \
shared/zalloc.h
@@ -337,7 +316,12 @@ x11_backend_la_SOURCES = \
shared/helpers.h
endif
-INPUT_BACKEND_LIBS = $(LIBINPUT_BACKEND_LIBS)
+lib_LTLIBRARIES += \
+ libweston-backend.la
+
+libweston_backend_la_CFLAGS = $(AM_CFLAGS) $(LIBDRM_CFLAGS) $(PIXMAN_CFLAGS)
$(COMPOSITOR_CFLAGS)
+libweston_backend_la_LIBADD = $(LIBDRM_LIBS) $(LIBINPUT_BACKEND_LIBS)
libshared.la
+
INPUT_BACKEND_SOURCES = \
libweston/libinput-seat.c \
libweston/libinput-seat.h \
@@ -345,16 +329,38 @@ INPUT_BACKEND_SOURCES = \
libweston/libinput-device.h \
shared/helpers.h
+SESSION_HELPER_SOURCES = \
+ libweston/launcher-util.c \
+ libweston/launcher-util.h \
+ libweston/launcher-impl.h \
+ libweston/weston-launch.h \
+ libweston/launcher-weston-launch.c \
+ libweston/launcher-direct.c
+
+if ENABLE_DBUS
+if HAVE_SYSTEMD_LOGIN
+SESSION_HELPER_SOURCES += \
+ libweston/dbus.h \
+ libweston/dbus.c \
+ libweston/launcher-logind.c
+libweston_backend_la_CFLAGS += $(SYSTEMD_LOGIN_CFLAGS) $(DBUS_CFLAGS)
+libweston_backend_la_LIBADD += $(SYSTEMD_LOGIN_LIBS) $(DBUS_LIBS)
+endif
+endif
+
+libweston_backend_la_SOURCES = \
+ $(INPUT_BACKEND_SOURCES) \
+ $(SESSION_HELPER_SOURCES)
+
Since you move everything in this library, you should remove both
INPUT_BACKEND_SOURCES and SESSION_HELPER_SOURCES entirely.
if ENABLE_DRM_COMPOSITOR
libweston_module_LTLIBRARIES += drm-backend.la
drm_backend_la_LDFLAGS = -module -avoid-version
drm_backend_la_LIBADD = \
$(COMPOSITOR_LIBS) \
$(DRM_COMPOSITOR_LIBS) \
- $(INPUT_BACKEND_LIBS) \
libshared.la \
$(CLOCK_GETTIME_LIBS) \
- libsession-helper.la
+ libweston-backend.la
drm_backend_la_CFLAGS = \
$(COMPOSITOR_CFLAGS) \
$(EGL_CFLAGS) \
@@ -416,8 +422,7 @@ fbdev_backend_la_LDFLAGS = -module -avoid-version
fbdev_backend_la_LIBADD = \
$(COMPOSITOR_LIBS) \
$(FBDEV_COMPOSITOR_LIBS) \
- $(INPUT_BACKEND_LIBS) \
- libsession-helper.la \
+ libweston-backend.la \
libshared.la
fbdev_backend_la_CFLAGS = \
$(COMPOSITOR_CFLAGS) \
diff --git a/compositor/main.c b/compositor/main.c
index 0e5af5b..a6ab81e 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -1525,7 +1525,6 @@ load_wayland_backend(struct weston_compositor *c,
return ret;
}
-
Unrelated change.
static int
load_backend(struct weston_compositor *compositor, const char *backend,
int *argc, char **argv, struct weston_config *config)
@@ -1542,9 +1541,9 @@ load_backend(struct weston_compositor *compositor, const
char *backend,
return load_x11_backend(compositor, argc, argv, config);
else if (strstr(backend, "wayland-backend.so"))
return load_wayland_backend(compositor, argc, argv, config);
-
- weston_log("Error: unknown backend \"%s\"\n", backend);
- return -1;
+ else
+ return weston_compositor_load_backend_ext(compositor, backend,
+ argc, argv, config);
I would not allow Weston to load such a backend. But that means no
possible testing…
I’ll let the others comment on that, but I would rather not allow Weston
to load arbitrary backends. It’s up to each compositor to have its own
whitelist.
(Yes, Weston can load arbitrary modules, but that’s clear in the
documentation, at least.)
}
static char *
diff --git a/libweston/compositor.c b/libweston/compositor.c
index 47907bb..685ebbb 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -5104,6 +5104,22 @@ weston_compositor_load_backend(struct weston_compositor
*compositor,
}
WL_EXPORT int
+weston_compositor_load_backend_ext(struct weston_compositor *compositor,
+ char *name, int *argc, char **argv,
+ struct weston_config *wc)
+{
+ int (*backend_init_ext)(struct weston_compositor *c,
+ int *argc, char **argv,
+ struct weston_config *wc);
+
+ backend_init_ext = weston_load_module(name, "backend_init_ext");
+ if (!backend_init_ext)
+ return -1;
+
+ return backend_init_ext(compositor, argc, argv, wc);
+}
+
As I said, re-use the same init function, without argc/argc/config.
+WL_EXPORT int
weston_compositor_load_xwayland(struct weston_compositor *compositor)
{
int (*module_init)(struct weston_compositor *ec,
diff --git a/libweston/compositor.h b/libweston/compositor.h
index 16db03b..1d94eb3 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -1568,6 +1568,12 @@ int
weston_compositor_load_backend(struct weston_compositor *compositor,
enum weston_compositor_backend backend,
struct weston_backend_config *config_base);
+
+int
+weston_compositor_load_backend_ext(struct weston_compositor *compositor,
+ char *name, int *argc, char **argv,
+ struct weston_config *wc);
+
void
weston_compositor_exit(struct weston_compositor *ec);
void *
@@ -1765,6 +1771,10 @@ int
backend_init(struct weston_compositor *c,
struct weston_backend_config *config_base);
int
+backend_init_ext(struct weston_compositor *compositor, int *argc, char *argv[],
+ struct weston_config * config);
+
+int
module_init(struct weston_compositor *compositor,
int *argc, char *argv[]);
diff --git a/libweston/libinput-seat.c b/libweston/libinput-seat.c
index 78a5fc4..97ca4e2 100644
--- a/libweston/libinput-seat.c
+++ b/libweston/libinput-seat.c
@@ -128,7 +128,7 @@ udev_seat_remove_devices(struct udev_seat *seat)
}
}
-void
+WL_EXPORT void
udev_input_disable(struct udev_input *input)
{
if (input->suspended)
@@ -224,7 +224,7 @@ const struct libinput_interface libinput_interface = {
close_restricted,
};
-int
+WL_EXPORT int
udev_input_enable(struct udev_input *input)
{
struct wl_event_loop *loop;
@@ -281,7 +281,7 @@ libinput_log_func(struct libinput *libinput,
weston_vlog(format, args);
}
-int
+WL_EXPORT int
udev_input_init(struct udev_input *input, struct weston_compositor *c,
struct udev *udev, const char *seat_id,
udev_configure_device_t configure_device)
@@ -326,7 +326,7 @@ udev_input_init(struct udev_input *input, struct
weston_compositor *c,
return udev_input_enable(input);
}
-void
+WL_EXPORT void
udev_input_destroy(struct udev_input *input)
{
struct udev_seat *seat, *next;
@@ -403,7 +403,7 @@ udev_seat_destroy(struct udev_seat *seat)
free(seat);
}
-struct udev_seat *
+WL_EXPORT struct udev_seat *
udev_seat_get_named(struct udev_input *input, const char *seat_name)
{
struct udev_seat *seat;
If we make them public, we should properly namespace them too.
diff --git a/libweston/libweston.pc.in b/libweston/libweston.pc.in
index 2391003..30b411a 100644
--- a/libweston/libweston.pc.in
+++ b/libweston/libweston.pc.in
@@ -8,4 +8,4 @@ Description: Header files for libweston compositors development
Version: @WESTON_VERSION@
Requires.private: wayland-server pixman-1 xkbcommon
Cflags: -I${includedir}/libweston-@LIBWESTON_MAJOR@
-Libs: -L${libdir} -lweston-@LIBWESTON_MAJOR@
+Libs: -L${libdir} -lweston-@LIBWESTON_MAJOR@ -lweston-backend
No. It is a different library, so make it have its own .pc file.
diff --git a/shared/config-parser.c b/shared/config-parser.c
index e2b5fa2..e2b90aa 100644
--- a/shared/config-parser.c
+++ b/shared/config-parser.c
@@ -403,7 +403,7 @@ section_add_entry(struct weston_config_section *section,
return entry;
}
-struct weston_config *
+WL_EXPORT struct weston_config *
weston_config_parse(const char *name)
{
FILE *fp;
diff --git a/shared/option-parser.c b/shared/option-parser.c
index eee7546..08c29e7 100644
--- a/shared/option-parser.c
+++ b/shared/option-parser.c
@@ -32,6 +32,8 @@
#include <assert.h>
#include <errno.h>
+#include <wayland-util.h>
+
#include "config-parser.h"
#include "string-helpers.h"
@@ -135,7 +137,7 @@ short_option_with_arg(const struct weston_option *options,
int count, char *arg,
return 0;
}
-int
+WL_EXPORT int
parse_options(const struct weston_option *options,
int count, int *argc, char *argv[])
{
These two files changes are then not needed.
--
2.1.4
Cheers,
--
Quentin “Sardem FF7” Glidic
_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
