#77501 [PATCH] Restore logging of OpenSMTPd to /var/log/maillog.

Message #5 received at submit@debbugs.gnu.org (full text, mbox, reply):

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

To: guix-patches@gnu.org

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

Subject: [PATCH] Restore logging of OpenSMTPd to /var/log/maillog.

Date: Thu, 3 Apr 2025 11:04:28 -0700

Please see this thread for details:

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

Change-Id: I485e040d680ccb39fa62e49d2e6ea916f047972c
---
 gnu/services/mail.scm | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/gnu/services/mail.scm b/gnu/services/mail.scm
index b9a61163d5..5a627dbbaf 100644
--- a/gnu/services/mail.scm
+++ b/gnu/services/mail.scm
@@ -1742,8 +1742,11 @@ (define (opensmtpd-shepherd-service config)
            (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)
+                       #:pid-file "/var/run/smtpd.pid"
+                       #:log-file "/var/log/maillog")))
            (stop #~(make-kill-destructor))))))
 
 (define %opensmtpd-accounts

base-commit: 52ce48e754a77675da69e14d8f0dfa5fb76539ef
-- 
2.48.1

Message #8 received at 77501@debbugs.gnu.org (full text, mbox, reply):

From: Andreas Enge <andreas@enge.fr>

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

Cc: Gabriel Wicki <gabriel@erlikon.ch>, 77501@debbugs.gnu.org

Subject: Re: OpenSMTP logging

Date: Thu, 3 Apr 2025 20:55:45 +0200

Hello,

Am Thu, Apr 03, 2025 at 11:13:50AM -0700 schrieb Felix Lechner via:
> P.S. I linked to my Debbugs clone since Mumi did not pick it up.

this was probably just a question of time; I can see it now on issues.

> I can confirm that logging in OpenSMTPd is broken.  I fixed it locally
> with the patch in Bug#77501. [1]

This looks good to me as someone who knows very little about services...
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?

#:log-file "/var/log/maillog"
And is this the good file name? Not mail.log?

Andreas

Message #11 received at 77501@debbugs.gnu.org (full text, mbox, reply):

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

To: Andreas Enge <andreas@enge.fr>

Cc: Gabriel Wicki <gabriel@erlikon.ch>, 77501@debbugs.gnu.org

Subject: Re: OpenSMTP logging

Date: Thu, 03 Apr 2025 12:39:46 -0700

Hi Andreas,

On Thu, Apr 03 2025, Andreas Enge wrote:

> This looks good to me

Thanks for taking a look!

> is #:pid-file still needed or useful?

No, it isn't.  It also occurred to me after I had sent in the patch, and
moved onto something else.

As a side note, I'm not sure we have the luxury of making such minor
improvements.  Guix is already a system with a thousand warts.  My patch
definitely makes Guix better.

Either way, I'd be happy to remove it if we can reach an acceptance deal
per below.

> And is this the good file name?

It isn't, but I think /var/log/maillog has a long tradition with
Sendmail.

The filename should actually be configurable, but I won't submit such a
patch unless you promise to accept it.  I already have a gazillion
rebases because my patches, which I carry in my custom branch, are
almost never accepted.

Kind regards
Felix

Message #14 received at 77501@debbugs.gnu.org (full text, mbox, reply):

From: Andreas Enge <andreas@enge.fr>

To: 77501@debbugs.gnu.org

Cc: Andreas Enge <andreas@enge.fr>, felix.lechner@lease-up.com

Subject: [PATCH] services: opensmtpd: Add logging to a file.

Date: Tue, 8 Apr 2025 11:14:23 +0200

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

* gnu/services/mail.scm (opensmtpd-shepherd-service)[start]:
Add a command line flag to not daemonize. Drop #:pid-file.
Add #:log-file.

Change-Id: I485e040d680ccb39fa62e49d2e6ea916f047972c
Signed-off-by: Andreas Enge <andreas@enge.fr>
---
Hello,

so just to be sure before I push, the patch below works for you?

Andreas


 gnu/services/mail.scm | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gnu/services/mail.scm b/gnu/services/mail.scm
index b9a61163d5..0d5af95986 100644
--- a/gnu/services/mail.scm
+++ b/gnu/services/mail.scm
@@ -1742,8 +1742,10 @@ (define (opensmtpd-shepherd-service config)
            (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 "/var/log/maillog")))
            (stop #~(make-kill-destructor))))))
 
 (define %opensmtpd-accounts

base-commit: 43d30332bf3b4de0003578a7b5b27472b11183c7
-- 
2.49.0

Message #17 received at 77501@debbugs.gnu.org (full text, mbox, reply):

From: Tomas Volf <~@wolfsden.cz>

To: Andreas Enge <andreas@enge.fr>

Cc: 77501@debbugs.gnu.org, felix.lechner@lease-up.com

Subject: Re: [bug#77501] [PATCH] services: opensmtpd: Add logging to a file.

Date: Tue, 08 Apr 2025 20:30:26 +0200

Andreas Enge <andreas@enge.fr> writes:

> diff --git a/gnu/services/mail.scm b/gnu/services/mail.scm
> index b9a61163d5..0d5af95986 100644
> --- a/gnu/services/mail.scm
> +++ b/gnu/services/mail.scm
> @@ -1742,8 +1742,10 @@ (define (opensmtpd-shepherd-service config)
>             (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 "/var/log/maillog")))

Probably out of scope but would be cool to have this configurable, I
personally prefer /var/log/mail.log.  I assume you have enough on your
hands already, so will send a separate patch later.

Tomas

-- 
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

Message #20 received at 77501@debbugs.gnu.org (full text, mbox, reply):

From: Andreas Enge <andreas@enge.fr>

To: Tomas Volf <~@wolfsden.cz>

Cc: 77501@debbugs.gnu.org, felix.lechner@lease-up.com

Subject: Re: [bug#77501] [PATCH] services: opensmtpd: Add logging to a file.

Date: Tue, 8 Apr 2025 20:41:24 +0200

Am Tue, Apr 08, 2025 at 08:30:26PM +0200 schrieb Tomas Volf:
> Probably out of scope but would be cool to have this configurable, I
> personally prefer /var/log/mail.log.  I assume you have enough on your
> hands already, so will send a separate patch later.

Sounds good, thanks! You can already start the bikeshedding discussion
about what the default value should be :)

Andreas

Message #23 received at 77501@debbugs.gnu.org (full text, mbox, reply):

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

To: 77501@debbugs.gnu.org

Cc: Tomas Volf <~@wolfsden.cz>, Gabriel Wicki <gabriel@erlikon.ch>, Andreas Enge <andreas@enge.fr>

Subject: Re: [bug#77501] [PATCH] services: opensmtpd: Add logging to a file.

Date: Tue, 08 Apr 2025 11:52:34 -0700

Hi Andreas & Tomas,

On Tue, Apr 08 2025, Andreas Enge wrote:

> You can already start the bikeshedding

No bikeshedding needed!  Given the emerging consensus, which includes a
senior committer, I will happily prepare a patch that makes the name of
the log file configurable.

Like both of you, I also find mail.log more agreeable.

Anyway, it's easy to change that default later, but it would be harder
to change the name of the option, which is part of the API.

Is "log-file" okay?

Kind regards
Felix

Message #26 received at 77501@debbugs.gnu.org (full text, mbox, reply):

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

To: 77501@debbugs.gnu.org

Cc: Tomas Volf <~@wolfsden.cz>, Gabriel Wicki <gabriel@erlikon.ch>, Andreas Enge <andreas@enge.fr>, Felix Lechner <felix.lechner@lease-up.com>

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

Date: Tue, 8 Apr 2025 14:32:27 -0700

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(-)

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

base-commit: c88f98bb3ca2616baea6b1e452cc900cc9c87503
-- 
2.49.0

Message #29 received at 77501@debbugs.gnu.org (full text, mbox, reply):

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

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

Cc: Tomas Volf <~@wolfsden.cz>, Gabriel Wicki <gabriel@erlikon.ch>, Maxim Cournoyer <maxim.cournoyer@gmail.com>, 77501@debbugs.gnu.org, Andreas Enge <andreas@enge.fr>

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

Date: Wed, 09 Apr 2025 23:22:58 +0200

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’.

Message #32 received at 77501@debbugs.gnu.org (full text, mbox, reply):

From: Andreas Enge <andreas@enge.fr>

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

Cc: Tomas Volf <~@wolfsden.cz>, Gabriel Wicki <gabriel@erlikon.ch>, Maxim Cournoyer <maxim.cournoyer@gmail.com>, 77501@debbugs.gnu.org, Felix Lechner <felix.lechner@lease-up.com>

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

Date: Wed, 9 Apr 2025 23:58:22 +0200

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

Message #35 received at 77501@debbugs.gnu.org (full text, mbox, reply):

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

To: Andreas Enge <andreas@enge.fr>

Cc: Tomas Volf <~@wolfsden.cz>, Gabriel Wicki <gabriel@erlikon.ch>, Maxim Cournoyer <maxim.cournoyer@gmail.com>, 77501@debbugs.gnu.org, Felix Lechner <felix.lechner@lease-up.com>

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

Date: Thu, 10 Apr 2025 12:24:24 +0200

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'.

Message #40 received at 77501-done@debbugs.gnu.org (full text, mbox, reply):

From: Andreas Enge <andreas@enge.fr>

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

Cc: Tomas Volf <~@wolfsden.cz>, Gabriel Wicki <gabriel@erlikon.ch>, Maxim Cournoyer <maxim.cournoyer@gmail.com>, Felix Lechner <felix.lechner@lease-up.com>, 77501-done@debbugs.gnu.org

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

Date: Thu, 10 Apr 2025 15:41:52 +0200

I have added a commit message and a copyright header, turned the list
into one line and pushed.

Thanks to all!

Andreas

Message #43 received at 77501@debbugs.gnu.org (full text, mbox, reply):

From: Maxim Cournoyer <maxim.cournoyer@gmail.com>

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

Cc: Tomas Volf <~@wolfsden.cz>, Andreas Enge <andreas@enge.fr>, Gabriel Wicki <gabriel@erlikon.ch>, 77501@debbugs.gnu.org, Felix Lechner <felix.lechner@lease-up.com>

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

Date: Mon, 14 Apr 2025 09:52:53 +0900

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).

-- 
Thanks,
Maxim

Message #46 received at 77501@debbugs.gnu.org (full text, mbox, reply):

From: Andreas Enge <andreas@enge.fr>

To: Maxim Cournoyer <maxim.cournoyer@gmail.com>

Cc: Tomas Volf <~@wolfsden.cz>, Ludovic Courtès <ludo@gnu.org>, Gabriel Wicki <gabriel@erlikon.ch>, 77501@debbugs.gnu.org, Felix Lechner <felix.lechner@lease-up.com>

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

Date: Mon, 14 Apr 2025 09:58:26 +0200

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

Send a report that this bug log contains spam.