bug#63082: [PATCH v3 07/16] services: mpd; Refactor start slot directory initialization.

[Bruno Victal]

On 2023-05-05 19:29, Maxim Cournoyer wrote:
> -                  (make-forkexec-constructor
> -                   (list #$(file-append package "/bin/mpd")
> -                         "--no-daemon"
> -                         #$config-file)
> -                   #:environment-variables '#$environment-variables)))
> +       (start
> +        (with-imported-modules (source-module-closure
> +                                '((gnu build activation)))

How about adding '(gnu build activation) into %default-imported-modules
(and %default-modules) at gnu/services/shepherd.scm?
Services should be using the start field to perform these kinds of tasks
anyways. (rather than extend activation-service-type which is incorrect use)

> +          #~(begin
> +              (use-modules (gnu build activation))

In general, rather than #~(begin (use-modules ...)), it's preferred to specify
additional modules using the 'modules' field e.g.

--8<---------------cut here---------------start------------->8---
(modules (cons '(gnu build activation)
                %default-modules))
--8<---------------cut here---------------end--------------->8---

> +
> +              (let ((user (getpw #$username)))
> +
> +                (define (init-directory directory)
> +                  (unless (file-exists? directory)
> +                    (mkdir-p/perms directory user #o755)))
> +
> +                (for-each
> +                 init-directory
> +                 '#$(map dirname
> +                         ;; XXX: Delete the potential "syslog"
> +                         ;; log-file value, which is not a directory.
> +                         (delete "syslog"
> +                                 (filter-map maybe-value
> +                                             (list db-file
> +                                                   log-file
> +                                                   state-file
> +                                                   sticker-file))))))

Perhaps treat “syslog” as a symbol instead?
Strings seem more adequate when the value is a path, with a symbol
being a sign that the value is to be treated “specially”.
(this aligns with how mympd handles this)


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

Cheers,
Bruno.