Bug#805878: dh-systemd: dh_systemd_start --no-start --restart-after-upgrade causes the service to be started on install

[Felipe Sateler]

Hi,

On 9 October 2016 at 17:26, Felipe Sateler  wrote:
> Control: tags -1 patch
>
> On 23 November 2015 at 11:34, Felipe Sateler  wrote:
>> Package: dh-systemd
>> Version: 1.24
>> Severity: normal
>>
>> Current autoscript has:
>>
>> 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 #UNITFILES# >/dev/null || true
>> fi
>>
>>
>> And this does not take into account that --no-start was passed. If it
>> was passed, then try-restart should always be used.
>
> This is more problematic now that restart-on-upgrade is default. The
> following patch fixes the issue:

Please find attached a patch rebased against current master.
Alternatively, you can fetch the branch `restartnorestart` from my
clone[1].

I have re-tested that this gives the desired result:

--restart-after-upgrade + --start => starts and restarts service.
--no-restart-after-upgrade + --start => always starts
--restart-after-upgrade + --no-start => only restarts
-- no-restart-after-upgrade + --no-start => nothing

[1] https://anonscm.debian.org/git/users/fsateler/debhelper.git

Please attach this so that this can be included in stretch.

-- 

Saludos,
Felipe Sateler
From 1afe86c10a378f5fda82c87db243a2ea7e4f8ebe Mon Sep 17 00:00:00 2001
From: Felipe Sateler 
Date: Sat, 22 Oct 2016 20:49:12 -0300
Subject: [PATCH] systemd_start: do not start when --no-start and
 --restart-after-upgrade are combined

But do restart on upgrades
---
 autoscripts/postinst-systemd-restartnostart | 6 ++
 dh_systemd_start| 3 ++-
 2 files changed, 8 insertions(+), 1 deletion(-)
 create mode 100644 autoscripts/postinst-systemd-restartnostart

