Discussion:
[PATCH Weston 1/1] desktop-shell: implement autolaunch
Pekka Paalanen
2014-10-22 13:53:55 UTC
Permalink
Process a new section 'autolaunch' from weston.ini, launching all
programs given there on desktop start-up.

[Frederic Plourde: cut redundancy between do_autolaunch and panel_add_launcher]
---
clients/desktop-shell.c | 97 ++++++++++++++++++++++++++++++++++++++++++-------
man/weston.ini.man | 17 +++++++++
weston.ini.in | 3 ++
3 files changed, 103 insertions(+), 14 deletions(-)

diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
index 961a9b2..a964094 100644
--- a/clients/desktop-shell.c
+++ b/clients/desktop-shell.c
@@ -597,72 +597,82 @@ load_icon_or_fallback(const char *icon)
cairo_move_to(cr, 4, 16);
cairo_line_to(cr, 16, 4);
cairo_stroke(cr);

cairo_destroy(cr);

return surface;
}

static void
-panel_add_launcher(struct panel *panel, const char *icon, const char *path)
+parse_launcher_path(char *path, struct wl_array *envp_array, struct wl_array *argv_array)
{
- struct panel_launcher *launcher;
char *start, *p, *eq, **ps;
int i, j, k;

- launcher = xzalloc(sizeof *launcher);
- launcher->icon = load_icon_or_fallback(icon);
- launcher->path = xstrdup(path);
+ struct wl_array *envp = envp_array;
+ struct wl_array *argv = argv_array;

- wl_array_init(&launcher->envp);
- wl_array_init(&launcher->argv);
+ wl_array_init(envp);
+ wl_array_init(argv);
for (i = 0; environ[i]; i++) {
- ps = wl_array_add(&launcher->envp, sizeof *ps);
+ ps = wl_array_add(envp, sizeof *ps);
*ps = environ[i];
}
j = 0;

- start = launcher->path;
+ start = path;
while (*start) {
for (p = start, eq = NULL; *p && !isspace(*p); p++)
if (*p == '=')
eq = p;

if (eq && j == 0) {
- ps = launcher->envp.data;
+ ps = envp->data;
for (k = 0; k < i; k++)
if (strncmp(ps[k], start, eq - start) == 0) {
ps[k] = start;
break;
}
if (k == i) {
- ps = wl_array_add(&launcher->envp, sizeof *ps);
+ ps = wl_array_add(envp, sizeof *ps);
*ps = start;
i++;
}
} else {
- ps = wl_array_add(&launcher->argv, sizeof *ps);
+ ps = wl_array_add(argv, sizeof *ps);
*ps = start;
j++;
}

while (*p && isspace(*p))
*p++ = '\0';

start = p;
}

- ps = wl_array_add(&launcher->envp, sizeof *ps);
+ ps = wl_array_add(envp, sizeof *ps);
*ps = NULL;
- ps = wl_array_add(&launcher->argv, sizeof *ps);
+ ps = wl_array_add(argv, sizeof *ps);
*ps = NULL;
+}
+
+static void
+panel_add_launcher(struct panel *panel, const char *icon, const char *path)
+{
+ struct panel_launcher *launcher;
+
+ launcher = xzalloc(sizeof *launcher);
+ launcher->icon = load_icon_or_fallback(icon);
+ launcher->path = xstrdup(path);
+
+ parse_launcher_path(launcher->path, &launcher->envp, &launcher->argv);

launcher->panel = panel;
wl_list_insert(panel->launcher_list.prev, &launcher->link);

launcher->widget = widget_add_widget(panel->widget, launcher);
widget_set_enter_handler(launcher->widget,
panel_launcher_enter_handler);
widget_set_leave_handler(launcher->widget,
panel_launcher_leave_handler);
widget_set_button_handler(launcher->widget,
@@ -1316,20 +1326,77 @@ panel_add_launchers(struct panel *panel, struct desktop *desktop)
}

if (count == 0) {
/* add default launcher */
panel_add_launcher(panel,
DATADIR "/weston/terminal.png",
BINDIR "/weston-terminal");
}
}

