Subject: bug#26306: exim service patches (including mail-aliases-service-type)

From: [email-masked] (Ludovic Courtès)
Date: Mon, 17 Apr 2017 23:27:18 +0200

Hi Carlo,

Did you have a chance to look into this?  Those patches are 90% there!
:-)

Ludo’.

[email-masked] (Ludovic Courtès) skribis:

> Hi Carlo,
>
> Carlo Zancanaro <[email-masked]> skribis:
>
>> I've wanted to fix up the exim service given Ludo's comments on my last
>> few patches (see #25789), so here are some patches to introduce a
>> mail-aliases-service-type, to change exim-service-type to use it
>> (instead of doing its own thing), and to add a system test for
>> exim-service-type.
>>
>> I haven't yet written the updates necessary to the documentation, but I
>> thought I'd send these now to get feedback before I do the work to fix
>> up the docs.
>
> This all LGTM.
>
> Minor comments:
>
>> From 11a5223f4a9487d3a9a17925488e18e80baec843 Mon Sep 17 00:00:00 2001
>> From: Carlo Zancanaro <[email-masked]>
>> Date: Thu, 30 Mar 2017 15:25:58 +1100
>> Subject: [PATCH 1/3] services: Add mail-aliases-service-type.
>>
>> * gnu/services/mail.scm (mail-aliases-etc): New procedure.
>> (mail-aliases-service-type): New variable.
>
> [...]
>
>> +(define (mail-aliases-etc aliases)
>> +  `(("aliases" ,(plain-file "aliases"
>> +                            ;; Ideally we'd use a format string like
>> +                            ;; "~:{~a: ~{~a~^,~}\n~}", but it gives a
>> +                            ;; warning that I can't figure out how to fix,
>> +                            ;; so we'll just use string-join below instead.
>> +                            (format #f "~:{~a: ~a\n~}"
>> +                                    (map (lambda (entry)
>> +                                           (list (car entry)
>> +                                                 (string-join (cdr entry) ",")))
>
> Please avoid car/cdr:
>
>   (match-lambda
>     ((user aliases ...)
>      (list user (string-joint aliases ","))))
>
>> +(define mail-aliases-service-type
>> +  (service-type
>> +   (name 'mail-aliases)
>> +   (extensions
>> +    (list (service-extension etc-service-type mail-aliases-etc)))
>> +   (compose concatenate)
>> +   (extend append)))
>
> If you add it to guix.texi (which would be nice) please leave a note as
> to what the values are (a list of user/addresses lists, right?).
>
>> From 8ac4f5fba3420ba5525cd7dff93d30f7fed8b0ae Mon Sep 17 00:00:00 2001
>> From: Carlo Zancanaro <[email-masked]>
>> Date: Thu, 30 Mar 2017 15:28:26 +1100
>> Subject: [PATCH 2/3] services: Make exim-service-type use
>>  mail-aliases-service-type
>>
>> * gnu/services/mail.scm (exim-configuration)[aliases]: Remove field.
>> (exim-activation, exim-shepherd-service): Remove alias from matches.
>> (exim-etc): Remove procedure.
>> (exim-service-type): Extend mail-aliases-service-type instead of
>> etc-service-type.
>
> OK.
>
>> From 984298f4cea4ac3bff530a4a767bf96567ec284f Mon Sep 17 00:00:00 2001
>> From: Carlo Zancanaro <[email-masked]>
>> Date: Thu, 30 Mar 2017 15:13:56 +1100
>> Subject: [PATCH 3/3] tests: mail: Add test for exim
>>
>> * gnu/tests/mail.scm (%exim-os, %test-exim): New variables.
>> (run-exim-test): New procedure.
>
> Nice test!
>
>> +(define %exim-os
>> +  (operating-system
>> +    (host-name "komputilo")
>
> Due to 892d9089a88abaa2ef1127f16308d03f4f08a4ce, this should use
> ‘simple-operating-system’.  Apologies!
>
> With a guix.texi update, it’ll be perfect.
>
> Could you send updated patches?
>
> Thanks!
>
> Ludo’.