Subject: [bug#54986] [PATCH 0/2] services: mpd: Refactor MPD service

From: Liliana Marie Prikler <[email-masked]>
Date: Thu, 08 Dec 2022 14:35:55 +0100

Am Donnerstag, dem 08.12.2022 um 13:11 +0000 schrieb mirai:
> On 2022-12-07 18:27, Liliana Marie Prikler wrote:
> > Note that there is [1], which attempts to make it so that shepherd
> > endpoints can be specified in lieu of MPD endpoints.  You don't
> > need to implement this logic – you are free to do so if you want to
> > – but could you make it so that there is an explicit endpoint
> > abstraction that would allow for extension later on?
> > 
> > Cheers
> > 
> > [1] https://issues.guix.gnu.org/54986
> 
> Hi,
> 
> After reading issue #54986, regarding mpd escaping shepherd
> management, my guess is that there could be two issues at play here:
> 
> * mpd.conf was serialized with pid_file [1]. The safest is
> to actually not serialize any unused directives and let MPD decide
> based on what's actually present. pid_file should only be set if
> MPD is to be launched as a "daemon process". [2]
> 
> * But the service definition for MPD is launched with '--no-daemon'
> option as seen in [3], which could be triggering a bug in MPD. It's
> probably worth testing the combinations of having pid_file set and
> invoking --no-daemon.
Nice catch, but completely unrelated to the issue.  In neither case do
we ever spawn MPD as a regular daemon, yet only in the systemd case
does shepherd have a problem with it.  That statement was concerning
shepherd's (mis)management of sockets, not MPD.

> > but could you make it so that there is an explicit endpoint
> > abstraction that would allow for extension later on?
> 
> If I'm understanding correctly what [1] is about, I think this can be
> implemented through the 'extend' interface. See how certbot adds a
> nginx-server-configuration to a nginx-service-type through the
> 'extend' interface. For MPD, this would amount to appending the
> 'adresses' field with "adequately formatted" values.
This doesn't work for #54986, which makes it so that in-file addresses
are ignored in favour of handing over the sockets directly through
shepherd.  Looking at [4], it appears the meaning of "port" is closer
to that of a default port, as addresses can have ports in them.  But I
would still prefer addresses to be "endpoints", which if they happen to
be a list of strings are taken as MPD addresses and if they happen to
be shepherd endpoints are passed on to the shepherd service.

WDYT?

[4] https://mpd.readthedocs.io/en/stable/user.html#client-connections