From mboxrd@z Thu Jan  1 00:00:00 1970
From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=)
Subject: Re: [PATCH] services: Add opensmtpd-service.
Date: Wed, 23 Nov 2016 23:24:27 +0100
Message-ID: <8760nd4yxw.fsf@gnu.org>
References: <20161119072920.17518-1-iyzsong@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Return-path: <guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org>
Received: from eggs.gnu.org ([2001:4830:134:3::10]:44466)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <ludo@gnu.org>) id 1c9fxz-0007DF-As
	for guix-devel@gnu.org; Wed, 23 Nov 2016 17:24:36 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <ludo@gnu.org>) id 1c9fxu-0002t4-DG
	for guix-devel@gnu.org; Wed, 23 Nov 2016 17:24:35 -0500
In-Reply-To: <20161119072920.17518-1-iyzsong@gmail.com> (=?utf-8?B?Ig==?=
	=?utf-8?B?5a6L5paH5q2mIidz?= message of
	"Sat, 19 Nov 2016 15:29:20 +0800")
List-Id: "Development of GNU Guix and the GNU System distribution."
	<guix-devel.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/guix-devel>,
	<mailto:guix-devel-request@gnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/archive/html/guix-devel/>
List-Post: <mailto:guix-devel@gnu.org>
List-Help: <mailto:guix-devel-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/guix-devel>,
	<mailto:guix-devel-request@gnu.org?subject=subscribe>
Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org
Sender: "Guix-devel" <guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org>
To: =?utf-8?B?5a6L5paH5q2m?= <iyzsong@gmail.com>
Cc: guix-devel@gnu.org

=E5=AE=8B=E6=96=87=E6=AD=A6 <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=E2=80=99t 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-configu=
ration)]
> +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
=E2=80=98opensmtpd-service-type=E2=80=99 and not provide an =E2=80=98opensm=
tpd-service=E2=80=99
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 =E2=80=98start=E2=80=99 func=
tion 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=E2=80=99.