<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">Ok, I split the patches and here's the
one doing the refactor, below.<br>
<br>
Dan : the other patch (the one adding autolaunch per se) still
needs a comment from you before I can update it. See below for the
conversation, I'd like to have your comments about using/not using
weston_client_start() and about adding an autostart param to it
and to the config. thanks :)<br>
*****************<br>
<br>
<pre wrap="">Some other desktop-shell bits will be (re)using most of panel_add_launcher
code, so this patch extracts useful bits and pieces into a new function,
parse_launcher_path.
Signed-off-by: Frederic Plourde <a class="moz-txt-link-rfc2396E" href="mailto:frederic.plourde@collabora.co.uk"><frederic.plourde@collabora.co.uk></a>
---
clients/desktop-shell.c | 38 ++++++++++++++++++++++++--------------
1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
index 961a9b2..c8e2810 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,
<div class="moz-txt-sig">--
1.9.1
</div></pre>
<div class="moz-signature">
<meta http-equiv="content-type" content="text/html;
charset=windows-1252">
<br>
<div class="moz-signature">
<meta http-equiv="CONTENT-TYPE" content="text/html;
charset=windows-1252">
<title></title>
<meta name="GENERATOR" content="LibreOffice 4.1-6 (Linux)">
<meta name="CREATED" content="0;0">
<meta name="CHANGEDBY" content="Frederic Plourde">
<meta name="CHANGED" content="20141022;112439857100097">
<style type="text/css">
<!--
P { color: #000000 }
BLOCKQUOTE { color: #000000 }
-->
</style>
<p style="margin-bottom: 0cm"><a
href="https://www.linkedin.com/in/fredericplourde"><img
alt="" src="cid:part2.09020506.02060906@collabora.co.uk"
height="15" width="15"></a><b> Frédéric Plourde</b></p>
<p><i>Principal Software engineer</i></p>
<blockquote> <b>T</b> :: (450) 415-0855<br>
<b>@</b>:: <a
href="mailto:frederic.plourde@collabora.co.uk">frederic.plourde@collabora.co.uk</a></blockquote>
<p style="line-height: 100%"><i>+++++++++++++</i></p>
<p style="line-height: 100%"><i>Open First ! We're hiring !</i></p>
<p style="line-height: 100%"><i>Our current opportunities can
be found here: <a href="http://bit.ly/Collabora-Careers">http://bit.ly/Collabora-Careers</a></i></p>
<p style="line-height: 100%"><i>Visit Collabora on the Web at
<a href="https://www.collabora.com/">https://www.collabora.com</a></i></p>
</div>
</div>
On 14-10-24 06:18 AM, Pekka Paalanen wrote:<br>
</div>
<blockquote
cite="mid:20141024131851.4506a623.pekka.paalanen@collabora.co.uk"
type="cite">
<pre wrap="">On Fri, 24 Oct 2014 10:28:57 +0100
Daniel Stone <a class="moz-txt-link-rfc2396E" href="mailto:daniel@fooishbar.org"><daniel@fooishbar.org></a> wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Hi,
On 22 October 2014 14:53, Pekka Paalanen <a class="moz-txt-link-rfc2396E" href="mailto:pekka.paalanen@collabora.co.uk"><pekka.paalanen@collabora.co.uk></a>
wrote:
</pre>
<blockquote type="cite">
<pre wrap="">+ 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);
+ }
</pre>
</blockquote>
<pre wrap="">
Hmm. Can we please use weston_client_start here instead of open-coding it?
</pre>
</blockquote>
<pre wrap="">
weston_client_start() does not support passing in environment
explicitly. 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.
</pre>
<blockquote type="cite">
<pre wrap="">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?
</pre>
</blockquote>
<pre wrap="">
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.
Thanks,
pq
</pre>
<blockquote type="cite">
<pre wrap="">Aside from that, and splitting refactor / autorestart code move / autorun
feature into separate patches, this looks good to me.
Cheers,
Daniel
</pre>
</blockquote>
<pre wrap="">
_______________________________________________
wayland-devel mailing list
<a class="moz-txt-link-abbreviated" href="mailto:wayland-devel@lists.freedesktop.org">wayland-devel@lists.freedesktop.org</a>
<a class="moz-txt-link-freetext" href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a>
</pre>
</blockquote>
<br>
</body>
</html>