diff --git a/autoscripts/postinst-systemd-restartnostart b/autoscripts/postinst-systemd-restartnostart
new file mode 100644
index ..eb52e273
--- /dev/null
+++ b/autoscripts/postinst-systemd-restartnostart
@@ -0,0 +1,6 @@
+if [ -d /run/systemd/system ]; then
+	systemctl --system daemon-reload >/dev/null || true
+	if [ -n "$2" ]; then
+		deb-systemd-invoke try-restart #UNITFILES# >/dev/null || true
+	fi
+fi
diff --git a/dh_systemd_start b/dh_systemd_start
index 940fc80e..46c14a7d 100755
--- a/dh_systemd_start
+++ b/dh_systemd_start
@@ -225,7 +225,8 @@ foreach my $package (@{$dh{DOPACKAGES}}) {
 	};
 
 	if ($dh{RESTART_AFTER_UPGRADE}) {
-		$sd_autoscript->("postinst", "postinst-systemd-restart");
+		my $snippet = "postinst-systemd-restart" . ($dh{NO_START} ? "nostart" : "");
+		$sd_autoscript->("postinst", $snippet);
 	} elsif (!$dh{NO_START}) {
 		# We need to stop/start before/after the upgrade.
 		$sd_autoscript->("postinst", "postinst-systemd-start");
-- 
2.11.0

Bug#805878: dh-systemd: dh_systemd_start --no-start --restart-after-upgrade causes the service to be started on install

[Michael Biebl]

Am 13.10.2016 um 18:08 schrieb Michael Biebl:
> Am 13.10.2016 um 17:56 schrieb Michael Biebl:
>> Am 13.10.2016 um 17:47 schrieb Felipe Sateler:
>>> On 13 October 2016 at 12:36, Michael Biebl  wrote:

 With compat level 9 and older, --no-start did not generate any
 maintainer scripts code at all.
>>>
>>> You can get the same with dh 10 with --no-start --no-restart-after-upgrade

> fwiw, in network-manager I use
> 
>> override_dh_systemd_start:
>>dh_link lib/systemd/system/NetworkManager.service \
>>lib/systemd/system/network-manager.service
>>dh_systemd_start -pnetwork-manager --no-start 
>> NetworkManager-dispatcher.service
>>dh_systemd_start -pnetwork-manager --no-start 
>> NetworkManager-wait-online.service
>>dh_systemd_start -pnetwork-manager --restart-after-upgrade 
>> NetworkManager.service


For some reason --no-start --no-restart-on-upgrade still generates code
for NetworkManager-dispatcher.service.

Something odd going on here. Maybe dh_systemd_start get's confused by the
[Install]
Also=NetworkManager-dispatcher.service

in NetworkManager.service

-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?



signature.asc
Description: OpenPGP digital signature

Bug#805878: dh-systemd: dh_systemd_start --no-start --restart-after-upgrade causes the service to be started on install

[Michael Biebl]

Am 13.10.2016 um 17:56 schrieb Michael Biebl:
> Am 13.10.2016 um 17:47 schrieb Felipe Sateler:
>> On 13 October 2016 at 12:36, Michael Biebl  wrote:
>>>
>>> Now that compat level 10 has --restart-after-upgrade as default, this
>>> bug has a wider impact (thus bumping the severity).
>>>
>>> With compat level 9 and older, --no-start did not generate any
>>> maintainer scripts code at all.
>>
>> You can get the same with dh 10 with --no-start --no-restart-after-upgrade
> 
> Right. But isn't this an unexpected change?
> See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=837528, which is
> related.
> I wonder if --no-start should imply --no-restart-after-upgrade
> 
> If not, we should at least improve the documentation in that regard.


fwiw, in network-manager I use

> override_dh_systemd_start:
>dh_link lib/systemd/system/NetworkManager.service \
>lib/systemd/system/network-manager.service
>dh_systemd_start -pnetwork-manager --no-start 
> NetworkManager-dispatcher.service
>dh_systemd_start -pnetwork-manager --no-start 
> NetworkManager-wait-online.service
>dh_systemd_start -pnetwork-manager --restart-after-upgrade 
> NetworkManager.service

After the compat bump to 10, I get the attached diff, which shows many odd 
issues

postinst/
- NetworkManager-dispatcher.service is restarted 3 times, 
NetworkManager-wait-online.service 2 times
- we now process NetworkManager.service, although that should be handled by 
update-rc.d

postrm/,
- systemctl --system daemon-reload has been added 4 times

prerm/
- NetworkManager-dispatcher.service is stopped 3 times, 
NetworkManager-wait-online.service 2 times

-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?
From be6b6122a9e8153956949460e3a4adabe01cc848 Mon Sep 17 00:00:00 2001
From: Michael Biebl 
Date: Thu, 13 Oct 2016 17:59:24 +0200
Subject: [PATCH] dh 10

---
 debian/network-manager.postinst.debhelper | 89 +++
 debian/network-manager.postrm.debhelper   | 34 
 debian/network-manager.prerm.debhelper| 20 +++
 3 files changed, 143 insertions(+)

diff --git a/debian/network-manager.postinst.debhelper b/debian/network-manager.postinst.debhelper
index 8bdeb31..51c7a9f 100644
--- a/debian/network-manager.postinst.debhelper
+++ b/debian/network-manager.postinst.debhelper
@@ -1,3 +1,48 @@
+# Automatically added by dh_systemd_enable
+# This will only remove masks created by d-s-h on package removal.
+deb-systemd-helper unmask NetworkManager-dispatcher.service >/dev/null || true
+
+# was-enabled defaults to true, so new installations run enable.
+if deb-systemd-helper --quiet was-enabled NetworkManager-dispatcher.service; then
+	# Enables the unit on first installation, creates new
+	# symlinks on upgrades if the unit file has changed.
+	deb-systemd-helper enable NetworkManager-dispatcher.service >/dev/null || true
+else
+	# Update the statefile to add new symlinks (if any), which need to be
+	# cleaned up on purge. Also remove old symlinks.
+	deb-systemd-helper update-state NetworkManager-dispatcher.service >/dev/null || true
+fi
+# End automatically added section
+# Automatically added by dh_systemd_enable
+# This will only remove masks created by d-s-h on package removal.
+deb-systemd-helper unmask NetworkManager-wait-online.service >/dev/null || true
+
+# was-enabled defaults to true, so new installations run enable.
+if deb-systemd-helper --quiet was-enabled NetworkManager-wait-online.service; then
+	# Enables the unit on first installation, creates new
+	# symlinks on upgrades if the unit file has changed.
+	deb-systemd-helper enable NetworkManager-wait-online.service >/dev/null || true
+else
+	# Update the statefile to add new symlinks (if any), which need to be
+	# cleaned up on purge. Also remove old symlinks.
+	deb-systemd-helper update-state NetworkManager-wait-online.service >/dev/null || true
+fi
+# End automatically added section
+# Automatically added by dh_systemd_enable
+# This will only remove masks created by d-s-h on package removal.
+deb-systemd-helper unmask NetworkManager.service >/dev/null || true
+
+# was-enabled defaults to true, so new installations run enable.
+if deb-systemd-helper --quiet was-enabled NetworkManager.service; then
+	# Enables the unit on first installation, creates new
+	# symlinks on upgrades if the unit file has changed.
+	deb-systemd-helper enable NetworkManager.service >/dev/null || true
+else
+	# Update the statefile to add new symlinks (if any), which need to be
+	# cleaned up on purge. Also remove old symlinks.
+	deb-systemd-helper update-state NetworkManager.service >/dev/null || true
+fi
+# End automatically added section
 # Automatically added by dh_installinit
 if [ "$1" = "configure" ] || [ "$1" = "abort-upgrade" ]; then
 	if [ -x "/etc/init.d/network-manager" ]; then
@@ -11,3 +56,47 @@ if [ "$1" = "configure" ] || [ "$1" = 

Bug#805878: dh-systemd: dh_systemd_start --no-start --restart-after-upgrade causes the service to be started on install

[Michael Biebl]

Am 13.10.2016 um 17:47 schrieb Felipe Sateler:
> On 13 October 2016 at 12:36, Michael Biebl  wrote:
>>
>> Now that compat level 10 has --restart-after-upgrade as default, this
>> bug has a wider impact (thus bumping the severity).
>>
>> With compat level 9 and older, --no-start did not generate any
>> maintainer scripts code at all.
> 
> You can get the same with dh 10 with --no-start --no-restart-after-upgrade

Right. But isn't this an unexpected change?
See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=837528, which is
related.
I wonder if --no-start should imply --no-restart-after-upgrade

If not, we should at least improve the documentation in that regard.

Michael


-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?



signature.asc
Description: OpenPGP digital signature

Bug#805878: dh-systemd: dh_systemd_start --no-start --restart-after-upgrade causes the service to be started on install

[Felipe Sateler]

On 13 October 2016 at 12:36, Michael Biebl  wrote:
> Control: severity -1 important
>
> On Mon, 23 Nov 2015 11:34:47 -0300 Felipe Sateler 
> wrote:
>> Package: dh-systemd
>> Version: 1.24
>> Severity: normal
>>
>> Current autoscript has:
>>
>> 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 #UNITFILES# >/dev/null || true
>> fi
>>
>>
>> And this does not take into account that --no-start was passed. If it
>> was passed, then try-restart should always be used.
>>
>
> Now that compat level 10 has --restart-after-upgrade as default, this
> bug has a wider impact (thus bumping the severity).
>
> With compat level 9 and older, --no-start did not generate any
> maintainer scripts code at all.

You can get the same with dh 10 with --no-start --no-restart-after-upgrade

>
> Aside from the bug that "start" is used on new installs, I wonder if we
> should (try-)restart the service at all.

There are 4 types of service:

1. Start on install, use stop/start on upgrades
  --no-restart-after-upgrade
2. Start on install, use restart on upgrades
  --restart-after-upgrade
3. Don't start on install, restart on upgrades
  --no-start --restart-after-upgrade # not working
4. Don't start on install, don't restart on upgrades
  --no-start --no-restart-after-upgrade

Currently, we can do 1, 2 and 4. With my patch, we add option 3.

However, the interface is less than intuitive :/

-- 

Saludos,
Felipe Sateler

Bug#805878: dh-systemd: dh_systemd_start --no-start --restart-after-upgrade causes the service to be started on install

[Michael Biebl]

Control: severity -1 important

On Mon, 23 Nov 2015 11:34:47 -0300 Felipe Sateler 
wrote:
> Package: dh-systemd
> Version: 1.24
> Severity: normal
> 
> Current autoscript has:
> 
> 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 #UNITFILES# >/dev/null || true
> fi
> 
> 
> And this does not take into account that --no-start was passed. If it
> was passed, then try-restart should always be used.
> 

Now that compat level 10 has --restart-after-upgrade as default, this
bug has a wider impact (thus bumping the severity).

With compat level 9 and older, --no-start did not generate any
maintainer scripts code at all.

Aside from the bug that "start" is used on new installs, I wonder if we
should (try-)restart the service at all.

Thoughts?


Michael


-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?



signature.asc
Description: OpenPGP digital signature

Bug#805878: dh-systemd: dh_systemd_start --no-start --restart-after-upgrade causes the service to be started on install

[Felipe Sateler]

Control: tags -1 patch

On 23 November 2015 at 11:34, Felipe Sateler  wrote:
> Package: dh-systemd
> Version: 1.24
> Severity: normal
>
> Current autoscript has:
>
> 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 #UNITFILES# >/dev/null || true
> fi
>
>
> And this does not take into account that --no-start was passed. If it
> was passed, then try-restart should always be used.

This is more problematic now that restart-on-upgrade is default. The
following patch fixes the issue:

diff --git a/autoscripts/postinst-systemd-restartnostart
b/autoscripts/postinst-systemd-restartnostart
index e69de29..eb52e27 100644
--- a/autoscripts/postinst-systemd-restartnostart
+++ b/autoscripts/postinst-systemd-restartnostart
@@ -0,0 +1,6 @@
+if [ -d /run/systemd/system ]; then
+ systemctl --system daemon-reload >/dev/null || true
+ if [ -n "$2" ]; then
+ deb-systemd-invoke try-restart #UNITFILES# >/dev/null || true
+ fi
+fi
diff --git a/dh_systemd_start b/dh_systemd_start
index 940fc80..46c14a7 100755
--- a/dh_systemd_start
+++ b/dh_systemd_start
@@ -225,7 +225,8 @@ foreach my $package (@{$dh{DOPACKAGES}}) {
  };

  if ($dh{RESTART_AFTER_UPGRADE}) {
- $sd_autoscript->("postinst", "postinst-systemd-restart");
+ my $snippet = "postinst-systemd-restart" . ($dh{NO_START} ? "nostart" : "");
+ $sd_autoscript->("postinst", $snippet);
  } elsif (!$dh{NO_START}) {
  # We need to stop/start before/after the upgrade.
  $sd_autoscript->("postinst", "postinst-systemd-start");


-- 

Saludos,
Felipe Sateler

Bug#805878: dh-systemd: dh_systemd_start --no-start --restart-after-upgrade causes the service to be started on install

[Felipe Sateler]

Package: dh-systemd
Version: 1.24
Severity: normal

Current autoscript has:

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 #UNITFILES# >/dev/null || true
fi


And this does not take into account that --no-start was passed. If it
was passed, then try-restart should always be used.


-- System Information:
Debian Release: stretch/sid
  APT prefers unstable
  APT policy: (500, 'unstable'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 4.2.0-1-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash
Init: systemd (via /run/systemd/system)

Versions of packages dh-systemd depends on:
ii  debhelper  9.20151117
ii  perl   5.20.2-6

dh-systemd recommends no packages.

Versions of packages dh-systemd suggests:
ii  augeas-tools  1.2.0-0.2

-- no debconf information