+static void
+do_autolaunch(const char *path_arg)
+{
+ struct wl_array envp;
+ struct wl_array argv;
+ pid_t pid;
+ char **argvpp;
+ char *path;
+
+ path = xstrdup(path_arg);
+
+ parse_launcher_path(path, &envp, &argv);
+
+ /* panel_launcher_activate */
+
+ pid = fork();
+ if (pid < 0) {
+ fprintf(stderr, "fork failed: %m\n");
+ goto out;
+ }
+
+ if (pid)
+ goto out;
+
+ argvpp = argv.data;
+ if (execve(argvpp[0], argvpp, envp.data) < 0) {
+ fprintf(stderr, "execl '%s' failed: %m\n", argvpp[0]);
+ exit(1);
+ }
+
+out:
+ wl_array_release(&argv);
+ wl_array_release(&envp);
+ free(path);
+}
+
+static void
+process_autolaunch(struct desktop *desktop)
+{
+ struct weston_config_section *s;
+ char *path;
+ const char *name;
+
+ s = NULL;
+ while (weston_config_next_section(desktop->config, &s, &name)) {
+ if (strcmp(name, "autolaunch") != 0)
+ continue;
+
+ weston_config_section_get_string(s, "path", &path, NULL);
+ if (!path)
+ continue;
+
+ do_autolaunch(path);
+ free(path);
+ }
+}
+
int main(int argc, char *argv[])
{
struct desktop desktop = { 0 };
struct output *output;
struct weston_config_section *s;

desktop.unlock_task.run = unlock_dialog_finish;
wl_list_init(&desktop.outputs);

desktop.config = weston_config_parse("weston.ini");
@@ -1349,20 +1416,22 @@ int main(int argc, char *argv[])
/* Create panel and background for outputs processed before the shell
* global interface was processed */
wl_list_for_each(output, &desktop.outputs, link)
if (!output->panel)
output_init(output, &desktop);

grab_surface_create(&desktop);

signal(SIGCHLD, sigchild_handler);

+ process_autolaunch(&desktop);
+
display_run(desktop.display);

/* Cleanup */
grab_surface_destroy(&desktop);
desktop_destroy_outputs(&desktop);
if (desktop.unlock_dialog)
unlock_dialog_destroy(desktop.unlock_dialog);
desktop_shell_destroy(desktop.shell);
display_destroy(desktop.display);

diff --git a/man/weston.ini.man b/man/weston.ini.man
index c05a221..365141c 100644
--- a/man/weston.ini.man
+++ b/man/weston.ini.man
@@ -66,20 +66,21 @@ Comment lines are ignored:
.RE
.PP
The section headers are:
.PP
.RS 4
.nf
.BR "core " "The core modules"
.BR "libinput " "Input device configuration"
.BR "shell " "Desktop customization"
.BR "launcher " "Add launcher to the panel"
+.BR "autolaunch " "Launch program automatically on startup"
.BR "screensaver " "Screensaver selection"
.BR "output " "Output configuration"
.BR "input-method " "Onscreen keyboard input"
.BR "keyboard " "Keyboard layouts"
.BR "terminal " "Terminal application options"
.BR "xwayland " "XWayland options"
.BR "screen-share " "Screen sharing options"
.fi
.RE
.PP
@@ -272,20 +273,36 @@ sets the path to the program that is run by clicking on this launcher (string).
It is possible to pass arguments and environment variables to the program. For
example:
.nf
.in +4n

path=GDK_BACKEND=wayland gnome-terminal --full-screen
.in
.fi
.PP
.RE
+.SH "AUTOLAUNCH SECTION"
+There can be multiple autolaunch sections for starting multiple programs.
+.TP 7
+.BI "path=" program
+sets the path (string) to the program that is run automatically when the
+desktop is starting.
+It is possible to pass arguments and environment variables to the program. For
+example:
+.nf
+.in +4n
+
+path=GDK_BACKEND=wayland gnome-terminal --full-screen
+.in
+.fi
+.PP
+.RE
.SH "SCREENSAVER SECTION"
The
.B screensaver
section is used to select and schedule a screensaver.
The
.B screensaver
section is optional, as are all of the entries that may be specified in
it.
.TP 7
.BI "path=" /usr/libexec/weston-screensaver
diff --git a/weston.ini.in b/weston.ini.in
index 4fca0bb..b0cb31f 100644
--- a/weston.ini.in
+++ b/weston.ini.in
@@ -30,20 +30,23 @@ icon=/usr/share/icons/gnome/24x24/apps/utilities-terminal.png
path=@bindir@/weston-terminal

[launcher]
icon=/usr/share/icons/hicolor/24x24/apps/google-chrome.png
path=/usr/bin/google-chrome

[launcher]
icon=/usr/share/icons/gnome/24x24/apps/arts.png
path=@abs_top_builddir@/weston-flower

+#[autolaunch]
+#path=@bindir@/weston-terminal
+
[screensaver]
# Comment path to disable screensaver
path=@libexecdir@/weston-screensaver
duration=600

[input-method]
path=@libexecdir@/weston-keyboard

#[output]
#name=LVDS1
--
1.9.1
Derek Foreman
2014-10-22 15:57:16 UTC
Permalink
I'd prefer to see the refactor and the new feature in separate patches,
but this is pretty trivial.

I also have a slight preference for exit(EXIT_FAILURE), which is already
used somewhere else in that file - though there's also precedent for
exit(1), so you make the call. :)

I'd not seen printf's %m until today - do we want to depend on a gnuism?
I've seen at least some activity towards a freebsd port - I don't
believe %m is supported there?


