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

[Bruno Victal]

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.

I'd consider this field to be within the same category as 
'shepherd-requirement', it's for flexibility.


>  doc/guix.texi          |  4 ++--
>  gnu/services/audio.scm | 19 ++++++++++++++-----
>  2 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 550e6606e5..23f3070f39 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -33578,8 +33578,8 @@ Audio Services
>  This is a list of symbols naming Shepherd services that this service
>  will depend on.
>  
> -@item @code{environment-variables} (default: 
> @code{("PULSE_CLIENTCONFIG=/etc/pulse/client.conf" 
> "PULSE_CONFIG=/etc/pulse/daemon.conf")}) (type: list-of-strings)
> -A list of strings specifying environment variables.
> +@item @code{environment-variables} (default: @code{#f}) (type: boolean)
> +Obsolete.  Do not use.
>  
>  @item @code{log-file} (type: maybe-string)
>  The location of the log file.  Unless specified, the logs are collected
> diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
> index c11a7cfd26..f0587b9106 100644
> --- a/gnu/services/audio.scm
> +++ b/gnu/services/audio.scm
> @@ -236,6 +236,12 @@ (define (mpd-group-sanitizer value)
>      (warning (G_ "'group' in <mpd-configuration> is obsolete; ignoring~%")))
>    #f)
>  
> +(define (mpd-environment-variables-sanitizer value)
> +  (when value
> +    (warning (G_ "'environment-variables' in <mpd-configuration> is 
> obsolete;\
> + ignoring~%")))
> +  #f)
> +
>  (define (mpd-log-file-sanitizer value)
>    (match value
>      (%unset-value
> @@ -420,10 +426,10 @@ (define-configuration mpd-configuration
>     empty-serializer)
>  
>    (environment-variables
> -   (list-of-strings '("PULSE_CLIENTCONFIG=/etc/pulse/client.conf"
> -                      "PULSE_CONFIG=/etc/pulse/daemon.conf"))
> -   "A list of strings specifying environment variables."
> -   empty-serializer)
> +   (boolean #f)
> +   "Obsolete.  Do not use."
> +   (sanitizer mpd-environment-variables-sanitizer)
> +   (serializer empty-serializer))
>  
>    (log-file
>     maybe-string
> @@ -611,7 +617,10 @@ (define (mpd-shepherd-service config)
>                     (list #$(file-append package "/bin/mpd")
>                           "--no-daemon"
>                           #$config-file)
> -                   #:environment-variables '#$environment-variables
> +                   #:environment-variables
> +                   ;; Use the system-configured pulse configuration.
> +                   (list "PULSE_CLIENTCONFIG=/etc/pulse/client.conf"
> +                         "PULSE_CONFIG=/etc/pulse/daemon.conf")
>                     #:user #$username
>                     #:group #$(user-account-group user)
>                     #:supplementary-groups