bug#63082: mpd defaul configuration does not work ('No database' error)

[Bruno Victal]

Hi Maxim,

On 2023-07-25 21:48, Maxim Cournoyer wrote:
> Hi Bruno,
> 
> Bruno Victal <mirai@makinata.eu> writes:
> 
>> Hi Maxim,
>>
>> On 2023-05-05 19:28, Maxim Cournoyer wrote:
>>> * gnu/services/audio.scm (mpd-shepherd-service): Register a new update 
>>> action.
>>> * doc/guix.texi (Audio Services): Document it.
>>> ---
>>>  doc/guix.texi          | 10 ++++++++++
>>>  gnu/services/audio.scm | 11 +++++++++++
>>>  2 files changed, 21 insertions(+)
>>>
>>
>> I've been looking at this part for the past few weeks in attempt to
>> make it more robust and after countless hours, I'd advise against this
>> (in its current form), reason being that this only works if your
>> configuration happens to match the default values used by mpc.
>>
>> My attempts at getting the values from the configuration into
>> something that mpc understands have been unsuccessful. Not only the
>> decision “logic” of what values to pass is non-trivial, parsing the
>> endpoints field has been so far a complete nightmare. (with interesting
>> gems like IPv6 address formats that the daemon is happy to use yet
>> mpc will reject)
>>
>> Having the proper hostname (and port) intelligently deduced from
>> the endpoints field is a big minefield that is likely to end in
>> unmaintainable spaghetti.
>>
>> Short of introducing additional fields like “internal-mpc-host” and
>> “internal-mpc-port”, you could modify this to relay the
>> 'environment-variables' field for mpc as well. (since it can make use
>> of the MPD_HOST and MPD_PORT varibles if present)
> 
> Apologies if it's been a couple weeks, but was the above comment really
> meant for patch 02/16 of this series ("services: mpd: Add an 'update'
> action to trigger a database update.") ?  I don't see how they relate.

Yes, since this action uses 'mpc' to issue the 'update' command and
the way it works is that mpc is using a 'default' value to connect to MPD. 

The 'default' works until you change the 'endpoints' to something that
_doesn't_ match the defaults that 'mpc' uses (e.g. [fe80::1234%eno1],
/var/run/mpd/not-your-expected-name.socket, 2001:db8::1, etc.)

Since the 'endpoints' field is equivalent to MPDs 'bind_to_address'
directive [1], you have to take into account the flexible formats it allows
and the 'port' field as well. This makes it somewhat non-trivial to
inspect the 'endpoints' and 'port' fields to select an address for the
shepherd 'update' action to use. You also need to take into account that
you have to pass the address and port separately (if needed) for 'mpc'.

One additional avenue (from the ones originally listed) we could explore
is to make the endpoints field no longer a maybe-type. We set a default
value (that is aligned with upstream, like localhost:6600) and also set
an “internal” unix socket to be used primarily by shepherd.

[1]: <https://mpd.readthedocs.io/en/latest/user.html#listeners>


-- 
Furthermore, I consider that nonfree software must be eradicated.

Cheers,
Bruno.