[debhelper-devel] Bug#881303: lvm2: dh_installsystemd fragment generates warning during configuration

[Felipe Sateler]

On Thu, Nov 9, 2017 at 8:29 PM, Michael Biebl <biebl at debian.org> wrote:
> Control: reassign -1 debhelper 10.9.2
>
> On Thu, 09 Nov 2017 21:59:27 +0000 Julian Gilbey <jdg at debian.org> wrote:
>> Package: lvm2
>> Version: 2.02.176-1
>> Severity: normal
>>
>> I don't know whether the bug here lies in dh_installsystemd or in
>> lvm2, so please reassign to dh_installsystemd if this is the wrong
>> package.
>>
>> At the end of lvm2's debian/rules, it says:
>>       dh_installsystemd --remaining-packages
>>
>> But the lvm2 postinst then ends up finishing with the following:
>>
>> # Automatically added by dh_installsystemd/10.10.5
>> if [ -d /run/systemd/system ]; then
>>       systemctl --system daemon-reload >/dev/null || true
>>       if [ -n "$2" ]; then
>>               _dh_action=try-restart
>>       else
>>               _dh_action=start
>>       fi
>>       deb-systemd-invoke $_dh_action  >/dev/null || true
>> fi
>> # End automatically added section
>>
>> and this throws up the warning message:
>>
>> Syntax: /usr/bin/deb-systemd-invoke <action> <unit file> [<unit file> ...]
>>
>> because no unit file is listed after $_dh_action.
>>
>> I'm inclined to think that this is a debhelper bug...
>>
>
> I agree. Re-assigning to debhelper. dh_installsystemd should not
> generate such a (bogus) start/restart action.
>
> I wonder if this is a regression from dh_systemd_* or if the old tools
> are affected as well.

I think this is a regression. By merging both actions, the "exit if
there are no actionable units" condition changed.

The attached (untested) patch should fix the issue.

-- 

Saludos,
Felipe Sateler
-------------- next part --------------
diff --git a/dh_installsystemd b/dh_installsystemd
index 29c60db4..8772d022 100755
--- a/dh_installsystemd
+++ b/dh_installsystemd
@@ -344,43 +344,45 @@ foreach my $package (@{$dh{DOPACKAGES}}) {
 		}
 	}
 
-	next if @start_units == 0 && @enable_units == 0;
-
-	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 });
-		} else {
-			autoscript($package, 'postinst', 'postinst-systemd-enable', { 'UNITFILE' => $base });
+	if (@enable_units) {
+		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 });
+			} else {
+				autoscript($package, 'postinst', 'postinst-systemd-enable', { 'UNITFILE' => $base });
+			}
 		}
+		my $enableunitargs = join(' ', sort map { q{'} . basename($_) . q{'} } @enable_units);
+		autoscript($package, 'postrm', 'postrm-systemd', {'UNITFILES' => $enableunitargs });
 	}
-	my $enableunitargs = join(' ', sort map { q{'} . basename($_) . q{'} } @enable_units);
-	autoscript($package, 'postrm', 'postrm-systemd', {'UNITFILES' => $enableunitargs });
 	
-	# The $package and $sed parameters are always the same.
-	# This wrapper function makes the following logic easier to read.
-	my $startunitargs = join(' ', sort map { q{'} . basename($_) . q{'} } @start_units);
-	my $start_autoscript = sub {
-		my ($script, $filename) = @_;
-		autoscript($package, $script, $filename, { 'UNITFILES' => $startunitargs });
-	};
-
-	if ($dh{RESTART_AFTER_UPGRADE}) {
-		my $snippet = "postinst-systemd-restart" . ($dh{NO_START} ? "nostart" : "");
-		$start_autoscript->("postinst", $snippet);
-	} elsif (!$dh{NO_START}) {
-		# We need to stop/start before/after the upgrade.
-		$start_autoscript->("postinst", "postinst-systemd-start");
-	}
+	if (@start_units) {
+		# The $package and $sed parameters are always the same.
+		# This wrapper function makes the following logic easier to read.
+		my $startunitargs = join(' ', sort map { q{'} . basename($_) . q{'} } @start_units);
+		my $start_autoscript = sub {
+			my ($script, $filename) = @_;
+			autoscript($package, $script, $filename, { 'UNITFILES' => $startunitargs });
+		};
+	
+		if ($dh{RESTART_AFTER_UPGRADE}) {
+				my $snippet = "postinst-systemd-restart" . ($dh{NO_START} ? "nostart" : "");
+			$start_autoscript->("postinst", $snippet);
+		} elsif (!$dh{NO_START}) {
+			# We need to stop/start before/after the upgrade.
+			$start_autoscript->("postinst", "postinst-systemd-start");
+		}
 
-	$start_autoscript->("postrm", "postrm-systemd-reload-only");
+		$start_autoscript->("postrm", "postrm-systemd-reload-only");
 
-	if ($dh{R_FLAG} || $dh{RESTART_AFTER_UPGRADE}) {
-		# stop service only on remove
-		$start_autoscript->("prerm", "prerm-systemd-restart");
-	} elsif (!$dh{NO_START}) {
-		# always stop service
-		$start_autoscript->("prerm", "prerm-systemd");
+		if ($dh{R_FLAG} || $dh{RESTART_AFTER_UPGRADE}) {
+			# stop service only on remove
+			$start_autoscript->("prerm", "prerm-systemd-restart");
+		} elsif (!$dh{NO_START}) {
+			# always stop service
+			$start_autoscript->("prerm", "prerm-systemd");
+		}
 	}
 }