dh_install{init,systemd}: Re-order service autosnippets

Depends on e6870ceafb9d51800de86a7106cdfb4ce9c9dad8

Closes: Debian#885998, #885998
Signed-off-by: Niels Thykier <niels@thykier.net>

7 files changed, 104 insertions, 22 deletions

diff --git a/debian/changelog b/debian/changelog
index b697f203..d70824dd 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -61,6 +61,16 @@ debhelper (11.1) UNRELEASED; urgency=medium
     Adrian Bunk for the suggestion.  (Closes: #886127)
   * Dh_Lib.pm/dh_testroot: Support the DEB_GAIN_ROOT_CMD environment
     that replaces DPKG_GAIN_ROOT_CMD.
+  * dh_installinit/dh_installsystemd: Re-order snippets so service
+    handling is always the first to happen in removal scripts and last
+    to happen on install scripts.  This means that configuration file
+    management and systemd-tmpfiles handling will now always happen
+    before the service is started on install/upgrade.  Thanks to
+    Дилян Палаузов and Simon McVittie for reporting the bug.
+    (Closes: #885998, #885998)
+    - Note that the deprecated dh_systemd_enable and dh_systemd_start
+      helpers have not been changed.  Services handled by these may
+      still be started before configuration management happens.
 
  -- Niels Thykier <niels@thykier.net>  Sun, 17 Dec 2017 07:59:18 +0000
 
diff --git a/dh_installinit b/dh_installinit
index 54107698..39b856ab 100755
--- a/dh_installinit
+++ b/dh_installinit
@@ -225,6 +225,8 @@ init(options => {
 
 # PROMISE: DH NOOP WITHOUT service tmpfile default upstart init init.d tmp(usr/lib/tmpfiles.d) tmp(etc/tmpfiles.d)
 
+my %snippet_options = ('snippet-order' => 'service');
+
 foreach my $package (@{$dh{DOPACKAGES}}) {
 	my $tmp=tmpdir($package);
 
@@ -358,34 +360,40 @@ foreach my $package (@{$dh{DOPACKAGES}}) {
 					# update-rc.d, and restart (or
 					# start if new install) script
 					autoscript($package,"postinst", "postinst-init-restart",
-						{ 'SCRIPT' => $script, 'INITPARMS' => $params, 'ERROR_HANDLER' => $dh{ERROR_HANDLER} });
+						{ 'SCRIPT' => $script, 'INITPARMS' => $params, 'ERROR_HANDLER' => $dh{ERROR_HANDLER} },
+						\%snippet_options);
 				}
 				else {
 					# update-rc.d, and start script
 					autoscript($package,"postinst", "postinst-init",
-						{ 'SCRIPT' => $script, 'INITPARMS' => $params, 'ERROR_HANDLER' => $dh{ERROR_HANDLER} });
+						{ 'SCRIPT' => $script, 'INITPARMS' => $params, 'ERROR_HANDLER' => $dh{ERROR_HANDLER} },
+						\%snippet_options);
 				}
 			
 				if ($dh{R_FLAG} || $dh{RESTART_AFTER_UPGRADE}) {
 					# stops script only on remove
 					autoscript($package,"prerm","prerm-init-norestart",
-						{ 'SCRIPT' => $script, 'ERROR_HANDLER' => $dh{ERROR_HANDLER} });
+						{ 'SCRIPT' => $script, 'ERROR_HANDLER' => $dh{ERROR_HANDLER} },
+						\%snippet_options);
 				}
 				else {
 					# always stops script
 					autoscript($package,"prerm","prerm-init",
-						{ 'SCRIPT' => $script, 'ERROR_HANDLER' => $dh{ERROR_HANDLER} });
+						{ 'SCRIPT' => $script, 'ERROR_HANDLER' => $dh{ERROR_HANDLER} },
+						\%snippet_options);
 				}
 			}
 			else {
 				# just update-rc.d
 				autoscript($package,"postinst", "postinst-init-nostart",
-					{ 'SCRIPT' => $script, 'INITPARMS' => $params, 'ERROR_HANDLER' => $dh{ERROR_HANDLER} });
+					{ 'SCRIPT' => $script, 'INITPARMS' => $params, 'ERROR_HANDLER' => $dh{ERROR_HANDLER} },
+					\%snippet_options);
 			}
 
 			# removes rc.d links
 			autoscript($package,"postrm","postrm-init",
-				{ 'SCRIPT' => $script, 'ERROR_HANDLER' => $dh{ERROR_HANDLER} });
+				{ 'SCRIPT' => $script, 'ERROR_HANDLER' => $dh{ERROR_HANDLER} },
+				\%snippet_options);
 		}
 	}
 }