That said, it runs nicely here and does what it says on the tin...
Post by Pekka Paalanen
Process a new section 'autolaunch' from weston.ini, launching all
programs given there on desktop start-up.
[Frederic Plourde: cut redundancy between do_autolaunch and panel_add_launcher]
---
clients/desktop-shell.c | 97 ++++++++++++++++++++++++++++++++++++++++++-------
man/weston.ini.man | 17 +++++++++
weston.ini.in | 3 ++
3 files changed, 103 insertions(+), 14 deletions(-)
diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
index 961a9b2..a964094 100644
--- a/clients/desktop-shell.c
+++ b/clients/desktop-shell.c
@@ -597,72 +597,82 @@ load_icon_or_fallback(const char *icon)
cairo_move_to(cr, 4, 16);
cairo_line_to(cr, 16, 4);
cairo_stroke(cr);
cairo_destroy(cr);
return surface;
}
static void
-panel_add_launcher(struct panel *panel, const char *icon, const char *path)
+parse_launcher_path(char *path, struct wl_array *envp_array, struct wl_array *argv_array)
{
- struct panel_launcher *launcher;
char *start, *p, *eq, **ps;
int i, j, k;
- launcher = xzalloc(sizeof *launcher);
- launcher->icon = load_icon_or_fallback(icon);
- launcher->path = xstrdup(path);
+ struct wl_array *envp = envp_array;
+ struct wl_array *argv = argv_array;
- wl_array_init(&launcher->envp);
- wl_array_init(&launcher->argv);
+ wl_array_init(envp);
+ wl_array_init(argv);
for (i = 0; environ[i]; i++) {
- ps = wl_array_add(&launcher->envp, sizeof *ps);
+ ps = wl_array_add(envp, sizeof *ps);
*ps = environ[i];
}
j = 0;
- start = launcher->path;
+ start = path;
while (*start) {
for (p = start, eq = NULL; *p && !isspace(*p); p++)
if (*p == '=')
eq = p;
if (eq && j == 0) {
- ps = launcher->envp.data;
+ ps = envp->data;
for (k = 0; k < i; k++)
if (strncmp(ps[k], start, eq - start) == 0) {
ps[k] = start;
break;
}
if (k == i) {
- ps = wl_array_add(&launcher->envp, sizeof *ps);
+ ps = wl_array_add(envp, sizeof *ps);
*ps = start;
i++;
}
} else {
- ps = wl_array_add(&launcher->argv, sizeof *ps);
+ ps = wl_array_add(argv, sizeof *ps);
*ps = start;
j++;
}
while (*p && isspace(*p))
*p++ = '\0';
start = p;
}
- ps = wl_array_add(&launcher->envp, sizeof *ps);
+ ps = wl_array_add(envp, sizeof *ps);
*ps = NULL;
- ps = wl_array_add(&launcher->argv, sizeof *ps);
+ ps = wl_array_add(argv, sizeof *ps);
*ps = NULL;
+}
+
+static void
+panel_add_launcher(struct panel *panel, const char *icon, const char *path)
+{
+ struct panel_launcher *launcher;
+
+ launcher = xzalloc(sizeof *launcher);
+ launcher->icon = load_icon_or_fallback(icon);
+ launcher->path = xstrdup(path);
+
+ parse_launcher_path(launcher->path, &launcher->envp, &launcher->argv);
launcher->panel = panel;
wl_list_insert(panel->launcher_list.prev, &launcher->link);
launcher->widget = widget_add_widget(panel->widget, launcher);
widget_set_enter_handler(launcher->widget,
panel_launcher_enter_handler);
widget_set_leave_handler(launcher->widget,
panel_launcher_leave_handler);
widget_set_button_handler(launcher->widget,
@@ -1316,20 +1326,77 @@ panel_add_launchers(struct panel *panel, struct desktop *desktop)
}
if (count == 0) {
/* add default launcher */
panel_add_launcher(panel,
DATADIR "/weston/terminal.png",
BINDIR "/weston-terminal");
}
}
+static void
+do_autolaunch(const char *path_arg)
+{
+ struct wl_array envp;
+ struct wl_array argv;
+ pid_t pid;
+ char **argvpp;
+ char *path;
+
+ path = xstrdup(path_arg);
+
+ parse_launcher_path(path, &envp, &argv);
+
+ /* panel_launcher_activate */
+
+ pid = fork();
+ if (pid < 0) {
+ fprintf(stderr, "fork failed: %m\n");
+ goto out;
+ }
+
+ if (pid)
+ goto out;
+
+ argvpp = argv.data;
+ if (execve(argvpp[0], argvpp, envp.data) < 0) {
+ fprintf(stderr, "execl '%s' failed: %m\n", argvpp[0]);
+ exit(1);
+ }
+
+ wl_array_release(&argv);
+ wl_array_release(&envp);
+ free(path);
+}
+
+static void
+process_autolaunch(struct desktop *desktop)
+{
+ struct weston_config_section *s;
+ char *path;
+ const char *name;
+
+ s = NULL;
+ while (weston_config_next_section(desktop->config, &s, &name)) {
+ if (strcmp(name, "autolaunch") != 0)
+ continue;
+
+ weston_config_section_get_string(s, "path", &path, NULL);
+ if (!path)
+ continue;
+
+ do_autolaunch(path);
+ free(path);
+ }
+}
+
int main(int argc, char *argv[])
{
struct desktop desktop = { 0 };
struct output *output;
struct weston_config_section *s;
desktop.unlock_task.run = unlock_dialog_finish;
wl_list_init(&desktop.outputs);
desktop.config = weston_config_parse("weston.ini");
@@ -1349,20 +1416,22 @@ int main(int argc, char *argv[])
/* Create panel and background for outputs processed before the shell
* global interface was processed */
wl_list_for_each(output, &desktop.outputs, link)
if (!output->panel)
output_init(output, &desktop);
grab_surface_create(&desktop);
signal(SIGCHLD, sigchild_handler);
+ process_autolaunch(&desktop);
+
display_run(desktop.display);
/* Cleanup */
grab_surface_destroy(&desktop);
desktop_destroy_outputs(&desktop);
if (desktop.unlock_dialog)
unlock_dialog_destroy(desktop.unlock_dialog);
desktop_shell_destroy(desktop.shell);
display_destroy(desktop.display);
diff --git a/man/weston.ini.man b/man/weston.ini.man
index c05a221..365141c 100644
--- a/man/weston.ini.man
+++ b/man/weston.ini.man
.RE
.PP
.PP
.RS 4
.nf
.BR "core " "The core modules"
.BR "libinput " "Input device configuration"
.BR "shell " "Desktop customization"
.BR "launcher " "Add launcher to the panel"
+.BR "autolaunch " "Launch program automatically on startup"
.BR "screensaver " "Screensaver selection"
.BR "output " "Output configuration"
.BR "input-method " "Onscreen keyboard input"
.BR "keyboard " "Keyboard layouts"
.BR "terminal " "Terminal application options"
.BR "xwayland " "XWayland options"
.BR "screen-share " "Screen sharing options"
.fi
.RE
.PP
@@ -272,20 +273,36 @@ sets the path to the program that is run by clicking on this launcher (string).
It is possible to pass arguments and environment variables to the program. For
.nf
.in +4n
path=GDK_BACKEND=wayland gnome-terminal --full-screen
.in
.fi
.PP
.RE
+.SH "AUTOLAUNCH SECTION"
+There can be multiple autolaunch sections for starting multiple programs.
+.TP 7
+.BI "path=" program
+sets the path (string) to the program that is run automatically when the
+desktop is starting.
+It is possible to pass arguments and environment variables to the program. For
+.nf
+.in +4n
+
+path=GDK_BACKEND=wayland gnome-terminal --full-screen
+.in
+.fi
+.PP
+.RE
.SH "SCREENSAVER SECTION"
The
.B screensaver
section is used to select and schedule a screensaver.
The
.B screensaver
section is optional, as are all of the entries that may be specified in
it.
.TP 7
.BI "path=" /usr/libexec/weston-screensaver
diff --git a/weston.ini.in b/weston.ini.in
index 4fca0bb..b0cb31f 100644
--- a/weston.ini.in
+++ b/weston.ini.in
@@ -30,20 +30,23 @@ icon=/usr/share/icons/gnome/24x24/apps/utilities-terminal.png
[launcher]
icon=/usr/share/icons/hicolor/24x24/apps/google-chrome.png
path=/usr/bin/google-chrome
[launcher]
icon=/usr/share/icons/gnome/24x24/apps/arts.png
+#[autolaunch]
+
[screensaver]
# Comment path to disable screensaver
duration=600
[input-method]
#[output]
#name=LVDS1
Karsten Otto
2014-10-23 06:04:57 UTC
Permalink
The %m from glibc would indeed be a portability problem. However, it is already lightly used within wayland (11 occurrences) and heavily in weston (125 occurrences). I suggest you keep them for now, then clean them all up in one patch later - assuming the wayland community and prospective users consider portability a worthwhile effort.

Cheers, Karsten
Post by Derek Foreman
I'd prefer to see the refactor and the new feature in separate patches,
but this is pretty trivial.
I also have a slight preference for exit(EXIT_FAILURE), which is already
used somewhere else in that file - though there's also precedent for
exit(1), so you make the call. :)
I'd not seen printf's %m until today - do we want to depend on a gnuism?
I've seen at least some activity towards a freebsd port - I don't
believe %m is supported there?
That said, it runs nicely here and does what it says on the tin...
Post by Pekka Paalanen
Process a new section 'autolaunch' from weston.ini, launching all
programs given there on desktop start-up.
[Frederic Plourde: cut redundancy between do_autolaunch and panel_add_launcher]
---
clients/desktop-shell.c | 97 ++++++++++++++++++++++++++++++++++++++++++-------
man/weston.ini.man | 17 +++++++++
weston.ini.in | 3 ++
3 files changed, 103 insertions(+), 14 deletions(-)
diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
index 961a9b2..a964094 100644
--- a/clients/desktop-shell.c
+++ b/clients/desktop-shell.c
@@ -597,72 +597,82 @@ load_icon_or_fallback(const char *icon)
cairo_move_to(cr, 4, 16);
cairo_line_to(cr, 16, 4);
cairo_stroke(cr);
cairo_destroy(cr);
return surface;
}
static void
-panel_add_launcher(struct panel *panel, const char *icon, const char *path)
+parse_launcher_path(char *path, struct wl_array *envp_array, struct wl_array *argv_array)
{
- struct panel_launcher *launcher;
char *start, *p, *eq, **ps;
int i, j, k;
- launcher = xzalloc(sizeof *launcher);
- launcher->icon = load_icon_or_fallback(icon);
- launcher->path = xstrdup(path);
+ struct wl_array *envp = envp_array;
+ struct wl_array *argv = argv_array;
- wl_array_init(&launcher->envp);
- wl_array_init(&launcher->argv);
+ wl_array_init(envp);
+ wl_array_init(argv);
for (i = 0; environ[i]; i++) {
- ps = wl_array_add(&launcher->envp, sizeof *ps);
+ ps = wl_array_add(envp, sizeof *ps);
*ps = environ[i];
}
j = 0;
- start = launcher->path;
+ start = path;
while (*start) {
for (p = start, eq = NULL; *p && !isspace(*p); p++)
if (*p == '=')
eq = p;
if (eq && j == 0) {
- ps = launcher->envp.data;
+ ps = envp->data;
for (k = 0; k < i; k++)
if (strncmp(ps[k], start, eq - start) == 0) {
ps[k] = start;
break;
}
if (k == i) {
- ps = wl_array_add(&launcher->envp, sizeof *ps);
+ ps = wl_array_add(envp, sizeof *ps);
*ps = start;
i++;
}
} else {
- ps = wl_array_add(&launcher->argv, sizeof *ps);
+ ps = wl_array_add(argv, sizeof *ps);
*ps = start;
j++;
}
while (*p && isspace(*p))
*p++ = '\0';
start = p;
}
- ps = wl_array_add(&launcher->envp, sizeof *ps);
+ ps = wl_array_add(envp, sizeof *ps);
*ps = NULL;
- ps = wl_array_add(&launcher->argv, sizeof *ps);
+ ps = wl_array_add(argv, sizeof *ps);
*ps = NULL;
+}
+
+static void
+panel_add_launcher(struct panel *panel, const char *icon, const char *path)
+{
+ struct panel_launcher *launcher;
+
+ launcher = xzalloc(sizeof *launcher);
+ launcher->icon = load_icon_or_fallback(icon);
+ launcher->path = xstrdup(path);
+
+ parse_launcher_path(launcher->path, &launcher->envp, &launcher->argv);
launcher->panel = panel;
wl_list_insert(panel->launcher_list.prev, &launcher->link);
launcher->widget = widget_add_widget(panel->widget, launcher);
widget_set_enter_handler(launcher->widget,
panel_launcher_enter_handler);
widget_set_leave_handler(launcher->widget,
panel_launcher_leave_handler);
widget_set_button_handler(launcher->widget,
@@ -1316,20 +1326,77 @@ panel_add_launchers(struct panel *panel, struct desktop *desktop)
}
if (count == 0) {
/* add default launcher */
panel_add_launcher(panel,
DATADIR "/weston/terminal.png",
BINDIR "/weston-terminal");
}
}
+static void
+do_autolaunch(const char *path_arg)
+{
+ struct wl_array envp;
+ struct wl_array argv;
+ pid_t pid;
+ char **argvpp;
+ char *path;
+
+ path = xstrdup(path_arg);
+
+ parse_launcher_path(path, &envp, &argv);
+
+ /* panel_launcher_activate */
+
+ pid = fork();
+ if (pid < 0) {
+ fprintf(stderr, "fork failed: %m\n");
+ goto out;
+ }
+
+ if (pid)
+ goto out;
+
+ argvpp = argv.data;
+ if (execve(argvpp[0], argvpp, envp.data) < 0) {
+ fprintf(stderr, "execl '%s' failed: %m\n", argvpp[0]);
+ exit(1);
+ }
+
+ wl_array_release(&argv);
+ wl_array_release(&envp);
+ free(path);
+}
+
+static void
+process_autolaunch(struct desktop *desktop)
+{
+ struct weston_config_section *s;
+ char *path;
+ const char *name;
+
+ s = NULL;
+ while (weston_config_next_section(desktop->config, &s, &name)) {
+ if (strcmp(name, "autolaunch") != 0)
+ continue;
+
+ weston_config_section_get_string(s, "path", &path, NULL);
+ if (!path)
+ continue;
+
+ do_autolaunch(path);
+ free(path);
+ }
+}
+
int main(int argc, char *argv[])
{
struct desktop desktop = { 0 };
struct output *output;
struct weston_config_section *s;
desktop.unlock_task.run = unlock_dialog_finish;
wl_list_init(&desktop.outputs);
desktop.config = weston_config_parse("weston.ini");
@@ -1349,20 +1416,22 @@ int main(int argc, char *argv[])
/* Create panel and background for outputs processed before the shell
* global interface was processed */
wl_list_for_each(output, &desktop.outputs, link)
if (!output->panel)
output_init(output, &desktop);
grab_surface_create(&desktop);
signal(SIGCHLD, sigchild_handler);
+ process_autolaunch(&desktop);
+
display_run(desktop.display);
/* Cleanup */
grab_surface_destroy(&desktop);
desktop_destroy_outputs(&desktop);
if (desktop.unlock_dialog)
unlock_dialog_destroy(desktop.unlock_dialog);
desktop_shell_destroy(desktop.shell);
display_destroy(desktop.display);
diff --git a/man/weston.ini.man b/man/weston.ini.man
index c05a221..365141c 100644
--- a/man/weston.ini.man
+++ b/man/weston.ini.man
.RE
.PP
.PP
.RS 4
.nf
.BR "core " "The core modules"
.BR "libinput " "Input device configuration"
.BR "shell " "Desktop customization"
.BR "launcher " "Add launcher to the panel"
+.BR "autolaunch " "Launch program automatically on startup"
.BR "screensaver " "Screensaver selection"
.BR "output " "Output configuration"
.BR "input-method " "Onscreen keyboard input"
.BR "keyboard " "Keyboard layouts"
.BR "terminal " "Terminal application options"
.BR "xwayland " "XWayland options"
.BR "screen-share " "Screen sharing options"
.fi
.RE
.PP
@@ -272,20 +273,36 @@ sets the path to the program that is run by clicking on this launcher (string).
It is possible to pass arguments and environment variables to the program. For
.nf
.in +4n
path=GDK_BACKEND=wayland gnome-terminal --full-screen
.in
.fi
.PP
.RE
+.SH "AUTOLAUNCH SECTION"
+There can be multiple autolaunch sections for starting multiple programs.
+.TP 7
+.BI "path=" program
+sets the path (string) to the program that is run automatically when the
+desktop is starting.
+It is possible to pass arguments and environment variables to the program. For
+.nf
+.in +4n
+
+path=GDK_BACKEND=wayland gnome-terminal --full-screen
+.in
+.fi
+.PP
+.RE
.SH "SCREENSAVER SECTION"
The
.B screensaver
section is used to select and schedule a screensaver.
The
.B screensaver
section is optional, as are all of the entries that may be specified in
it.
.TP 7
.BI "path=" /usr/libexec/weston-screensaver
diff --git a/weston.ini.in b/weston.ini.in
index 4fca0bb..b0cb31f 100644
--- a/weston.ini.in
+++ b/weston.ini.in
@@ -30,20 +30,23 @@ icon=/usr/share/icons/gnome/24x24/apps/utilities-terminal.png
[launcher]
icon=/usr/share/icons/hicolor/24x24/apps/google-chrome.png
path=/usr/bin/google-chrome
[launcher]
icon=/usr/share/icons/gnome/24x24/apps/arts.png
+#[autolaunch]
+
[screensaver]
# Comment path to disable screensaver
duration=600
[input-method]
#[output]
#name=LVDS1
_______________________________________________
wayland-devel mailing list
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Frederic Plourde
2014-10-23 19:07:01 UTC
Permalink
_______________________________________________
wayland-devel mailing list
wayland-***@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Pekka Paalanen
2014-10-24 06:42:36 UTC
Permalink
On Thu, 23 Oct 2014 15:07:01 -0400
+1 for refactoring in a separate patch.
by merging the change with panel_add_launcher in the same patch, it gets *really* hard to read because of the intertwining.
Yeah, please split it.

Btw. it was Fred who actually sent the patch, but made a slight mistake
in trying to preserve my authorship in the process, so it ended
up looking like I sent the patch. :-P

I did write the original feature[1], but Fred has been cleaning it up
since. Credit where credit is due. :-)

