diff mbox series

[bug#77501] Log OpenSMTPd to /var/log/mail.log via a configurable option. (Closes: #77501)

Message ID b00a02bb307e60e40478f87af7507fa05d019cb0.1744147946.git.felix.lechner@lease-up.com
State New
Headers
Series [bug#77501] Log OpenSMTPd to /var/log/mail.log via a configurable option. (Closes: #77501) |

Commit Message

Felix Lechner April 8, 2025, 9:32 p.m. UTC 
  Please also see this thread:

    https://lists.gnu.org/archive/html/help-guix/2025-04/msg00009.html

Change-Id: I485e040d680ccb39fa62e49d2e6ea916f047972c
---
Hi,

This was deployed briefly on production equipment and appears to work.

Kind regards
Felix

 doc/guix.texi         |  4 ++++
 gnu/services/mail.scm | 10 +++++++---
 2 files changed, 11 insertions(+), 3 deletions(-)


base-commit: c88f98bb3ca2616baea6b1e452cc900cc9c87503

Comments

Ludovic Courtès April 9, 2025, 9:22 p.m. UTC | #1 
Hi,

Felix Lechner <felix.lechner@lease-up.com> skribis:

> Please also see this thread:
>
>     https://lists.gnu.org/archive/html/help-guix/2025-04/msg00009.html
>
> Change-Id: I485e040d680ccb39fa62e49d2e6ea916f047972c

Could you add a conventional commit log?


[...]

>                      #~(make-forkexec-constructor
> -                       (list #$smtpd "-f" #$config-file)
> -                       #:pid-file "/var/run/smtpd.pid")))
> +                       (list #$smtpd
> +                             "-d"
> +                             "-f" #$config-file)
> +                       #:log-file #$log-file)))

I believe #:pid-file should be kept, no?

I would also keep the (list …) bit on a single line.

Thanks,
Ludo’.
Andreas Enge April 9, 2025, 9:58 p.m. UTC | #2 
Am Wed, Apr 09, 2025 at 11:22:58PM +0200 schrieb Ludovic Courtès:
> Could you add a conventional commit log?

This was there in my version of the patch a bit further up.

> >                      #~(make-forkexec-constructor
> > -                       (list #$smtpd "-f" #$config-file)
> > -                       #:pid-file "/var/run/smtpd.pid")))
> > +                       (list #$smtpd
> > +                             "-d"
> > +                             "-f" #$config-file)
> > +                       #:log-file #$log-file)))
> I believe #:pid-file should be kept, no?

This was dropped following my remark above:
"I have looked at the shepherd manual, and in
   https://www.gnu.org/software/shepherd/manual/shepherd.html#Service-De_002d-and-Constructors
it says the following about make-fork-exec-constructor:
'Note: This will not work as expected if the process “daemonizes”
(forks); in that case, you will need to pass #:pid-file, as explained
below.'
Now that you do not daemonize, is #:pid-file still needed or useful?"

The "-d" flag passed to #$smtpd prevents it from "daemonizing". So we
concluded that the #:pid-file thing was not needed (maybe it is needed,
but then the documentation should be adapted).

Andreas
Ludovic Courtès April 10, 2025, 10:24 a.m. UTC | #3 
Andreas Enge <andreas@enge.fr> skribis:

> The "-d" flag passed to #$smtpd prevents it from "daemonizing". So we
> concluded that the #:pid-file thing was not needed (maybe it is needed,
> but then the documentation should be adapted).

Oh I see, understood.

Thanks for explaining!

Ludo'.
Andreas Enge April 10, 2025, 1:41 p.m. UTC | #4 
I have added a commit message and a copyright header, turned the list
into one line and pushed.

Thanks to all!

Andreas
Maxim Cournoyer April 14, 2025, 12:52 a.m. UTC | #5 
Hi Andreas,

Ludovic Courtès <ludo@gnu.org> writes:

> Andreas Enge <andreas@enge.fr> skribis:
>
>> The "-d" flag passed to #$smtpd prevents it from "daemonizing". So we
>> concluded that the #:pid-file thing was not needed (maybe it is needed,
>> but then the documentation should be adapted).

From my understanding, a pid file is still useful for *synchronization*
reasons, whether the service daemonizes or not.  It's a token that lets
the service notify the init system that "OK, I've done my basic startup,
I'm ready to receive client requests".

That's been progressively replaced by socket activation in the systemd
world (and we support that via make-systemd-constructor, though I've
found some issue with it when using TLS connections -- to be reported
separately).
Andreas Enge April 14, 2025, 7:58 a.m. UTC | #6 
Hello Maxim,

Am Mon, Apr 14, 2025 at 09:52:53AM +0900 schrieb Maxim Cournoyer:
> From my understanding, a pid file is still useful for *synchronization*
> reasons, whether the service daemonizes or not.  It's a token that lets
> the service notify the init system that "OK, I've done my basic startup,
> I'm ready to receive client requests".

would you like to update the documentation and re-add the pid file
for this service here?

Andreas
diff mbox series

Patch

diff --git a/doc/guix.texi b/doc/guix.texi
index bee80cd4e2..c2640e5063 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -29219,6 +29219,10 @@  Mail Services
 users and daemons on the local machine, as well as permitting email to
 remote servers.  Run @command{man smtpd.conf} for more information.
 
+@item @code{log-file} (default: @code{"/var/log/mail.log"})
+The file location for the OpenSMTPD log file.  Logging occurs in the
+foreground via the Shepherd, i.e. OpenSMTPD does not detach.
+
 @item @code{setgid-commands?} (default: @code{#t})
 Make the following commands setgid to @code{smtpq} so they can be
 executed: @command{smtpctl}, @command{sendmail}, @command{send-mail},
diff --git a/gnu/services/mail.scm b/gnu/services/mail.scm
index ee61887369..cf23f76bc7 100644
--- a/gnu/services/mail.scm
+++ b/gnu/services/mail.scm
@@ -1719,6 +1719,8 @@  (define-record-type* <opensmtpd-configuration>
                         (default '())) ; list of symbols
   (config-file opensmtpd-configuration-config-file
                (default %default-opensmtpd-config-file))
+  (log-file opensmtpd-configuration-log-file
+            (default "/var/log/mail.log"))
   (setgid-commands? opensmtpd-setgid-commands? (default #t)))
 
 (define %default-opensmtpd-config-file
@@ -1734,15 +1736,17 @@  (define %default-opensmtpd-config-file
 
 (define (opensmtpd-shepherd-service config)
   (match-record config <opensmtpd-configuration>
-                       (package config-file shepherd-requirement)
+                       (package config-file log-file shepherd-requirement)
     (list (shepherd-service
            (provision '(smtpd))
            (requirement `(pam loopback ,@shepherd-requirement))
            (documentation "Run the OpenSMTPD daemon.")
            (start (let ((smtpd (file-append package "/sbin/smtpd")))
                     #~(make-forkexec-constructor
-                       (list #$smtpd "-f" #$config-file)
-                       #:pid-file "/var/run/smtpd.pid")))
+                       (list #$smtpd
+                             "-d"
+                             "-f" #$config-file)
+                       #:log-file #$log-file)))
            (stop #~(make-kill-destructor))))))
 
 (define %opensmtpd-accounts