Control: tags -1 + patch Hello!
Please see the attached patch which updates the packaging to ship the upstream systemd unit file included in the sources. While doing so I also installed the upstream enviroment file and added conversion from the old format previously used by the package. Potential improvement would be to also auto- convert OPTIONS to IRQBALANCE_ARGS. (The init script takes both into consideration but upstream/systemd unit only takes IRQBALANCE_ARGS into consideration currently.) While doing this I also got rid of the "ENABLED in /etc/default/foo" anti-pattern, which was quite more complicated than expected because of debconf prompting for enable/disabled state. The maintainer script would be much less complicated if removing this debconf prompt and simply relying on the maintainer running update-rc.d after the package installation, so please consider dropping debconf question for enabled. Hopefully the patch should cover all the install/upgrade/reconfigure cases, but someone might want to run this through piuparts to verify I haven't screwed up my manual testing. Both under systemd and sysvinit. Both with ENABLE=1 or 0 or update-rc.d irqbalance enable or disable. Regards, Andreas Henriksson
diff -Nru irqbalance-1.1.0/debian/changelog irqbalance-1.1.0/debian/changelog --- irqbalance-1.1.0/debian/changelog 2016-01-09 11:24:25.000000000 +0100 +++ irqbalance-1.1.0/debian/changelog 2016-06-14 17:44:46.000000000 +0200 @@ -1,3 +1,33 @@ +irqbalance (1.1.0-2.1) UNRELEASED; urgency=medium + + * Non-maintainer upload. + * Ship upstreams misc/irqbalance.env as /etc/default/irqbalance + - this means it'll automatically become a conffile (Closes: #710942) + * Add dh-exec build-dependency used in debian/irqbalance.install + for the above file rename. + * Override dh_clean in debian/rules to make debian/irqbalance.install + executable as needed when using dh-exec. + * Add debian/irqbalance.preinst to save existing /etc/default/irqbalance + settings on upgrades to this version which ships it as a conffile. + * Modify debian/irqbalance.config to take old saved default file into + consideration when preseeding the debconf answers. + * Modify debian/irqbalance.postinst to convert the saved default file + and append any remaining parts of it to the new /etc/default/irqbalance + if needed. + * Add dh-systemd build-dependency an use --with systemd in debian/rules + * Ship upstreams misc/irqbalance.service (Closes: #803232) + * Add debian/patches/debian-systemd-service-adaptions.patch + - set EnvironmentFile path to /etc/default/irqbalance + * debian/irqbalance.init: Drop ENABLED anti-pattern and update for + conversion to use upstream environment variable names. + * Create flag files in debian/irqbalance.preinst to tell + if we're upgrading or installing, since debian/irqbalance.config has + no way to tell by itself. + * Add debian/irqbalance.lintian-overrides to silence "duplicate update-rc.d + calls" lintian warning until dh_installinit support --no-enable. + + -- Andreas Henriksson <andr...@fatal.se> Tue, 14 Jun 2016 17:44:40 +0200 + irqbalance (1.1.0-2) unstable; urgency=medium * Fix FTBFS on arm64 diff -Nru irqbalance-1.1.0/debian/control irqbalance-1.1.0/debian/control --- irqbalance-1.1.0/debian/control 2015-10-21 13:03:40.000000000 +0200 +++ irqbalance-1.1.0/debian/control 2016-06-14 11:03:05.000000000 +0200 @@ -2,7 +2,7 @@ Section: utils Priority: extra Maintainer: Anibal Monsalve Salazar <ani...@debian.org> -Build-Depends: dpkg-dev (>= 1.16.1~), debhelper (>= 9.20130604), pkg-config, libglib2.0-dev (>= 2.28), xutils-dev, libcap-ng-dev, libnuma-dev [!hurd-any !kfreebsd-any !armel !armhf !s390x] +Build-Depends: dpkg-dev (>= 1.16.1~), debhelper (>= 9.20130604), dh-systemd (>= 1.5), dh-exec, pkg-config, libglib2.0-dev (>= 2.28), xutils-dev, libcap-ng-dev, libnuma-dev [!hurd-any !kfreebsd-any !armel !armhf !s390x] Standards-Version: 3.9.6 Homepage: https://github.com/Irqbalance/irqbalance diff -Nru irqbalance-1.1.0/debian/irqbalance.config irqbalance-1.1.0/debian/irqbalance.config --- irqbalance-1.1.0/debian/irqbalance.config 2012-04-20 04:39:38.000000000 +0200 +++ irqbalance-1.1.0/debian/irqbalance.config 2016-06-14 17:38:11.000000000 +0200 @@ -6,16 +6,56 @@ db_version 2.0 CONF=/etc/default/irqbalance +CONFCONVERT=/etc/default/irqbalance.dpkg-needs-convert +# config has no way to detect upgrade vs fresh installs, +# so preinst hands us a flag files. +UPGRADE_FLAG_FILE=/run/irqbalance.dpkg-upgrade +INSTALL_FLAG_FILE=/run/irqbalance.dpkg-install -if test -e $CONF; then - . $CONF || true - - if [ "$ENABLED" = "1" ]; then - db_set irqbalance/enable true +is_irqbalance_enabled() { + # See https://bugs.debian.org/705254 but lets try ourselves for now... + if [ -e /run/systemd/system ]; then + if systemctl -q is-enabled irqbalance.service; then + return 0 + else + return 1 + fi else + if test -e /etc/rc*.d/S*irqbalance; then + return 0 + else + return 1 + fi + fi +} + +if test -e $CONF || test -e $CONFCONVERT; then + test -e $CONF && . $CONF || true + test -e $CONFCONVERT && . $CONFCONVERT || true + + # ENABLED is the old format up for conversion, + # will be switched to update-rc.d handling in postinst... + if [ "$ENABLED" = "0" ]; then db_set irqbalance/enable false + elif [ -e $UPGRADE_FLAG_FILE ] && ! is_irqbalance_enabled; then + db_set irqbalance/enable false + elif [ ! -e $INSTALL_FLAG_FILE ] && [ ! -e $UPGRADE_FLAG_FILE ]; then + # dpkg-reconfigure + if is_irqbalance_enabled; then + db_set irqbalance/enable true + else + db_set irqbalance/enable false + fi + else + db_set irqbalance/enable true fi - if [ "$ONESHOT" = "1" ]; then + # We no longer need flag files, clean up.... + rm -f $UPGRADE_FLAG_FILE $INSTALL_FLAG_FILE + + # ONESHOT is the old format used before conversion. + # Note: irqbalance.c treats IRQBALANCE_ONESHOT as active if set to + # anything (even empty string). + if [ "$ONESHOT" = "1" ] || [ ! -z ${IRQBALANCE_ONESHOT+x} ]; then db_set irqbalance/oneshot true else db_set irqbalance/oneshot false diff -Nru irqbalance-1.1.0/debian/irqbalance.init irqbalance-1.1.0/debian/irqbalance.init --- irqbalance-1.1.0/debian/irqbalance.init 2014-09-10 03:07:43.000000000 +0200 +++ irqbalance-1.1.0/debian/irqbalance.init 2016-06-14 12:54:41.000000000 +0200 @@ -23,7 +23,6 @@ DOPTIONS="" # Defaults - don't touch, edit /etc/default/ -ENABLED=0 OPTIONS="" ONESHOT=0 @@ -33,11 +32,20 @@ test -f /etc/default/irqbalance && . /etc/default/irqbalance -test "$ENABLED" != "0" || exit 0 - -if test "$ONESHOT" != "0"; then +# Beware: irqbalance tries to read and handle environment variables +# directly itself, but since start-stop-daemon clears the env +# we convert the variables to commandline arguments here... +# (Note: in the daemon an option is enabled even if its set to +# e.g. the empty string or 0 or whatever. To disable it should not +# be exported at all!) +# Warning: this will need to be maintained and updated on upgrades +# to new upstream release which might introduce new ones! +if [ ! -z ${IRQBALANCE_ONESHOT+x} ]; then DOPTIONS="--oneshot" fi +if [ ! -z ${IRCBALANCE_ARGS+x} ]; then + OPTIONS="$OPTIONS $IRQBALANCE_ARGS" +fi case "$1" in start) diff -Nru irqbalance-1.1.0/debian/irqbalance.install irqbalance-1.1.0/debian/irqbalance.install --- irqbalance-1.1.0/debian/irqbalance.install 1970-01-01 01:00:00.000000000 +0100 +++ irqbalance-1.1.0/debian/irqbalance.install 2016-06-14 11:04:22.000000000 +0200 @@ -0,0 +1,3 @@ +#!/usr/bin/dh-exec +misc/irqbalance.env => /etc/default/irqbalance +misc/irqbalance.service /lib/systemd/system diff -Nru irqbalance-1.1.0/debian/irqbalance.lintian-overrides irqbalance-1.1.0/debian/irqbalance.lintian-overrides --- irqbalance-1.1.0/debian/irqbalance.lintian-overrides 1970-01-01 01:00:00.000000000 +0100 +++ irqbalance-1.1.0/debian/irqbalance.lintian-overrides 2016-06-14 17:43:17.000000000 +0200 @@ -0,0 +1,6 @@ +# We don't have a better way to write the maintainer script +# without completely disabling autogenerated parts from +# dh_systemd_enable / dh_installinit / dh_systemd_start +# Ideally dh_installinit would support --no-enable so we +# could use it on all three.... See https://bugs.debian.org/709384 +duplicate-updaterc.d-calls-in-postinst irqbalance diff -Nru irqbalance-1.1.0/debian/irqbalance.postinst irqbalance-1.1.0/debian/irqbalance.postinst --- irqbalance-1.1.0/debian/irqbalance.postinst 2012-11-12 11:38:04.000000000 +0100 +++ irqbalance-1.1.0/debian/irqbalance.postinst 2016-06-14 15:53:31.000000000 +0200 @@ -6,40 +6,55 @@ db_version 2.0 CONF=/etc/default/irqbalance +CONFCONVERT=/etc/default/irqbalance.dpkg-needs-convert if [ "$1" = "configure" ]; then if [ "$2" != "" ] && dpkg --compare-versions "$2" lt "1.0.5-1"; then update-rc.d -f irqbalance remove >/dev/null fi - if [ ! -e $CONF ]; then - echo "#Configuration for the irqbalance daemon" > $CONF.tmp - echo >> $CONF.tmp - echo "#Should irqbalance be enabled?" >> $CONF.tmp - echo "ENABLED=1" >> $CONF.tmp - echo "#Balance the IRQs only once?" >> $CONF.tmp - echo "ONESHOT=0" >> $CONF.tmp - - mv -f "$CONF.tmp" "$CONF" + # The debian/irqbalance.config file already takes care of + # integrating the old ENABLED and ONESHOT into the loop, but + # we still need this here to preserve any other random + # things the local sysadmin has added to the old /etc/default/irqbalance + # file to be policy compliant. + if [ -e $CONFCONVERT ]; then + # Insert a header to help sysadmin figure out why these things are here. + sed -i '1 i\# This file was modified on irqbalance 1.1.0-2.1 pkg upgrade. Preserved settings:' $CONFCONVERT + + # Drop the ENABLED and ONESHOT settings, already handled via debconf. + sed -i -e '/ *ENABLED=.*/d' -e '/ *ONESHOT=.*/d' $CONFCONVERT + + # Check if we have anything remaining except comments, if so + # we append the old file to the new one. + # This is to preserve any random things that might have + # been added to the file by the local sysadmin. + if grep -qv '^$\|^\s*#' $CONFCONVERT; then + cat $CONFCONVERT >> $CONF + fi + rm -f $CONFCONVERT fi db_get irqbalance/enable + update-rc.d irqbalance defaults if [ "$RET" = "true" ]; then - ENABLE="1" + update-rc.d irqbalance enable else - ENABLE="0" + update-rc.d irqbalance disable fi db_get irqbalance/oneshot if [ "$RET" = "true" ]; then - ONESHOT="1" + # Modify existing line to enable or add a new one. + if grep -q '^\s*#*\s*IRQBALANCE_ONESHOT\s*=' $CONF ; then + sed -i 's/^\s*#*\s*IRQBALANCE_ONESHOT\s*=.*/IRQBALANCE_ONESHOT=yes/' $CONF + else + # Disable by commenting any existing line + # (or ignore relying on default disabled). + sed -i 's/^\s*IRQBALANCE_ONESHOT\s*=/#IRQBALANCE_ONESHOT=/' $CONF + fi else - ONESHOT="0" + sed -i 's/^\s*IRQBALANCE_ONESHOT=/#IRQBALANCE_ONESHOT=/' $CONF fi - cp -a -f $CONF $CONF.tmp - sed -e "s/^ *ENABLED=.*/ENABLED=\"$ENABLE\"/" \ - -e "s/^ *ONESHOT=.*/ONESHOT=\"$ONESHOT\"/" \ - < $CONF > $CONF.tmp - mv -f $CONF.tmp $CONF db_stop fi diff -Nru irqbalance-1.1.0/debian/irqbalance.preinst irqbalance-1.1.0/debian/irqbalance.preinst --- irqbalance-1.1.0/debian/irqbalance.preinst 1970-01-01 01:00:00.000000000 +0100 +++ irqbalance-1.1.0/debian/irqbalance.preinst 2016-06-14 17:27:14.000000000 +0200 @@ -0,0 +1,29 @@ +#!/bin/sh + +set -e + +CONF=/etc/default/irqbalance +CONFCONVERT=/etc/default/irqbalance.dpkg-needs-convert +UPGRADE_FLAG_FILE=/run/irqbalance.dpkg-upgrade +INSTALL_FLAG_FILE=/run/irqbalance.dpkg-install + +# tell irqbalance.config if we're upgrading or installing since it can't tell. +if [ "$1" = "upgrade" ]; then + touch $UPGRADE_FLAG_FILE +fi +if [ "$1" = "install" ]; then + touch $INSTALL_FLAG_FILE +fi + +# the old file once generated by irqbalance.postinst and thus not a +# proper conffile needs to be taken into consideration here to avoid +# blatantly overwriting it on upgrades. +# (We'll do the actuan conversion in config/postinst.) +if [ "$1" = "upgrade" ] && [ "$2" != "" ] && \ + [ -e /etc/default/irqbalance ] && \ + dpkg --compare-versions "$2" lt "1.1.0-2.1~" +then + mv $CONF $CONFCONVERT +fi + +#DEBHELPER# diff -Nru irqbalance-1.1.0/debian/patches/debian-systemd-service-adaptions.patch irqbalance-1.1.0/debian/patches/debian-systemd-service-adaptions.patch --- irqbalance-1.1.0/debian/patches/debian-systemd-service-adaptions.patch 1970-01-01 01:00:00.000000000 +0100 +++ irqbalance-1.1.0/debian/patches/debian-systemd-service-adaptions.patch 2016-06-14 12:16:34.000000000 +0200 @@ -0,0 +1,33 @@ +From: Andreas Henriksson <andr...@fatal.se> +Subject: Debian adaptions of the upstream systemd unit file + +Forwarded: not-needed + +Drop the After=syslog.target as that target is obsolete and lintian +warns about using it since syslog is now socket activated when needed. + +Change the path to the environment file to match Debian defacto standard +and make it optional by prepending a -. + +Explicitly set Restart=no (the default) and add a comment about why. +It would be better if oneshot was handled via Type=oneshot when +that mode is desired, so that other modes could restart the daemon +on failure/abort/whatever. + +--- a/misc/irqbalance.service ++++ b/misc/irqbalance.service +@@ -1,10 +1,12 @@ + [Unit] + Description=irqbalance daemon +-After=syslog.target + + [Service] +-EnvironmentFile=/path/to/irqbalance.env ++EnvironmentFile=-/etc/default/irqbalance + ExecStart=/usr/sbin/irqbalance --foreground $IRQBALANCE_ARGS ++# If IRQBALANCE_ONESHOT environment is set, the service will exit so: ++Restart=no ++# (The above case would be better as a real Type=oneshot unit.) + + [Install] + WantedBy=multi-user.target diff -Nru irqbalance-1.1.0/debian/patches/series irqbalance-1.1.0/debian/patches/series --- irqbalance-1.1.0/debian/patches/series 2016-01-09 11:18:36.000000000 +0100 +++ irqbalance-1.1.0/debian/patches/series 2016-06-14 11:52:49.000000000 +0200 @@ -1,2 +1,3 @@ irqbalance.1.patch arm64.patch +debian-systemd-service-adaptions.patch diff -Nru irqbalance-1.1.0/debian/rules irqbalance-1.1.0/debian/rules --- irqbalance-1.1.0/debian/rules 2014-09-10 02:53:25.000000000 +0200 +++ irqbalance-1.1.0/debian/rules 2016-06-14 11:07:33.000000000 +0200 @@ -8,4 +8,9 @@ include /usr/share/dpkg/buildflags.mk %: - dh $@ + dh $@ --with systemd + +override_dh_clean: + # uses dh-exec so needs to be executable + chmod +x debian/irqbalance.install + dh_clean
