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

[iyzsong@member.fsf.org (宋文武)]

ludo@gnu.org (Ludovic Courtès) writes:

> 宋文武 <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.
>

Sure!

>> +@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.

Ah, OK, I was unaware of this.

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

Right…  I find it always reports ‘lo’ is not a valid interface here.

(Now I think it’s not necessary to do an addition check, since it will
fail to start anyway.)

> Otherwise looks fine.
>
> Could you send an updated patch?

Sure.


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

Will look into it later :-)

Also, I find that my slow box is unable start this opensmtpd service
during a fresh boot.  ‘smtpd’ has a timeout of 10s hardcoded, I patched
that to 60s to make it fully start.  But it turns out that
‘read-pid-file’ of shepherd has a timeout of 5s, so I guess that’s why
the service is marked as stop when it actually running…



Thanks for the review!