[bug#75571] gnu: opensmtpd: Fix fix for queuing of offline messages.

Message ID 819164cdd734cac9a531db6c3d0e940efdb579e3.1736905090.git.~@wolfsden.cz
State New
Headers
Series [bug#75571] gnu: opensmtpd: Fix fix for queuing of offline messages. |

Commit Message

Tomas Volf Jan. 15, 2025, 1:38 a.m. UTC
  The substituted path in smtpd.h was not used due to an #ifndef.  The correct
place to patch it seems to be mk/pathnames.  This sadly triggers a bootstrap,
so we need to add autoconf and automake to the native-inputs.

* gnu/packages/mail.scm (opensmtpd)[arguments]<#:phases>
{'patch-test-FHS-file-names}: Patch in mk/pathnames instead.
[native-inputs]: Add autoconf and automake.

Change-Id: I1d569b8aaae839d6fd4871ccb97c116e6930f1c9
---
 gnu/packages/mail.scm | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)
  

Comments

Felix Lechner Jan. 15, 2025, 4:17 a.m. UTC | #1
Hi Tomas,

Okay, maybe your patch is the right way but, aside from adding makemap,
is it all that superior?

Kind regards,
Felix

P.S. More power to OpenSMTPd!
  
Tomas Volf Jan. 15, 2025, 4:21 p.m. UTC | #2
Felix Lechner <felix.lechner@lease-up.com> writes:

> Hi Tomas,
>
> Okay, maybe your patch is the right way but, aside from adding makemap,
> is it all that superior?

Well the reason I even wrote this patch is that (at least for me) the
original fix no longer works.

Notice the path in the following snippet (the commit is current HEAD at
the time of writing this):

--8<---------------cut here---------------start------------->8---
$ strings $(guix time-machine -q --commit=b696658ee8e0655b17f5d26e024956b5148e36d6 -- build opensmtpd)/sbin/smtpd | grep smtpctl
/gnu/store/7p98k2p2d3zza41sv1npy3y90c280grc-opensmtpd-7.6.0p1/sbin/smtpctl
warn: smtpd: couldn't enqueue offline message %s; smtpctl %s
--8<---------------cut here---------------end--------------->8---

As you can see, it points into the store.

During debugging I noticed two issues:

1. The path in smtpd.h[0] is /usr/sbin/smtpctl, but the code looks for
/usr/bin/smtpctl (sbin vs bin).

2. The #define is guarded by #ifndef[1], so even if the path is
replaced, it is never used (due to [2]).

After my fix, the binary now contains the correct path:

--8<---------------cut here---------------start------------->8---
$ strings $(guix build opensmtpd)/sbin/smtpd | grep smtpctl
/run/privileged/bin/smtpctl
warn: smtpd: couldn't enqueue offline message %s; smtpctl %s
--8<---------------cut here---------------end--------------->8---

Does the current version work for you?  I do not think I am doing
anything weird in my configuration, but I was not able to pinpoint any
upstream change that would have effect on this, so I am unsure why the
original version stopped working.

Have a nice day,
Tomas

0: https://github.com/OpenSMTPD/OpenSMTPD/blob/master/usr.sbin/smtpd/smtpd.h#L81
1: https://github.com/OpenSMTPD/OpenSMTPD/blob/master/usr.sbin/smtpd/smtpd.h#L80
2: https://github.com/OpenSMTPD/OpenSMTPD/blob/master/mk/pathnames#L7
  
Felix Lechner Jan. 15, 2025, 6:58 p.m. UTC | #3
Hi Tomas,

On Wed, Jan 15 2025, Tomas Volf wrote:

> Does the current version work for you?

The path is also incorrect in my binary.  I'm not sure why I believe it
worked once.

The issue arises only intermittently, for example when smtpd fails to
start after a reboot as it sometimes does locally, and I send mail.

The mail queue on my server is presently empty, and I don't have time to
experiment.  Instead, I take your word for it and wholeheartedly support
the acceptance of this patch.  Thanks for working on it!

Kind regards
Felix

Cc: Andreas Enge, who may have committed my original change.
  
Andreas Enge Jan. 22, 2025, 2:28 p.m. UTC | #4
Am Wed, Jan 15, 2025 at 10:58:43AM -0800 schrieb Felix Lechner:
> The path is also incorrect in my binary.  I'm not sure why I believe it
> worked once.
> 
> The mail queue on my server is presently empty, and I don't have time to
> experiment.  Instead, I take your word for it and wholeheartedly support
> the acceptance of this patch.  Thanks for working on it!
> 
> Cc: Andreas Enge, who may have committed my original change.

Well, I sometimes do this kind of things without understanding them and
without remembering them afterwards...

The patch looks good on the surface of it, and since the two of you have
tested it, I have just pushed it. Thanks!

Andreas
  

Patch

diff --git a/gnu/packages/mail.scm b/gnu/packages/mail.scm
index 3744288fa3..5a16d670a3 100644
--- a/gnu/packages/mail.scm
+++ b/gnu/packages/mail.scm
@@ -3311,7 +3311,9 @@  (define-public opensmtpd
            libxcrypt
            zlib))
     (native-inputs
-     (list bison
+     (list autoconf
+           automake
+           bison
            groff                        ;for man pages
            pkg-config))
     (arguments
@@ -3336,10 +3338,13 @@  (define-public opensmtpd
          ;; Fix some incorrectly hard-coded external tool file names.
          (add-after 'unpack 'patch-FHS-file-names
            (lambda* (#:key inputs #:allow-other-keys)
-             ;; avoids warning smtpd: couldn't enqueue offline message
-             ;; smtpctl exited abnormally
-             (substitute* "usr.sbin/smtpd/smtpd.h"
-               (("/usr/bin/smtpctl") "/run/privileged/bin/smtpctl"))
+             (substitute* "mk/pathnames"
+               ;; avoids warning smtpd: couldn't enqueue offline message
+               ;; smtpctl exited abnormally
+               (("(-DPATH_SMTPCTL=).*\\\\" all def)
+                (string-append def "\\\"/run/privileged/bin/smtpctl\\\" \\"))
+               (("(-DPATH_MAKEMAP=).*\\\\" all def)
+                (string-append def "\\\"/run/privileged/bin/makemap\\\" \\")))
              (substitute* "usr.sbin/smtpd/smtpctl.c"
                ;; ‘gzcat’ is auto-detected at compile time, but ‘cat’ isn't.
                (("/bin/cat" file) (search-input-file inputs file)))