Hi, Bruno Victal <mi...@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
