bug#63082: [PATCH 14/17] services: mpd: Obsolete 'environment-variables' field.

[Bruno Victal]

Hi Maxim,

On 2023-05-03 02:44, Maxim Cournoyer wrote:
> Hi Bruno,
> 
> Bruno Victal <mirai@makinata.eu> writes:
> 
>> On 2023-04-29 18:04, Maxim Cournoyer wrote:
>>> Hi Bruno,
>>>
>>> Bruno Victal <mirai@makinata.eu> writes:
>>>
>>>> On 2023-04-28 15:27, Maxim Cournoyer wrote:
>>>>> Rationale: Services can be extended via the simple-service mechanism 
>>>>> instead
>>>>> of having to expose fields on service configurations that are not directly
>>>>> connected to the service's configuration.
>>>>>
>>>>> * gnu/services/audio.scm (mpd-environment-variables-sanitizer): New 
>>>>> sanitizer.
>>>>> (mpd-configuration): Use it.
>>>>> (mpd-shepherd-service): Hard code the useful environment variables inside 
>>>>> the
>>>>> Shepherd service.
>>>>> ---
>>>>
>>>> This field shouldn't be deprecated as one of it's primary purposes is to 
>>>> allow for
>>>> the pulseaudio daemon configuration to be set to another one.
>>>> What you're doing here is effectively hardcoding the pulseaudio 
>>>> configuration.
>>>
>>> Our only means to declare a pulseaudio configuration
>>> (pulseaudio-service-type) places it at this location, so it seems
>>> reasonable to hard code it.  What use case do you have for a custom
>>> pulseaudio configuration that pulseaudio-service-type could not cater
>>> to?  This prevents users defining another environment variable and
>>> forgetting to replace these, then wondering why the default pulse
>>> configuration doesn't work, and it felt out of place to me (an
>>> implementation detail better encapsulated).
>>
>> Indeed but note that there's a small subtlety to pulseaudio-service-type, 
>> chiefly that
>> the service is not your typical ¿monodaemonic? process that is used 
>> throughout the system,
>> rather it simply provides you a default config for the pulseaudio daemon.
>> The fact that multiple pulseaudio daemons can be launched alongside is a 
>> strong indicator that
>> perhaps you will want for some of them to use different configurations, 
>> which is done via the
>> environment variables.
>>
>> Right now this would be mainly achieved using local-file, text-file or 
>> specifying a path
>> but in theory the procedures for pulseaudio-service-type could be reused for 
>> serializing
>> configurations to be used outside of the service.
>>
>> Regarding the users forgetting the variables, it looks obvious that if you 
>> omit the default
>> values there then the behavior will also change accordingly.
>> If you strongly feel that this is very pitfall prone (IMO it's no worse than 
>> forgetting to add %base-services
>> at the end of the services field) then perhaps documenting it would suffice?
> 
> Is this a use case in actual use?  It seems a bit of a stretch in my
> mind, especially considering the service was already difficult to get
> working in its default configuration; I doubt someone would go out of
> their way to manage multiple distinctly configured pulseaudio daemons
> :-).  But if it's something in actual use providing value, I don't mind
> to keep it until we have a better way to extend a common set of basic
> properties for services in general.

I added this field because while I was refactoring this in #59866 I cleared 
#:environment-variables
from the service constructor as they were setting “wrong variables” (stemming 
from the abuse of the service
as a home service) and since I was mostly interested on the network (http) 
outputs which didn't require any
variables set I didn't notice any issues until I learned about PulseAudio's rtp 
modules and tried using them.

With this, I realized that the environment variables should be adjustable in 
order for the service to be flexible
enough for general usage. (in general I prefer the services to be less 
opinionated)

Though originally thought towards multiple PulseAudio configurations, I should 
stress that PulseAudio
(and PipeWire when we get there) understand a plethora of environment variables:

* 
<https://www.freedesktop.org/wiki/Software/PulseAudio/FAQ/#whatenvironmentvariablesdoespulseaudiocareabout>
* <https://docs.pipewire.org/page_daemon.html>


Cheers,
Bruno