Hi Maxim, On 2023-05-03 02:44, Maxim Cournoyer wrote: > Hi Bruno, > > Bruno Victal <[email protected]> writes: > >> On 2023-04-29 18:04, Maxim Cournoyer wrote: >>> Hi Bruno, >>> >>> Bruno Victal <[email protected]> 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
