Re: [PATCH] services: Add opensmtpd-service.

[ludo@gnu.org (Ludovic Courtès)]

宋文武 <iyzsong@gmail.com> skribis:

> * gnu/services/mail.scm (<opensmtpd-configuration>): New record type.
> (%default-opensmtpd-config-file, %opensmtpd-accounts): New variables.
> (opensmtpd-shepherd-service, opensmtpd-activation): New procedures.
> (opensmtpd-service-type): New variable.
> (opensmtpd-service): New procedure.
> * doc/guix.texi (Mail Services): Document it.

Nice!

> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -9996,16 +9996,9 @@ For MariaDB, the root password is empty.
>  @cindex mail
>  @cindex email
>  The @code{(gnu services mail)} module provides Guix service definitions
> -for mail services.  Currently the only implemented service is Dovecot,
> -an IMAP, POP3, and LMTP server.
> +for following mail services:

I think we shouldn’t put a colon just before an @subsubheading.  How
about something like this instead:

  The @code{(gnu services mail)} module provides Guix service
  definitions for email services: IMAP, POP3, and LMTP servers, as well
  as mail transport agents (MTAs).  Lots of acronyms!  These services
  are detailed in the subsections below.

> +@subsubheading OpenSMTPD Service
> +
> +@deffn {Scheme Procedure} opensmtpd-service [#:config (opensmtpd-configuration)]
> +Return a service that runs the @command{smtpd} daemon of OpenSMTPD
> +package, an implementation of SMTP server as defined by RFC 5321.

s/OpenSMTP package/@uref{https://www.opensmtpd.org, OpenSMTPD}/

However, as discussed elsewhere, I prefer that we document
‘opensmtpd-service-type’ and not provide an ‘opensmtpd-service’
procedure at all.

> +(define opensmtpd-activation
> +  (match-lambda
> +    (($ <opensmtpd-configuration> package config-file)
> +     (let ((smtpd (file-append package "/sbin/smtpd")))
> +       #~(begin
> +           ;; Create the mbox directory.
> +           (mkdir-p "/var/mail")
> +           ;; Check the configuration file for validity.
> +           (system* #$smtpd "-n" "-f" #$config-file))))))

I think the configuration check belongs in the ‘start’ function of the
service rather than here.

Otherwise looks fine.

Could you send an updated patch?

Of course, bonus point if you can get a system test.  :-)

Thank you!

Ludo’.