Fred, since you're going to split the patch, you should take the
authorship of the parse_launcher_path patch, since I never did that.
Refactor first, new feature second - less code to review.
now, for exit(<int>) instead of using EXIT_FAILURE/SUCCESS, currently there is roughly 50 usages of exit(<int>) out of 100 total calls to exit() in Weston codebase.
So we can't really say one if more trendy than the other.
I'd suggest we leave that as is for now in this patch and open up another bug to change all of them separately someday.
Do make a difference between compositor vs. other programs' exit()
usage. There are only a few exit() calls in the compositor, and note
that the test suite also uses the Weston exit codes to relay test
success/skip/failure IIRC.

Fred has been writing a way to return an exit code from Weston also
with a normal shutdown, so that will interact here. I think a graceful
shutdown would be "better" than calling exit(), but that needs to be
verified. OTOH, calling exit() should not have any adverse effects like
leaving a VT unusable, but somehow I'm not quite convinced over all
possible cases (e.g. running weston directly as root for debugging on
DRM - or what about restoring KMS state?).

Oh, but the exit() call in this particular case is not Weston but the
forked process. So nevermind yet.
I'd prefer to see the refactor and the new feature in separate patches,
but this is pretty trivial.
I also have a slight preference for exit(EXIT_FAILURE), which is already
used somewhere else in that file - though there's also precedent for
exit(1), so you make the call. :)
I'd not seen printf's %m until today - do we want to depend on a gnuism?
I've seen at least some activity towards a freebsd port - I don't
believe %m is supported there?
Yeah, we already use that a lot, and I think we can continue adding
those, until someone comes and complains it doesn't work on
Android/BSD/Windows/whatever/...

