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

From: Liliana Marie Prikler <[email-masked]>
Date: Fri, 09 Dec 2022 20:22:06 +0100

Am Freitag, dem 09.12.2022 um 13:44 +0000 schrieb mirai:
> On 2022-12-08 13:35, Liliana Marie Prikler wrote:
> > 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.
> 
> Are you proposing for the 'addresses' field to be a
> "maybe-list-of-string-or-shepherd-endpoint"? (more of a xor as they
> can't be used simultaneously)
> Example:
> 
> --8<---------------cut here---------------start------------->8---
> ;; should fire a error message during guix system reconfigure
> (mpd-configuration
>   (addresses `("[::]:6645"  
>                ,(shepherd-endpoint
>                   (address "/var/run/mpd-shepherd-socket")))))
> --8<---------------cut here---------------end--------------->8---
> 
> I don't think it breaks backward compatibility to introduce this
> after #59866 is merged.
> The type of field 'addresses' could be changed transparently to
> something like:
> 
> --8<---------------cut here---------------start------------->8---
> (define list-of-addresses (list-of (lambda (x) (or (string? x)
> (shepherd-endpoint? x)))))
> --8<---------------cut here---------------end--------------->8---
Something like that, but I don't think the vocabulary matches 1:1.  In
my opinion, an address is an endpoint – not a shepherd endpoint, but an
endpoint still – while a shepherd endpoint is not an address.  Thus, I
propose changing the vocabulary now to not break backwards
compatibility later.  IIUC, the change from the previous records to
define-configuration is already an API change, so it'd be good to have
both in the same series.

Cheers