Hi,
On Sat, Mar 11, 2017 at 10:03:34PM +0100, Wolfgang Schweer wrote:
> > There is still one thing left: in the SysV case, the user will have to
> > issue update-rc.d thinkfan enable AND edit /etc/default/thinkfan.
> > 
> > Thinking how to fix that.
>  
> I guess it would be sufficient to add a line to /etc/default/thinkfan, 
> so that it's like this:
> 
> # Should thinkfan be started automatically on boot?
> # Only say "yes" when you know what you are doing, have configured
> # thinkfan correctly for *YOUR* machine and loaded thinkpad_acpi
> # with fan_control=1 (if you have a ThinkPad).
> # Also, running 'update-rc.d thinkfan enable' is needed.
> START=no

I went a bit further, droped that code, and migrated the setting
to update-rc.d on upgrade. See attached patchset.

What do you think?
>From b5d70ee07a27b4baae726c36786b78725dec0a5a Mon Sep 17 00:00:00 2001
From: Evgeni Golov <evg...@debian.org>
Date: Sat, 11 Mar 2017 14:58:59 +0100
Subject: [PATCH 1/4] call update-rc.d thinkfan disable on fresh installs

Closes: #758579
---
 debian/thinkfan.lintian-overrides |  3 +++
 debian/thinkfan.postinst          | 44 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)
 create mode 100644 debian/thinkfan.lintian-overrides
 create mode 100644 debian/thinkfan.postinst

diff --git a/debian/thinkfan.lintian-overrides b/debian/thinkfan.lintian-overrides
new file mode 100644
index 0000000..62ce724
--- /dev/null
+++ b/debian/thinkfan.lintian-overrides
@@ -0,0 +1,3 @@
+# pre-deployment of a disabled service as dh_installinit is
+# not capable to do so for us.
+thinkfan: duplicate-updaterc.d-calls-in-postinst thinkfan
diff --git a/debian/thinkfan.postinst b/debian/thinkfan.postinst
new file mode 100644
index 0000000..c385313
--- /dev/null
+++ b/debian/thinkfan.postinst
@@ -0,0 +1,44 @@
+#!/bin/sh
+# postinst script for thinkfan
+#
+# see: dh_installdeb(1)
+
+set -e
+
+# summary of how this script can be called:
+#        * <postinst> `configure' <most-recently-configured-version>
+#        * <old-postinst> `abort-upgrade' <new version>
+#        * <conflictor's-postinst> `abort-remove' `in-favour' <package>
+#          <new-version>
+#        * <postinst> `abort-remove'
+#        * <deconfigured's-postinst> `abort-deconfigure' `in-favour'
+#          <failed-install-package> <version> `removing'
+#          <conflicting-package> <version>
+# for details, see https://www.debian.org/doc/debian-policy/ or
+# the debian-policy package
+
+
+case "$1" in
+    configure)
+        if [ -x "/etc/init.d/thinkfan" ] && [ -z "$2" ]; then
+            # Disable the service by default on new installations
+            update-rc.d thinkfan defaults >/dev/null || true
+            update-rc.d thinkfan disable >/dev/null || true
+	    fi
+    ;;
+
+    abort-upgrade|abort-remove|abort-deconfigure)
+    ;;
+
+    *)
+        echo "postinst called with unknown argument \`$1'" >&2
+        exit 1
+    ;;
+esac
+
+# dh_installdeb will replace this with shell code automatically
+# generated by other debhelper scripts.
+
+#DEBHELPER#
+
+exit 0
-- 
2.11.0

>From 38cb2ac4448044f40931ec97065c7a7f8b4fd54c Mon Sep 17 00:00:00 2001
From: Evgeni Golov <evg...@debian.org>
Date: Sun, 12 Mar 2017 08:39:33 +0100
Subject: [PATCH 2/4] migrate old installations to a properly disabled sysv
 service too

---
 debian/thinkfan.lintian-overrides |  4 ++++
 debian/thinkfan.preinst           | 43 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)
 create mode 100644 debian/thinkfan.preinst

diff --git a/debian/thinkfan.lintian-overrides b/debian/thinkfan.lintian-overrides
index 62ce724..292b9cd 100644
--- a/debian/thinkfan.lintian-overrides
+++ b/debian/thinkfan.lintian-overrides
@@ -1,3 +1,7 @@
 # pre-deployment of a disabled service as dh_installinit is
 # not capable to do so for us.
 thinkfan: duplicate-updaterc.d-calls-in-postinst thinkfan
+
+# only called for upgrades from older versions to migrate
+# enabled/disabled state
+thinkfan: preinst-calls-updaterc.d thinkfan
diff --git a/debian/thinkfan.preinst b/debian/thinkfan.preinst
new file mode 100644
index 0000000..da9ed04
--- /dev/null
+++ b/debian/thinkfan.preinst
@@ -0,0 +1,43 @@
+#!/bin/sh
+# preinst script for thinkfan
+#
+# see: dh_installdeb(1)
+
+set -e
+
+# summary of how this script can be called:
+#        * <new-preinst> `install'
+#        * <new-preinst> `install' <old-version>
+#        * <new-preinst> `upgrade' <old-version>
+#        * <old-preinst> `abort-upgrade' <new-version>
+# for details, see https://www.debian.org/doc/debian-policy/ or
+# the debian-policy package
+
+
+case "$1" in
+    install|upgrade)
+        if [ -n "$2" ] && dpkg --compare-versions "$2" lt "0.9.3-2~" && [ -r /etc/default/thinkfan ]; then
+            . /etc/default/thinkfan
+            if [ "${START}" != "yes" ]; then
+		# Disable the service if it was disabled via /e/d/thinkfan
+		update-rc.d thinkfan defaults >/dev/null || true
+		update-rc.d thinkfan disable >/dev/null || true
+            fi
+        fi
+    ;;
+
+    abort-upgrade)
+    ;;
+
+    *)
+        echo "preinst called with unknown argument \`$1'" >&2
+        exit 1
+    ;;
+esac
+
+# dh_installdeb will replace this with shell code automatically
+# generated by other debhelper scripts.
+
+#DEBHELPER#
+
+exit 0
-- 
2.11.0

