[PATCH] gnu: opensmtpd: Fix fix for queuing of offline messages.

  • Done
  • quality assurance status badge
Details
3 participants
  • Andreas Enge
  • Felix Lechner
  • Tomas Volf
Owner
unassigned
Submitted by
Tomas Volf
Severity
normal

Debbugs page

Tomas Volf wrote 2 months ago
(address . guix-patches@gnu.org)(name . Tomas Volf)(address . ~@wolfsden.cz)
819164cdd734cac9a531db6c3d0e940efdb579e3.1736905090.git.~@wolfsden.cz
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(-)

Toggle diff (35 lines)
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)))
--
2.47.1
Felix Lechner wrote 2 months ago
Queued messages in OpenSMTPd
(address . 75571@debbugs.gnu.org)(name . Tomas Volf)(address . ~@wolfsden.cz)
87frlk4so5.fsf@lease-up.com
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 wrote 2 months ago
(name . Felix Lechner)(address . felix.lechner@lease-up.com)(address . 75571@debbugs.gnu.org)
87ed14rqs8.fsf@wolfsden.cz
Felix Lechner <felix.lechner@lease-up.com> writes:

Toggle quote (5 lines)
> 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):

Toggle snippet (5 lines)
$ 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

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:

Toggle snippet (5 lines)
$ strings $(guix build opensmtpd)/sbin/smtpd | grep smtpctl
/run/privileged/bin/smtpctl
warn: smtpd: couldn't enqueue offline message %s; smtpctl %s

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


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

iQJCBAEBCgAsFiEEt4NJs4wUfTYpiGikL7/ufbZ/wakFAmeH4KcOHH5Ad29sZnNk
ZW4uY3oACgkQL7/ufbZ/walH0hAAi5PZiVxMFL5JiaXESFKL0RjZMUHHtyrFTkmm
cL44+0vRpOHVO29qe/VqzZD8+FW0CtMLA8xqk6bOQc/T5/qCAX8mrPHuMjNekWFD
t87eAdSDYiqetQBLN9ceSXHXhiSj7YSGmOP4JEsvymi62O5AEN96eoMZgX+RcpAk
Sf2SAfUvfoEEvbhYVYPdvPWsIX2jBeq/173YC1VBe8LT/lShTeRknr8wgzM2nSwf
gYJV/GfI/arsjC5IIP8f8pvf6moZ7X3/fVqQVmoRt0aUt8huGaZK0QC0qOxYdheE
2ba+zVVeZA8IVxRPqq2nerfpRW5y4mN1W70GdjVgeMSfXV5SPwbybrpHBtzRW4OL
GCuDu616Ha0aEVpL+dP0RGmVQB0ltGR3/vx6QnEVdr1gVCrrlQMgo/6Q52+/YCOr
r++yNiKtuDPa0HkJJBQoXdYbam2+PF7XOkOHfSZZISXAwmeY2dJhYvSJWtoVHcIV
GYl4EjTi90JqUq8OOo0di8Ap6j97ctRlfuMNLno4aYM6IqixFiEy3lgfouwzHS7v
nL1iEhwo2jtRiabeGLGplFyBKwXD8kDTAi4oQWkfsjxeGQKfuaxEFVvK9MNaRfVR
bT90Dv+k0VUD9dRQpo+E9fwkCQ+JZrBXdNrokTkeJQlvOAj6srGKna7rLamI5HSp
gudXJeU=
=2IfI
-----END PGP SIGNATURE-----

Felix Lechner wrote 2 months ago
(name . Tomas Volf)(address . ~@wolfsden.cz)
8734hjucnw.fsf@lease-up.com
Hi Tomas,

On Wed, Jan 15 2025, Tomas Volf wrote:

Toggle quote (2 lines)
> 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 wrote 2 months ago
(name . Felix Lechner)(address . felix.lechner@lease-up.com)
Z5EAiE-zMAe1UElk@jurong
Am Wed, Jan 15, 2025 at 10:58:43AM -0800 schrieb Felix Lechner:
Toggle quote (9 lines)
> 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
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 75571@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 75571
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help