Can do a mass change then.
That said, it runs nicely here and does what it says on the tin...
Process a new section 'autolaunch' from weston.ini, launching all
programs given there on desktop start-up.
[Frederic Plourde: cut redundancy between do_autolaunch and panel_add_launcher]
---
clients/desktop-shell.c | 97 ++++++++++++++++++++++++++++++++++++++++++-------
man/weston.ini.man | 17 +++++++++
weston.ini.in | 3 ++
3 files changed, 103 insertions(+), 14 deletions(-)
Thanks,
pq


[1] http://cgit.collabora.com/git/user/pq/weston.git/commit/?h=next&id=8a0e419fb33c3ab7d4036f25b0c33482e1059d45
Pekka Paalanen
2014-10-24 10:18:51 UTC
Permalink
This post might be inappropriate. Click to display it.
Bryce Harrington
2014-10-24 21:47:00 UTC
Permalink
Post by Pekka Paalanen
And, while you're at it - as this was written for kiosk mode, it spawns a
shell script which just restarts the video player in a loop. Can we please
add an autostart param to weston_client_run/start (as a new enum with
WESTON_CLIENT_RESTART, not bool) and to the config (as a bool) which lifts
most of
desktop_shell_client_destroy/check_desktop_shell_crash_too_early/respawn_desktop_shell_process?
I'm not sure that's feasible. Watching the process exit is not used to
trigger respawn for weston-desktop-shell, but losing the wl_client is.
These two events happen in arbitrary order, and we recently fixed a
related bug by exactly not respawning based on process exit. We want
weston-desktop-shell to respawn if the compositor disconnects it, even
if the actual process gets left behind due to malfunction.
Restart for autolaunch OTOH would be based on process exit rather than
losing the connection.
IOW, weston_client_start() and the existing respawn mechanism are
intended for special purpose known clients, while the panel
launchers and the autolaunch feature are meant for running arbitrary
programs (that might not even be clients themselves/directly or at all).
I'm not sure if adding restart to autolaunch is in scope... there are
many aspects to configure for restart (delays, when to give up)
so I'd rather maybe keep with the script. Or systemd user session.
After all, the autolaunch is just a quick'n'useful hack when no session
manager (systemd or other) is available.
Hmm, the several special cases / hacks here maybe suggests some
generalization is needed?