>From a43574c2fa86fa8732ffe73b85c6a32ca8c902ec Mon Sep 17 00:00:00 2001
From: Evgeni Golov <evg...@debian.org>
Date: Sun, 12 Mar 2017 08:48:24 +0100
Subject: [PATCH 3/4] drop the old START param from the init script and
 defaults file

we can now enable and disable the service using update-rc.d
---
 debian/patches/rcscripts-drop-start-check.patch | 35 +++++++++++++++++++++++++
 debian/patches/series                           |  1 +
 2 files changed, 36 insertions(+)
 create mode 100644 debian/patches/rcscripts-drop-start-check.patch

diff --git a/debian/patches/rcscripts-drop-start-check.patch b/debian/patches/rcscripts-drop-start-check.patch
new file mode 100644
index 0000000..84d8864
--- /dev/null
+++ b/debian/patches/rcscripts-drop-start-check.patch
@@ -0,0 +1,35 @@
+diff --git a/rcscripts/thinkfan.default b/rcscripts/thinkfan.default
+index e8f43ad..dd43d31 100644
+--- a/rcscripts/thinkfan.default
++++ b/rcscripts/thinkfan.default
+@@ -1,8 +1,2 @@
+-# Should thinkfan be started automatically on boot?
+-# Only say "yes" when you know what you are doing, have configured
+-# thinkfan correctly for *YOUR* machine and loaded thinkpad_acpi
+-# with fan_control=1 (if you have a ThinkPad).
+-START=no
+-
+ # Additional startup parameters
+ DAEMON_ARGS="-q"
+diff --git a/rcscripts/thinkfan.init b/rcscripts/thinkfan.init
+index 01f74b4..6361421 100644
+--- a/rcscripts/thinkfan.init
++++ b/rcscripts/thinkfan.init
+@@ -22,7 +22,6 @@ DAEMON_ARGS="-q"
+ # This one is compiled-in, you can't change it here!
+ PIDFILE=/var/run/$NAME.pid
+ SCRIPTNAME=/etc/init.d/$NAME
+-START=no
+ 
+ # Exit if the package is not installed
+ [ -x "$DAEMON" ] || exit 0
+@@ -30,9 +29,6 @@ START=no
+ # Read configuration variable file if it is present
+ [ -r /etc/default/$NAME ] && . /etc/default/$NAME
+ 
+-# Check if daemon is to be started
+-[ "$START" = "yes" ] || exit 0
+-
+ # Load the VERBOSE setting and other rcS variables
+ . /lib/init/vars.sh
+ 
diff --git a/debian/patches/series b/debian/patches/series
index 055d67b..37c0495 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1 +1,2 @@
 cmake-man.patch
+rcscripts-drop-start-check.patch
-- 
2.11.0

>From a0e26625fb5d41a65e41b61d7ad7f2ce18a4f2d8 Mon Sep 17 00:00:00 2001
From: Evgeni Golov <evg...@debian.org>
Date: Sun, 12 Mar 2017 09:01:50 +0100
Subject: [PATCH 4/4] update docs on enabling the service under sysvinit

---
 debian/NEWS          | 11 +++++++++++
 debian/README.Debian |  9 +++++----
 2 files changed, 16 insertions(+), 4 deletions(-)
 create mode 100644 debian/NEWS

diff --git a/debian/NEWS b/debian/NEWS
new file mode 100644
index 0000000..6dbc4ab
--- /dev/null
+++ b/debian/NEWS
@@ -0,0 +1,11 @@
+thinkfan (0.9.3-2) unstable; urgency=medium
+
+    On systems using SysV init, thinkfan is now enabled and disabled
+    using     update-rc.d and not via the obsolete START=(yes|no) flag
+    in /etc/default/thinkfan. During the upgrade the state from the
+    file is migrated automatically.
+
+    Please refer to README.Debian how to manually manage the service.
+
+ -- Evgeni Golov <evg...@debian.org>  Sun, 12 Mar 2017 09:01:50 +0100
+
diff --git a/debian/README.Debian b/debian/README.Debian
index 2450d4d..47ee31b 100644
--- a/debian/README.Debian
+++ b/debian/README.Debian
@@ -5,10 +5,11 @@ WARNING!
  thinkfan CAN kill your system and is thus disabled by default
 
 If you really want to enable thinkfan, adjust /etc/thinkfan.conf to your
-needs and set START=yes in /etc/default/thinkfan.
+needs and then enable thinkfan in your init system.
 
-If you are using systemd, use “systemctl enable thinkfan.service” — as is
-customary with systemd, thinkfan.service ignores /etc/default/thinkfan.
+If you are using SysV init, use “update-rc.d thinkfan enable”.
+
+If you are using systemd, use “systemctl enable thinkfan.service”.
 
 Additionally, on ThinkPads, you need to load the thinkpad_acpi module with
 fan_control=1, or it will refuse to accept control from thinkfan.
@@ -16,4 +17,4 @@ That's most easily done with a file /etc/modprobe.d/thinkfan.conf and the
 following entry in it:
 options thinkpad_acpi fan_control=1
 
- -- Evgeni Golov <evg...@debian.org>  Mon, 09 Nov 2009 11:53:11 +0100
+ -- Evgeni Golov <evg...@debian.org>  Sun, 12 Mar 2017 09:01:50 +0100
-- 
2.11.0

Reply via email to