Re: [PATCH 2/2] gnu: Add dovecot service

[Ludovic Courtès]

Andy Wingo <address@hidden> skribis:

> * gnu/services/mail.scm: New file.
> (dovecot-service, dovecot-configuration, dict-configuration)
> (passdb-configuration, userdb-configuration)
> (unix-listener-configuration, fifo-listener-configuration)
> (inet-listener-configuration, service-configuration)
> (protocol-configuration, plugin-configuration, mailbox-configuration)
> (namespace-configuration): New public variables.
>
> * gnu-system.am (GNU_SYSTEM_MODULES): Add (gnu services mail).
>
> * doc/guix.texi (Mail Services): New node.

Impressive piece of work!  Really nice.

Regarding the Schemefied configuration, do you think Dovecot’s
configuration model is stable enough that this won’t have to change
much?  Is a “cheat mode” (where one can pass raw strings to paste into
the configuration file) needed?

> +To add IMAP support to a GuixSD system, add a @code{dovecot-service} to

s|IMAP support|an IMAP/POP3 email server|

> address@hidden The following documentation was initially generated by
> address@hidden (generate-documentation) in (gnu services mail).  Manually 
> maintained
> address@hidden documentation is better, so we shouldn't hesitate to edit 
> below as
> address@hidden needed.  However if the change you want to make to this 
> documentation
> address@hidden can be done in an automated way, it's probably easier to change
> address@hidden (generate-documentation) than to make it below and have to 
> deal with
> address@hidden the churn as dovecot updates.

Makes sense to me!

> +(define-configuration unix-listener-configuration
> +  (path
> +   (file-name (error "unix listener path required"))
> +   "The file name on which to listen.")

s/path/file-name/ maybe

We should probably use a ‘dovecot-error’ procedure instead of ‘error’,
like:

  (define (dovecot-error message)
    (raise (condition (&message (message message))
                      (&dovecot-configuration-error))))

This would be better handled by the UI and possibly other users.  WDYT?

> +(define (validate-configuration config fields)
> +  (for-each (lambda (field)
> +              (let ((val ((configuration-field-getter field) config)))
> +                (unless ((configuration-field-predicate field) val)
> +                  (error "Invalid value for field:"
> +                         (configuration-field-name field) val))))
> +            fields))
> +
> +(define (validate-package field-name package)
> +  (unless (package? package)
> +    (error "not a package" package)))

Likewise here.

Otherwise LGTM.

Thank you!

Ludo’.