[PATCH Weston 1/1] desktop-shell: implement autolaunch

[Pekka Paalanen]

On Tue, 28 Oct 2014 16:46:24 +0000
Daniel Stone <daniel at fooishbar.org> wrote:

> Hi,
> 
> On 24 October 2014 11:18, Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> wrote:
> 
> > On Fri, 24 Oct 2014 10:28:57 +0100
> > Daniel Stone <daniel at fooishbar.org> wrote:
> > >
> > > 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.
> 
> 
> > 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
> > 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.
> 
> 
> > 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