diff --git a/dh_installsystemd b/dh_installsystemd
index 75a3abd3..647dd3ef 100755
--- a/dh_installsystemd
+++ b/dh_installsystemd
@@ -210,6 +210,8 @@ sub extract_key {
 my %requested_files = map { basename($_) => 1 } @ARGV;
 my %installed_files;
 
+my %snippet_options = ('snippet-order' => 'service');
+
 foreach my $package (@{$dh{DOPACKAGES}}) {
 	my $tmpdir = tmpdir($package);
 	my (@installed_units, @start_units,  @enable_units, %aliases, @tmpfiles);
@@ -349,9 +351,11 @@ foreach my $package (@{$dh{DOPACKAGES}}) {
 		for my $unit (sort @enable_units) {
 			my $base = q{'} . basename($unit) . q{'};
 			if ($dh{NO_ENABLE}) {
-				autoscript($package, 'postinst', 'postinst-systemd-dont-enable', { 'UNITFILE' => $base });
+				autoscript($package, 'postinst', 'postinst-systemd-dont-enable', { 'UNITFILE' => $base },
+					\%snippet_options);
 			} else {
-				autoscript($package, 'postinst', 'postinst-systemd-enable', { 'UNITFILE' => $base });
+				autoscript($package, 'postinst', 'postinst-systemd-enable', { 'UNITFILE' => $base },
+					\%snippet_options);
 			}
 		}
 		my $enableunitargs = join(' ', sort map { q{'} . basename($_) . q{'} } @enable_units);
@@ -364,7 +368,8 @@ foreach my $package (@{$dh{DOPACKAGES}}) {
 		my $startunitargs = join(' ', sort map { q{'} . basename($_) . q{'} } @start_units);
 		my $start_autoscript = sub {
 			my ($script, $filename) = @_;
-			autoscript($package, $script, $filename, { 'UNITFILES' => $startunitargs });
+			autoscript($package, $script, $filename, { 'UNITFILES' => $startunitargs },
+				\%snippet_options);
 		};
 
 		if ($dh{RESTART_AFTER_UPGRADE}) {
diff --git a/t/dh_installinit/dh_installinit.t b/t/dh_installinit/dh_installinit.t
index 20860f19..240d44d5 100755
--- a/t/dh_installinit/dh_installinit.t
+++ b/t/dh_installinit/dh_installinit.t
@@ -24,7 +24,7 @@ each_compat_up_to_and_incl_subtest(10, sub {
 	make_path(qw(debian/foo debian/bar debian/baz));
 	ok(run_dh_tool({ 'needs_root' => 1 }, 'dh_installinit'));
 	ok(-e "debian/foo/lib/systemd/system/foo.service");
-	ok(-e "debian/foo.postinst.debhelper");
+	ok(find_script('foo', 'postinst'));
 	ok(run_dh_tool('dh_clean'));
 
 });
@@ -34,13 +34,13 @@ each_compat_from_and_above_subtest(11, sub {
 
 	ok(run_dh_tool({ 'needs_root' => 1 }, 'dh_installinit'));
 	ok(! -e "debian/foo/lib/systemd/system/foo.service");
-	ok(! -e "debian/foo.postinst.debhelper");
+	ok(!find_script('foo', 'postinst'));
 	ok(run_dh_tool('dh_clean'));
 
 	make_path(qw(debian/foo/lib/systemd/system/ debian/bar debian/baz));
 	install_file('debian/foo.service', 'debian/foo/lib/systemd/system/foo.service');
 	ok(run_dh_tool({ 'needs_root' => 1 }, 'dh_installinit'));
-	ok(! -e "debian/foo.postinst.debhelper");
+	ok(!find_script('foo', 'postinst'));
 	ok(run_dh_tool('dh_clean'));
 });
 
diff --git a/t/dh_installsystemd/debian/foo.init b/t/dh_installsystemd/debian/foo.init
new file mode 100644
index 00000000..2b77921a
--- /dev/null
+++ b/t/dh_installsystemd/debian/foo.init
@@ -0,0 +1,4 @@
+#!/bin/sh
+
+some script
+
diff --git a/t/dh_installsystemd/dh_installsystemd.t b/t/dh_installsystemd/dh_installsystemd.t
index f7e806ff..a33a8359 100755
--- a/t/dh_installsystemd/dh_installsystemd.t
+++ b/t/dh_installsystemd/dh_installsystemd.t
@@ -26,10 +26,12 @@ sub unit_is_enabled {
 	my @output;
 	my $matches;
 	$num_masks = $num_masks // $num_enables;
-	@output=`cat debian/$package.postinst.debhelper`;
+	my @postinst_snippets = find_script($package, 'postinst');
+	@output=`cat @postinst_snippets` if @postinst_snippets;
 	$matches = grep { m{^if deb-systemd-helper .* was-enabled .*'\Q$unit\E\.service'} } @output;
 	ok($matches == $num_enables) or diag("$unit appears to have been enabled $matches times (expected $num_enables)");
-	@output=`cat debian/$package.postrm.debhelper`;
+	my @postrm_snippets = find_script($package, 'postrm');
+	@output=`cat @postrm_snippets` if @postrm_snippets;
 	$matches = grep { m{deb-systemd-helper mask.*'\Q$unit\E\.service'} } @output;
 	ok($matches == $num_masks) or diag("$unit appears to have been masked $matches times (expected $num_masks)");
 }
@@ -38,10 +40,12 @@ sub unit_is_started {
 	my @output;
 	my $matches;
 	$num_stops = $num_stops // $num_starts;
-	@output=`cat debian/$package.postinst.debhelper`;
+	my @postinst_snippets = find_script($package, 'postinst');
+	@output=`cat @postinst_snippets` if @postinst_snippets;
 	$matches = grep { m{deb-systemd-invoke \$_dh_action .*'\Q$unit\E.service'} } @output;
 	ok($matches == $num_starts) or diag("$unit appears to have been started $matches times (expected $num_starts)");
-	@output=`cat debian/$package.prerm.debhelper`;
+	my @prerm_snippets = find_script($package, 'prerm');
+	@output=`cat @prerm_snippets` if @prerm_snippets;
 	$matches = grep { m{deb-systemd-invoke stop .*'\Q$unit\E.service'} } @output;
 	ok($matches == $num_stops) or diag("$unit appears to have been stopped $matches times (expected $num_stops)");
 }
@@ -50,7 +54,7 @@ sub unit_is_started {
 each_compat_subtest {
 	ok(run_dh_tool({ 'needs_root' => 1 }, 'dh_installsystemd'));
 	ok(-e "debian/foo/lib/systemd/system/foo.service");
-	ok(-e "debian/foo.postinst.debhelper");
+	ok(find_script('foo', 'postinst'));
 	unit_is_enabled('foo', 'foo', 1);
 	unit_is_started('foo', 'foo', 1);
 	unit_is_enabled('foo', 'foo2', 0);
@@ -61,7 +65,7 @@ each_compat_subtest {
 	install_file('debian/foo2.service', 'debian/foo/lib/systemd/system/foo2.service');
 	ok(run_dh_tool({ 'needs_root' => 1 }, 'dh_installsystemd'));
 	ok(-e "debian/foo/lib/systemd/system/foo.service");
-	ok(-e "debian/foo.postinst.debhelper");
+	ok(find_script('foo', 'postinst'));
 	unit_is_enabled('foo', 'foo', 1);
 	unit_is_started('foo', 'foo', 1);
 	unit_is_enabled('foo', 'foo2', 1);
@@ -72,7 +76,7 @@ each_compat_subtest {
 	install_file('debian/foo2.service', 'debian/foo/lib/systemd/system/foo2.service');
 	ok(run_dh_tool({ 'needs_root' => 1 }, 'dh_installsystemd', '--no-start'));
 	ok(-e "debian/foo/lib/systemd/system/foo.service");
-	ok(-e "debian/foo.postinst.debhelper");
+	ok(find_script('foo', 'postinst'));
 	unit_is_enabled('foo', 'foo', 1);
 	unit_is_started('foo', 'foo', 0, 1); # present units are stopped on remove even if no start
 	unit_is_enabled('foo', 'foo2', 1);
@@ -84,7 +88,7 @@ each_compat_subtest {
 	ok(run_dh_tool({ 'needs_root' => 1 }, 'dh_installsystemd', '--no-start', 'debian/foo.service'));
 	ok(run_dh_tool({ 'needs_root' => 1 }, 'dh_installsystemd', '-p', 'foo', 'foo2.service'));
 	ok(-e "debian/foo/lib/systemd/system/foo.service");
-	ok(-e "debian/foo.postinst.debhelper");
+	ok(find_script('foo', 'postinst'));
 	unit_is_enabled('foo', 'foo', 1);
 	unit_is_started('foo', 'foo', 0, 1);
 	unit_is_enabled('foo', 'foo2', 1);
@@ -96,7 +100,7 @@ each_compat_subtest {
 	ok(run_dh_tool({ 'needs_root' => 1 }, 'dh_installsystemd', '--no-enable', 'debian/foo.service'));
 	ok(run_dh_tool({ 'needs_root' => 1 }, 'dh_installsystemd', '-p', 'foo', 'foo2.service'));
 	ok(-e "debian/foo/lib/systemd/system/foo.service");
-	ok(-e "debian/foo.postinst.debhelper");
+	ok(find_script('foo', 'postinst'));
 	unit_is_enabled('foo', 'foo', 0, 1); # Disabled units are still masked on removal
 	unit_is_started('foo', 'foo', 1, 1);
 	unit_is_enabled('foo', 'foo2', 1);
@@ -105,7 +109,9 @@ each_compat_subtest {
 
 	make_path('debian/foo/lib/systemd/system/');
 	ok(run_dh_tool({ 'needs_root' => 1 }, 'dh_installsystemd', '--no-restart-after-upgrade'));
-        my $matches = grep { m{deb-systemd-invoke start .*foo.service} } `cat debian/foo.postinst.debhelper`;
+	my @foo_postinst = find_script('foo', 'postinst');
+	ok(@foo_postinst);
+	my $matches = @foo_postinst ? grep { m{deb-systemd-invoke start .*foo.service} } `cat @foo_postinst` : -1;
 	ok($matches == 1);
 	ok(run_dh_tool('dh_clean'));
 
diff --git a/t/dh_installsystemd/dh_installsystemd_tmpfiles.t b/t/dh_installsystemd/dh_installsystemd_tmpfiles.t
new file mode 100755
index 00000000..9934dac5
--- /dev/null
+++ b/t/dh_installsystemd/dh_installsystemd_tmpfiles.t
@@ -0,0 +1,49 @@
+#!/usr/bin/perl
+use strict;
+use Test::More;
+
+use File::Basename qw(dirname);
+use lib dirname(dirname(__FILE__));
+use Test::DH;
+use File::Path qw(remove_tree make_path);
+use Debian::Debhelper::Dh_Lib qw(!dirname);
+
+our @TEST_DH_EXTRA_TEMPLATE_FILES = (qw(
+    debian/changelog
+    debian/control
+    debian/foo.service
+    debian/foo.init
+));
+
+if (uid_0_test_is_ok()) {
+	plan(tests => 1);
+} else {
+	plan skip_all => 'fakeroot required';
+}
+
+# Units are installed and enabled
+each_compat_from_and_above_subtest(11, sub {
+	make_path('debian/foo/usr/lib/tmpfiles.d');
+	create_empty_file('debian/foo/usr/lib/tmpfiles.d/foo.conf');
+	ok(run_dh_tool({ 'needs_root' => 1 }, 'dh_installinit'));
+	ok(run_dh_tool({ 'needs_root' => 1 }, 'dh_installsystemd'));
+	ok(-e "debian/foo/etc/init.d/foo");
+	ok(-e "debian/foo/lib/systemd/system/foo.service");
+	my @postinst = find_script('foo', 'postinst');
+	# We should have too snippets (one for the tmpfiles and one for the services).
+	is(scalar(@postinst), 2);
+	if (scalar(@postinst) == 2) {
+		open(my $fd, '<', $postinst[0]) or error("open($postinst[0]) failed: $!");
+		my $early_snippet = readlines($fd);
+		close($fd);
+		open($fd, '<', $postinst[1]) or error("open($postinst[1]) failed: $!");
+		my $late_snippet = readlines($fd);
+		close($fd);
+		ok(! grep { m/(?:invoke|update)-rc.d|deb-systemd-invoke/ } @{$early_snippet});
+		ok(grep { m/(?:invoke|update)-rc.d|deb-systemd-invoke/ } @{$late_snippet});
+		ok(grep { m/systemd-tmpfiles/ } @{$early_snippet});
+		ok(! grep { m/systemd-tmpfiles/ } @{$late_snippet});
+	}
+	ok(run_dh_tool('dh_clean'));
+
+});