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]