Subject: Re: [bug#42427] [PATCH] services: Fix auditd startup.

From: Ludovic Courtès <[email-masked]>
Date: Thu, 23 Jul 2020 00:07:28 +0200

Hello Robin,

Robin Green <[email-masked]> skribis:

> * gnu/services/auditd.scm: Make auditd start successfully in the default case.
> * gnu/services/aux-files/auditd/auditd.conf: New file.
> * doc/guix.texi (Miscellaneous Services): Update docs to reflect changes.

Nice, it’s a good idea.  Some comments below:

> -(define-configuration auditd-configuration
> -  (audit
> -   (package audit)
> -   "Audit package."))
> +(define-record-type* <auditd-configuration>

I think we should keep using ‘define-configuration’, unless there’s a
good reason to change.  WDYT?

> +  auditd-configuration make-auditd-configuration
> +  auditd-configuration?
> +  (audit           auditd-configuration-audit            ; package
> +                   (default audit))
> +  (configdir       auditd-configuration-configdir))      ; local-file

s/configdir/configuration-directory/, to be consistent with the rest of
the code.  You can also set its default value.

> +                  (auditd-configuration
> +                   (configdir (local-file "aux-files/auditd" #:recursive? #t))))))
> diff --git a/gnu/services/aux-files/auditd/auditd.conf b/gnu/services/aux-files/auditd/auditd.conf
> new file mode 100644
> index 0000000000..6e7555cf4c
> --- /dev/null
> +++ b/gnu/services/aux-files/auditd/auditd.conf

Since it’s a small file, I have a slight preference for using
‘plain-file’ + ‘computed-file’:

  (define auditd.conf
    (plain-file …))

  (define %default-auditd-configuration-directory ;make it public
    (computed-file "auditd"
                   #~(begin
                       (mkdir #$output)
                       (copy-file #$auditd.conf
                                  (string-append #$output "/auditd.conf")))))

WDYT?

Thanks,
Ludo’.