Also, for X, there is typically a .xinitrc, .gnomerc, or similar that's
executed as a shell script during system launch. Do we have plans for
adding something similar for weston? Like a .westonrc?

If not, perhaps this feature gives us a place for hanging startup
scripts?

(Not saying that shell scripts are the best way to do autostart of
applications, but it has been a tradition...)

Bryce
Pekka Paalanen
2014-10-27 08:10:25 UTC
Permalink
This post might be inappropriate. Click to display it.
Frederic Plourde
2014-10-27 17:10:15 UTC
Permalink
_______________________________________________
wayland-devel mailing list
wayland-***@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Daniel Stone
2014-10-28 16:46:24 UTC
Permalink
Hi,
Post by Pekka Paalanen
On Fri, 24 Oct 2014 10:28:57 +0100
Hmm. Can we please use weston_client_start here instead of open-coding
it?
weston_client_start() does not support passing in environment
explicitly.
Entirely fixable.
Post by Pekka Paalanen
It also automatically sets up WAYLAND_SOCKET environment
variable and creates a connection (wl_client) before the program even
starts.
I do not think it makes sense to use weston_client_start() here,
because whatever we launch here, might not be a single-shot Wayland
client. Especially in the script case mentioned below, it would
fail all restart attempts in the script as WAYLAND_SOCKET would be set
to a disconnected/invalid fd.
Well sure, but the script case is indeed something which just restarts in a
loop - which would be 100% unnecessary if autostart did that for us.
Allowing multiple autostart entries would eliminate another reason to write
scripts, in that you could launch multiple clients. And if you really,
really, really need it: unset WAYLAND_SOCKET? Obviously that would preclude
use of autorestart.

