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

[Maxim Cournoyer <maxim.cournoyer@gmail.com>]

Hi,

Bruno Victal <mirai@makinata.eu> writes:

> 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)

I think that's something to discuss outside the scope of this series,
which is already a bit unwieldy :-).  I think originally they were put
there because adding them to (guix build utils) would entail a world
rebuild.

>> +          #~(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.

I prefer to keep it the way it is, because the use-modules works in
tandem with the with-imported-modules.  I'd also have to wrap the stop
procedure in a second with-imported-modules if I used the 'global'
modules field, since it applies to both.

> (modules (cons '(gnu build activation)
>                 %default-modules))
>
>> +
>> +              (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)

There no longer is a delete "syslog" in the code, reworked in "services:
mpd: Log to syslog by default."

-- 
Thanks,
Maxim