First of all, thanks for the review.
Answers are embedded below.
On Sat, Jun 24, 2023 at 05:45:33PM +0100, Jonathan Wiltshire wrote:
Control: tag -1 moreinfo
On Thu, Jun 22, 2023 at 02:29:54PM +0200, Francesco P. Lovergine wrote:
diff -Nru proftpd-dfsg-1.3.8+dfsg/debian/changelog
proftpd-dfsg-1.3.8+dfsg/debian/changelog
--- proftpd-dfsg-1.3.8+dfsg/debian/changelog 2023-03-14 10:16:31.000000000
+0100
+++ proftpd-dfsg-1.3.8+dfsg/debian/changelog 2023-06-22 11:15:57.000000000
+0200
@@ -1,3 +1,15 @@
+proftpd-dfsg (1.3.8+dfsg-4+deb12u1) bookworm-proposed-updates; urgency=medium
You should target `bookworm`, not the admin suites.
Right, to be done.
diff -Nru proftpd-dfsg-1.3.8+dfsg/debian/proftpd-core.prerm
proftpd-dfsg-1.3.8+dfsg/debian/proftpd-core.prerm
--- proftpd-dfsg-1.3.8+dfsg/debian/proftpd-core.prerm 1970-01-01
01:00:00.000000000 +0100
+++ proftpd-dfsg-1.3.8+dfsg/debian/proftpd-core.prerm 2023-06-22
11:13:30.000000000 +0200
@@ -0,0 +1,11 @@
+#!/bin/sh
+
+set -e
+
+if [ -z "${DPKG_ROOT:-}" ] && [ "$1" = remove ] && [ -d /run/systemd/system ] ;
+then
+ deb-systemd-invoke stop 'proftpd.service' >/dev/null || true
+ deb-systemd-invoke stop 'proftpd.socket' >/dev/null || true
+fi
This gives rise to a race condition where the socket starts the service
again before the socket is stopped.
Well, this is exactly what debhelper does in current prerm in bookworm.
Eventually, it could be unified in `deb-systemd-invoke stop 'proftpd.socket' 'proftpd.service' || true`
like other packages do.
I'm not sure if this is what you intend, but if that risks a race condition it
would applies to
a lots of other packages.
diff -Nru proftpd-dfsg-1.3.8+dfsg/debian/proftpd-core.proftpd-run.service
proftpd-dfsg-1.3.8+dfsg/debian/proftpd-core.proftpd-run.service
--- proftpd-dfsg-1.3.8+dfsg/debian/proftpd-core.proftpd-run.service
1970-01-01 01:00:00.000000000 +0100
+++ proftpd-dfsg-1.3.8+dfsg/debian/proftpd-core.proftpd-run.service
2023-06-22 11:12:42.000000000 +0200
@@ -0,0 +1,14 @@
+[Unit]
+Description=ProFTPD FTP Server in standalone/socket mode
+Documentation=man:proftpd(8)
+OnFailure=proftpd.socket
+OnSuccess=proftpd.service
+
+[Service]
+Type=oneshot
+Environment=CONFIG_FILE=/etc/proftpd/proftpd.conf
+EnvironmentFile=-/etc/default/proftpd
+ExecStart=/usr/bin/grep -iqE '^[[:space:]]*ServerType[[:space:]]+standalone$'
$CONFIG_FILE
Maybe I missed something important, but this seems a very odd way of doing
things. Do you really set up a dummy service unit which is expected to fail
in standalone mode, and therefore starts the socket instead?
Why not use an ExecStartPre= or ExecCondition= in your normal units to
prevent starting when in inetd mode?
Unfortunately, Exec* directives can only be used in .service units. That's
the reason to enable an external oneshot .service unit to start alternatively
one of the two other units. Ideally one day or another such features could
be available also in other type of units (there is an issue open since 2019).
Incidentally, it is possible to add a ConditionPathExists and a something
like /etc/proftpd/proftpd_not_to_be_run (which is the trick used in sshd)
but would be completely Debian specific and out of the usual workflow
to manage inetd/standalone modes in proftpd. So, I'm not that keen on
this kind of trick.
--
Francesco P. Lovergine