I'm not sure that's feasible. Watching the process exit is not used to
Post by Pekka Paalanen
trigger respawn for weston-desktop-shell, but losing the wl_client is.
These two events happen in arbitrary order, and we recently fixed a
related bug by exactly not respawning based on process exit. We want
weston-desktop-shell to respawn if the compositor disconnects it, even
if the actual process gets left behind due to malfunction.
Restart for autolaunch OTOH would be based on process exit rather than
losing the connection.
I can see where you're going with this, but again, this could be solved
with documentation: if you have a client which terminates the
WAYLAND_SOCKET before it's actually exited, either don't enable
autorestart, or handle it yourself.
Post by Pekka Paalanen
IOW, weston_client_start() and the existing respawn mechanism are
intended for special purpose known clients, while the panel
launchers and the autolaunch feature are meant for running arbitrary
programs (that might not even be clients themselves/directly or at all).
I'm not sure if adding restart to autolaunch is in scope... there are
many aspects to configure for restart (delays, when to give up)
so I'd rather maybe keep with the script. Or systemd user session.
After all, the autolaunch is just a quick'n'useful hack when no session
manager (systemd or other) is available.
Sure, but having everyone just code the same while true script is
necessarily that great either. Having Weston more usable OOTB as a kiosk
mode is a pretty good thing, and having it be more robust doubly so.

I mean, I don't disagree with you, it's just that I don't see these issues
as showstoppers - if people want to write random weird scripts or clients
which don't fit in with passing sockets or autorestart, then don't use
those features?

Cheers,
Daniel
Post by Pekka Paalanen
Thanks,
pq
Aside from that, and splitting refactor / autorestart code move / autorun
feature into separate patches, this looks good to me.
Cheers,
Daniel
Pekka Paalanen
2014-10-29 08:03:50 UTC
Permalink
On Tue, 28 Oct 2014 16:46:24 +0000
Post by Daniel Stone
Hi,
Post by Pekka Paalanen
On Fri, 24 Oct 2014 10:28:57 +0100
Hmm. Can we please use weston_client_start here instead of open-coding
it?
weston_client_start() does not support passing in environment
explicitly.
Entirely fixable.
Post by Pekka Paalanen
It also automatically sets up WAYLAND_SOCKET environment
variable and creates a connection (wl_client) before the program even
starts.
I do not think it makes sense to use weston_client_start() here,
because whatever we launch here, might not be a single-shot Wayland
client. Especially in the script case mentioned below, it would
fail all restart attempts in the script as WAYLAND_SOCKET would be set
to a disconnected/invalid fd.
Well sure, but the script case is indeed something which just restarts in a
loop - which would be 100% unnecessary if autostart did that for us.
Allowing multiple autostart entries would eliminate another reason to write
scripts, in that you could launch multiple clients. And if you really,
really, really need it: unset WAYLAND_SOCKET? Obviously that would preclude
use of autorestart.
I'm not sure that's feasible. Watching the process exit is not used to
Post by Pekka Paalanen
trigger respawn for weston-desktop-shell, but losing the wl_client is.
These two events happen in arbitrary order, and we recently fixed a
related bug by exactly not respawning based on process exit. We want
weston-desktop-shell to respawn if the compositor disconnects it, even
if the actual process gets left behind due to malfunction.
Restart for autolaunch OTOH would be based on process exit rather than
losing the connection.
I can see where you're going with this, but again, this could be solved
with documentation: if you have a client which terminates the
WAYLAND_SOCKET before it's actually exited, either don't enable
autorestart, or handle it yourself.
Post by Pekka Paalanen
IOW, weston_client_start() and the existing respawn mechanism are
intended for special purpose known clients, while the panel
launchers and the autolaunch feature are meant for running arbitrary
programs (that might not even be clients themselves/directly or at all).
I'm not sure if adding restart to autolaunch is in scope... there are
many aspects to configure for restart (delays, when to give up)
so I'd rather maybe keep with the script. Or systemd user session.
After all, the autolaunch is just a quick'n'useful hack when no session
manager (systemd or other) is available.
Sure, but having everyone just code the same while true script is
necessarily that great either. Having Weston more usable OOTB as a kiosk
mode is a pretty good thing, and having it be more robust doubly so.
I mean, I don't disagree with you, it's just that I don't see these issues
as showstoppers - if people want to write random weird scripts or clients
which don't fit in with passing sockets or autorestart, then don't use
those features?
I think we all forgot a tiny detail here.

weston_client_start() is a compositor internal function, while this
patch is modifying weston-desktop-shell client.

We would need to move the stuff into libshared, and make sure that
weston_client_start() can be implemented on top of the shared code.
Sharing the code that parses a command into a structure of environment,
executable and arguments is ok, but I think that is all there would be
shared. OTOH, the compositor side does not need to parse such commands
at all at the moment (but it could be a useful addition).

