On Thu, Aug 18, 2022 at 07:56:27PM +0200, Ansgar wrote:
> 1. Creating files in non-persistent locations (/run) and cleanup (at boot
> and via cron job):
> 
> One example is the case above.
> 
> The dependency on "systemd | systemd-tmpfiles" does not cause tmpfiles to be
> processed during boot or normal system operation with sysvinit-core as the
> systemd-standalone-tmpfiles package ships no init script or cron job taking
> over the function of systemd-tmpfiles-setup.service and
> systemd-tmpfiles-clean.timer.
> 
> I expect that such tmpfiles could mostly be ignored in init-less
> environments such as containers as these usually don't expect cleanup
> actions for boot and service startup is handled differently.
> 
> With one exception this could be covered by mandating that init systems must
> run tmpfiles during boot and packages do not need an explicit dependency for
> this to happen.
> 
> (Why mandate the init system to do so? Because I do not think we want to
> require passwd and other packages to ship an init script just to call
> systemd-tmpfiles or reimplement the equivalent behavior in a different way.)
> 
> However there is also the issue that tmpfiles should probably be called on
> package installation to create the requested files below /run before the
> next system reboot; they might be relevant for the service to start. It
> might be sufficient to only call systemd-tmpfiles if some init system is
> used (i.e., not in the init-less container case) and so also be covered by
> the requirement above (it would have to include some systemd-tmpfiles binary
> to be available).
> 
> Some tmpfiles also include only exception rules for cleanup, for example
> gvfsd-fuse-tmpfiles.conf:
> 
>     x /run/user/*/gvfs
> 
> This case does not need an explicit dependency either.

I just had my attention drawn to this bug because a man-db upload
unexpectedly (to me) gained a dependency on systemd | systemd-tmpfiles,
thus adding to the build-essential set.  /usr/lib/tmpfiles.d/man-db.conf
reads:

  d /var/cache/man 0755 man man 1w

This line exists only for cleanup purposes; the directory is also
shipped in the package with matching ownership and permissions, so
man-db clearly doesn't rely on tmpfiles for creating that directory.

Now, for systems where we have a tmpfiles implementation, this is
definitely simpler than the cron equivalent (which takes extra care to
be polite about I/O, but with tmpfiles I assume that's the
implementation's problem, and indeed systemd-tmpfiles.service has
IOSchedulingClass=idle):

  # expunge old catman pages which have not been read in a week
  if [ -d /var/cache/man ]; then
    cd /
    start-stop-daemon --start --pidfile /dev/null --startas /bin/sh \
          --oknodo --chuid man $iosched_idle -- -c \
          "find /var/cache/man -type f -name '*.gz' -atime +6 -print0 | \
           xargs -r0 rm -f"
  fi

But currently, man-db's cron.daily job starts with a [ -d
/run/systemd/system ] guard, so it'll run on systems not booted with
systemd; and in any case man-db has no dependency on cron, so this
cleanup work is supposed to be optional either way.  As such, I'd
definitely prefer to avoid this dependency.

Perhaps we could have this new behaviour be controlled by a flag to
dh_installtmpfiles, since we have good arguments for different packages
wanting different behaviour?  For compatibility, it would seem most
sensible for the old behaviour to be the default, and for packages to
opt into the new behaviour using something like "dh_installtmpfiles
--require-tmpfiles".

I didn't see anyone suggesting a patch for this bug as yet, so to get
the ball rolling, how about the following untested patch?  I doubt it
solves all the use cases raised in this thread, but it still permits the
roundcube maintainer to deal with the situation described in #1013969
with a simple debhelper override, it avoids introducing many unplanned
dependencies elsewhere, and it doesn't seem like unreasonable extra
complexity in debhelper.  People interested in improving how the
tmpfiles system works on non-systemd init systems can still do that
separately.

diff --git a/autoscripts/postinst-init-tmpfiles 
b/autoscripts/postinst-init-tmpfiles
index d651e09c..2430095b 100644
--- a/autoscripts/postinst-init-tmpfiles
+++ b/autoscripts/postinst-init-tmpfiles
@@ -1,5 +1,7 @@
 if [ "$1" = "configure" ] || [ "$1" = "abort-upgrade" ] || [ "$1" = 
"abort-deconfigure" ] || [ "$1" = "abort-remove" ] ; then
-       if [ -z "${DPKG_ROOT:-}" ] ; then
+       # In case this system is running systemd, we need to ensure that all
+       # necessary tmpfiles (if any) are created before starting.
+       if [ -z "${DPKG_ROOT:-}" ] && [ -d /run/systemd/system ] ; then
                systemd-tmpfiles --create #TMPFILES# >/dev/null || true
        fi
 fi
diff --git a/autoscripts/postinst-init-tmpfiles-required 
b/autoscripts/postinst-init-tmpfiles-required
new file mode 100644
index 00000000..d651e09c
--- /dev/null
+++ b/autoscripts/postinst-init-tmpfiles-required
@@ -0,0 +1,5 @@
+if [ "$1" = "configure" ] || [ "$1" = "abort-upgrade" ] || [ "$1" = 
"abort-deconfigure" ] || [ "$1" = "abort-remove" ] ; then
+       if [ -z "${DPKG_ROOT:-}" ] ; then
+               systemd-tmpfiles --create #TMPFILES# >/dev/null || true
+       fi
+fi
diff --git a/dh_installinit b/dh_installinit
index cc63f58b..d69f73c2 100755
--- a/dh_installinit
+++ b/dh_installinit
@@ -15,7 +15,7 @@ our $VERSION = DH_BUILTIN_VERSION;
 
 =head1 SYNOPSIS
 
-B<dh_installinit> [S<I<debhelper options>>] [B<--name=>I<name>] [B<-n>] 
[B<-R>] [B<-r>] [B<-d>] [S<B<--> I<params>>]
+B<dh_installinit> [S<I<debhelper options>>] [B<--name=>I<name>] [B<-n>] 
[B<-R>] [B<-r>] [B<-d>] [B<--require-tmpfiles>] [S<B<--> I<params>>]
 
 =head1 DESCRIPTION
 
@@ -199,6 +199,14 @@ Call the named shell I<function> if running the init 
script fails. The
 function should be provided in the F<prerm> and F<postinst> scripts, before the
 B<#DEBHELPER#> token.
 
+=item B<--require-tmpfiles>
+
+This option ensures that a tmpfiles implementation is always available, so
+that the package can assume that any tmpfiles.d configuration files are
+processed. This is appropriate for cases where creating the declared files
+is necessary for ordinary operation of the package. Only used in compat
+levels 10 and below.
+
 =back
 
 =head1 NOTES
@@ -211,6 +219,7 @@ instances of the same text to be added to maintainer 
scripts.
 
 $dh{RESTART_AFTER_UPGRADE} = '';
 $dh{NO_START} = '';
+$dh{REQUIRE_TMPFILES} = '';
 
 init(options => {
        "r" => \$dh{R_FLAG},
@@ -226,6 +235,7 @@ init(options => {
        "update-rcd-params=s", => \$dh{U_PARAMS},
        "remove-d" => \$dh{D_FLAG},
        "no-enable" => \$dh{NO_ENABLE},
+       "require-tmpfiles" => \$dh{REQUIRE_TMPFILES},
 });
 
 if ($dh{RESTART_AFTER_UPGRADE} eq '') {
@@ -343,9 +353,11 @@ foreach my $package (@{$dh{DOPACKAGES}}) {
                if (@tmpfiles > 0) {
                        # Not migrated to hashref based autoscripts.  This will
                        # happen as people migrate to dh_installsystemd.
-                       autoscript($package,"postinst", 
"postinst-init-tmpfiles",
+                       my $snippet = $dh{REQUIRE_TMPFILES} ? 
'postinst-init-tmpfiles-required' : 'postinst-init-tmpfiles';
+                       autoscript($package,"postinst", $snippet,
                                "s,#TMPFILES#," . join(" ", sort 
@tmpfiles).",g");
-                       addsubstvar($package, "misc:Depends", 'systemd | 
systemd-tmpfiles');
+                       addsubstvar($package, "misc:Depends", 'systemd | 
systemd-tmpfiles')
+                               if $dh{REQUIRE_TMPFILES};
                }
        }
 
diff --git a/dh_installsystemd b/dh_installsystemd
index 4e5be46b..036ff540 100755
--- a/dh_installsystemd
+++ b/dh_installsystemd
@@ -16,7 +16,7 @@ our $VERSION = DH_BUILTIN_VERSION;
 
 =head1 SYNOPSIS
 
-B<dh_installsystemd> [S<I<debhelper options>>] [B<--restart-after-upgrade>] 
[B<--no-stop-on-upgrade>] [B<--no-enable>] [B<--no-start>] [B<--name=>I<name>] 
[S<I<unit file> ...>]
+B<dh_installsystemd> [S<I<debhelper options>>] [B<--restart-after-upgrade>] 
[B<--no-stop-on-upgrade>] [B<--no-enable>] [B<--no-start>] [B<--name=>I<name>] 
[B<--require-tmpfiles>] [S<I<unit file> ...>]
 
 =head1 DESCRIPTION
 
@@ -142,6 +142,14 @@ B<Note> that this option does not affect whether the 
services are
 enabled.  Please remember to also use B<--no-enable> if the services
 should not be enabled.
 
+=item B<--require-tmpfiles>
+
+This option ensures that a tmpfiles implementation is always available, so
+that the package can assume that any tmpfiles.d configuration files are
+processed. This is appropriate for cases where creating the declared files
+is necessary for ordinary operation of the package. Only used in compat 12
+or earlier.
+
 =item S<B<unit file> ...>
 
 Only process and generate maintscripts for the installed unit files
@@ -164,6 +172,7 @@ maintainer scripts.
 
 $dh{RESTART_AFTER_UPGRADE} = '';
 $dh{NO_START} = '';
+$dh{REQUIRE_TMPFILES} = '';
 
 init(options => {
        "no-enable" => \$dh{NO_ENABLE},
@@ -172,6 +181,7 @@ init(options => {
        "no-restart-on-upgrade" => \$dh{R_FLAG},
        "no-start" => \$dh{NO_START},
        "R|restart-after-upgrade!" => \$dh{RESTART_AFTER_UPGRADE},
+       "require-tmpfiles" => \$dh{REQUIRE_TMPFILES},
        "no-also" => \$dh{NO_ALSO}, # undocumented option
 });
 
@@ -318,8 +328,10 @@ if (compat(12)) {
                        }, @dirs) if @dirs;
 
                if (@tmpfiles) {
-                       autoscript($package, 'postinst', 
'postinst-init-tmpfiles', { 'TMPFILES' => join(' ', sort @tmpfiles) });
-                       addsubstvar($package, "misc:Depends", 'systemd | 
systemd-tmpfiles');
+                       my $snippet = $dh{REQUIRE_TMPFILES} ? 
'postinst-init-tmpfiles-required' : 'postinst-init-tmpfiles';
+                       autoscript($package, 'postinst', $snippet, { 'TMPFILES' 
=> join(' ', sort @tmpfiles) });
+                       addsubstvar($package, "misc:Depends", 'systemd | 
systemd-tmpfiles')
+                               if $dh{REQUIRE_TMPFILES};
                }
        }
 }
diff --git a/dh_installtmpfiles b/dh_installtmpfiles
index 9fa6b6e0..fd0a28ce 100755
--- a/dh_installtmpfiles
+++ b/dh_installtmpfiles
@@ -15,7 +15,7 @@ our $VERSION = DH_BUILTIN_VERSION;
 
 =head1 SYNOPSIS
 
-B<dh_installtmpfiles> [S<I<debhelper options>>][B<--name=>I<name>]
+B<dh_installtmpfiles> [S<I<debhelper options>>][B<--name=>I<name>] 
[B<--require-tmpfiles>]
 
 =head1 DESCRIPTION
 
@@ -39,6 +39,13 @@ This option controls both a prefix used for lookng up 
maintainer provided
 tmpfiles.d configuration files (those mentioned in the L</FILES> section)
 and also the base name used for the installed version of the file.
 
+=item B<--require-tmpfiles>
+
+This option ensures that a tmpfiles implementation is always available, so
+that the package can assume that any tmpfiles.d configuration files are
+processed. This is appropriate for cases where creating the declared files
+is necessary for ordinary operation of the package.
+
 =back
 
 =head1 FILES
@@ -66,7 +73,11 @@ maintainer scripts.
 
 =cut
 
-init();
+$dh{REQUIRE_TMPFILES} = '';
+
+init(options => {
+       "require-tmpfiles" => \$dh{REQUIRE_TMPFILES},
+});
 
 sub uniq {
        my %seen;
@@ -117,8 +128,10 @@ foreach my $package (@{$dh{DOPACKAGES}}) {
        }, @dirs) if @dirs;
 
        if (@tmpfiles) {
-               autoscript($package, 'postinst', 'postinst-init-tmpfiles', { 
'TMPFILES' => join(' ', uniq(sort(@tmpfiles))) });
-               addsubstvar($package, "misc:Depends", 'systemd | 
systemd-tmpfiles');
+               my $snippet = $dh{REQUIRE_TMPFILES} ? 
'postinst-init-tmpfiles-required' : 'postinst-init-tmpfiles';
+               autoscript($package, 'postinst', $snippet, { 'TMPFILES' => 
join(' ', uniq(sort(@tmpfiles))) });
+               addsubstvar($package, "misc:Depends", 'systemd | 
systemd-tmpfiles')
+                       if $dh{REQUIRE_TMPFILES};
        }
 }
 

Thanks,

-- 
Colin Watson (he/him)                              [cjwat...@debian.org]

Reply via email to