The compositor watches process exit with wl_event_loop's signalfd
handler, which is a libwayland-server feature.

In client side, there is no such thing as wl_client destruction. The
only common thing you can have is SIGCHLD to trigger respawn, which the
server side does not want. You'd have two different respawn triggers
even with shared respawn logic.

Then respawn needs a callback for respawning, so that the server code
can set up WAYLAND_SOCKET and then call back into libshared to
fork/exec the process, when libshared decides it needs to respawn
something, which in the server case needs the server to tell it to
respawn in the first place.

Is there anything worth sharing the code between the compositor and
weston-desktop-shell here?


Thanks,
pq
Daniel Stone
2014-10-29 20:01:33 UTC
Permalink
Hi,

On Wednesday, October 29, 2014, Pekka Paalanen <
Post by Pekka Paalanen
On Tue, 28 Oct 2014 16:46:24 +0000
<javascript:;>>
I think we all forgot a tiny detail here.
weston_client_start() is a compositor internal function, while this
patch is modifying weston-desktop-shell client.
Egads, I'd completely missed that, and assumed it was compositor rather
than shell.

Any reason to have it like that? Compositor seems more natural to me ...

-d
Pekka Paalanen
2014-10-30 08:06:45 UTC
Permalink
On Wed, 29 Oct 2014 20:01:33 +0000
Post by Daniel Stone
Hi,
On Wednesday, October 29, 2014, Pekka Paalanen <
Post by Pekka Paalanen
On Tue, 28 Oct 2014 16:46:24 +0000
<javascript:;>>
I think we all forgot a tiny detail here.
weston_client_start() is a compositor internal function, while this
patch is modifying weston-desktop-shell client.
Egads, I'd completely missed that, and assumed it was compositor rather
than shell.
Any reason to have it like that? Compositor seems more natural to me ...
The compositor could do the autolaunch, but we also have the panel
launch buttons, that need to be executed in weston-desktop-shell. And
autolaunch and the panel launchers are very similar, but not too similar
to the in-weston launching.

But I see what you might be getting at: if weston-desktop-shell does
autolaunch, and it respawns, it will autolaunch everything again. Not
good.

So we'd probably need autolaunch in the server side indeed. Then we
could put the command line parsing into env/cmd/args in libshared to
share that with weston-desktop-shell. Keep all the respawn logic inside
the compositor and maybe refactor that into some shared weston core
code.

Below I'll attach what I had written earlier but not sent, because
then I realized the server vs. weston-desktop-shell thing. If we do
autolaunch in the server, the below more or less applies again.

**

Okay, if Fred or someone actually has time to do all that refactoring
and writing, it's cool. It's just more work than simply adding
autolaunch, which was the original goal here.

So, we need to cater for two different cases:
a) start a client with a pre-made connection (set WAYLAND_SOCKET), and
b) start a new arbitrary process (do not set WAYLAND_SOCKET).

By refactoring, both cases should then have customizable environment,
and maybe optional respawn with conditions to give up if the process
exits too often.

Custom environment may come from weston.ini in case of manual
and auto-launchers, or hardcoded in plugins (currently not done).

Respawn needs the following parameters:
- is respawn triggered by wl_client destruction or SIGCHLD
- when to give up
(- delay to respawn?)

If I understood right, you suggested that respawn would be limited to
observing the wl_client, which practically means purely case a). That
should work, I think.

Manual launchers (the panel buttons) should never support respawn.

As a detail, case a) function will also need a hook that delivers the
new wl_client to the calling code when respawn happens. Otherwise e.g.
shell.c would not get the new authorized connection if
weston-desktop-shell crashes. We will also need a way to stop
respawning, e.g. shell.c uses that during shutdown when it deliberately
disconnects weston-desktop-shell.

When to give up?

The current logic for weston-desktop-shell is that if it crashes during
the first 30 seconds from session start, respawn will not be attempted,
and weston is shut down instead. Later, if weston-desktop-shell crashes
more than 5 times within 30 seconds, it gives up but does not exit
Weston, since other clients may still work.

**

Do we want to support autolaunching anything else than Wayland clients
directly? If not, then case b) can be dropped, which would be nice.

Can the weston-desktop-shell respawn/shutdown logic be supported by a
generic respawn code, or is it too complicated to be worth it? Maybe
e.g. weston-keyboard has a less complicated respawn logic that could
share with autolaunch.

Details... up to the person writing the code. We can then see how it
looks like. I would just want to avoid designing a big and complicated
launching and respawning framework just to cater the need of one
special case (weston-desktop-shell). If we can do with a simple design
otherwise but force weston-desktop-shell logic still be separate, that
is fine. Simplicity should be the driving goal in the code design. ;-)


Thanks,
pq
Frederic Plourde
2014-11-03 16:59:30 UTC
Permalink
Post by Pekka Paalanen
Below I'll attach what I had written earlier but not sent, because
then I realized the server vs. weston-desktop-shell thing. If we do
autolaunch in the server, the below more or less applies again.
**
Okay, if Fred or someone actually has time to do all that refactoring
and writing, it's cool. It's just more work than simply adding
autolaunch, which was the original goal here.
Yes, I definitely have time to tackle this as well.
We'll discuss it some more in a short while. thanks.
Post by Pekka Paalanen
a) start a client with a pre-made connection (set WAYLAND_SOCKET), and
b) start a new arbitrary process (do not set WAYLAND_SOCKET).
Thanks,
pq
_______________________________________________
wayland-devel mailing list
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Loading...