launchpad-reviewers team mailing list archive

[Steve Langasek <steve.langasek@xxxxxxxxxxxxx>]

Steve Langasek has proposed merging lp:~vorlon/maas/maintscript-style into lp:~maas-maintainers/maas/packaging.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~vorlon/maas/maintscript-style/+merge/154234


-- 
https://code.launchpad.net/~vorlon/maas/maintscript-style/+merge/154234
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~vorlon/maas/maintscript-style into lp:~maas-maintainers/maas/packaging.

=== modified file 'debian/changelog'
--- debian/changelog	2013-03-19 19:43:49 +0000
+++ debian/changelog	2013-03-20 01:26:21 +0000
@@ -2,6 +2,33 @@
 
   * UNRELEASED
 
+  [ Steve Langasek ]
+  * postinst scripts are never called with 'reconfigure' as the script
+    argument.  Remove references to this (mythical) invocation.
+  * always call 'set -e' from maintainer scripts instead of passing 'sh -e'
+    as the interpreter, so that scripts will behave correctly when run via
+    'sh -x'.
+  * invoke-rc.d is never allowed to not exist - simplify scripts (and make
+    them better policy-compliant) by invoking unconditionally.  (The only
+    possible exception is in the postrm, where it's *theoretically* possible
+    for invoke-rc.d to be missing if the user has completely stripped
+    down their system; that's a fairly unreasonable corner case, but we
+    might as well be correct if it ever happens.)
+  * db_get+db_set is a no-op; don't call db_set to push back a value we just
+    got from db_get.
+  * Omit superfluous calls to 'exit 0' at the end of each script.
+  * Remove maas-cluster-controller prerm script, which called debconf for no
+    reason.
+  * Don't invoke debconf in the postrm script either, debhelper already does
+    this for us.
+  * Other miscellaneous maintainer script fixes
+  * debian/maas-common.postinst: call adduser and addgroup unconditionally;
+    the tools are already designed to DTRT, we don't need to check for the
+    user/group existence before calling them nor should we worry about
+    calling them only once on first install.
+  * debian/maas-common.postrm: delete the maas group, not just the user,
+    as the comment in the code implies we should do.
+
  -- Andres Rodriguez <andreserl@xxxxxxxxxx>  Tue, 19 Mar 2013 15:38:22 -0400
 
 maas (1.3+bzr1452+dfsg-0ubuntu1) raring; urgency=low

=== modified file 'debian/maas-cluster-controller.config'
--- debian/maas-cluster-controller.config	2012-11-13 20:40:07 +0000
+++ debian/maas-cluster-controller.config	2013-03-20 01:26:21 +0000
@@ -1,4 +1,6 @@
-#!/bin/sh -e
+#!/bin/sh
+
+set -e
 
 . /usr/share/debconf/confmodule
 db_version 2.0
@@ -8,9 +10,7 @@
 if ([ "$1" = "configure" ] && [ -z "$2" ]); then
 
     db_get maas-cluster-controller/maas-url || true
-    if [ -n "$RET" ]; then
-        db_set maas-cluster-controller/maas-url "$RET"
-    else
+    if [ -z "$RET" ]; then
         # Attempt to pre-populate if installing on the region controller.
         if [ -e /etc/maas/maas_local_settings.py ]; then
             url=$(awk '$1 == "DEFAULT_MAAS_URL" { split($0,array,"\"")} END{print array[2] }' /etc/maas/maas_local_settings.py)

=== modified file 'debian/maas-cluster-controller.postinst'
--- debian/maas-cluster-controller.postinst	2012-12-13 10:05:52 +0000
+++ debian/maas-cluster-controller.postinst	2013-03-20 01:26:21 +0000
@@ -1,4 +1,6 @@
-#!/bin/sh -e
+#!/bin/sh
+
+set -e
 
 . /usr/share/debconf/confmodule
 db_version 2.0
@@ -72,7 +74,7 @@
     configure_maas_tgt
 fi
 
-if ([ "$1" = "configure" ] && [ -z "$2" ]) || [ "$1" = "reconfigure" ] || [ -n "$DEBCONF_RECONFIGURE" ]; then
+if ([ "$1" = "configure" ] && [ -z "$2" ]) || [ -n "$DEBCONF_RECONFIGURE" ]; then
 
     if dpkg --compare-versions "$2" lt 0.1+bzr1239+dfsg-0ubuntu1; then
         create_log_dir
@@ -104,4 +106,3 @@
 fi
 
 #DEBHELPER#
-exit 0

=== modified file 'debian/maas-cluster-controller.postrm'
--- debian/maas-cluster-controller.postrm	2012-11-06 16:41:05 +0000
+++ debian/maas-cluster-controller.postrm	2013-03-20 01:26:21 +0000
@@ -2,15 +2,12 @@
 
 set -e
 
-. /usr/share/debconf/confmodule
-db_version 2.0
+#DEBHELPER#
 
 case "$1" in
         purge)
                 # remove log directory
-                if [ -d /var/log/maas ]; then
-                    rm -rf /var/log/maas
-                fi
+                rm -rf /var/log/maas
 		# remove var directory
 		rm -rf /var/lib/maas/celerybeat-cluster-schedule
 		DIR=/var/lib/maas
@@ -24,7 +21,3 @@
 		fi
 
 esac
-
-#DEBHELPER#
-
-exit 0

=== removed file 'debian/maas-cluster-controller.prerm'
--- debian/maas-cluster-controller.prerm	2012-10-08 17:16:30 +0000
+++ debian/maas-cluster-controller.prerm	1970-01-01 00:00:00 +0000
@@ -1,10 +0,0 @@
-#!/bin/sh
-
-set -e
-
-. /usr/share/debconf/confmodule
-db_version 2.0
-
-#DEBHELPER#
-
-exit 0

=== modified file 'debian/maas-common.postinst'
--- debian/maas-common.postinst	2012-10-11 18:16:28 +0000
+++ debian/maas-common.postinst	2013-03-20 01:26:21 +0000
@@ -1,22 +1,14 @@
-#!/bin/sh -e
+#!/bin/sh
+
+set -e
 
 add_user_group(){
 	local user="maas"
 	local group="maas"
-	if ! getent group "$group" >/dev/null; then
-		addgroup --quiet --system "$group" || true
-	fi
-	if ! getent passwd "$user" > /dev/null 2>&1; then
-		adduser --quiet \
-		    --system \
-		    --group \
-		    --no-create-home \
-		    "$user" || true
-	fi
+	addgroup --quiet --system "$group" || true
+	adduser --quiet --system --group --no-create-home "$user" || true
 }
 
-if [ "$1" = "configure" ]; then
-	add_user_group
-fi
+add_user_group
 
 #DEBHELPER#

=== modified file 'debian/maas-common.postrm'
--- debian/maas-common.postrm	2012-10-11 18:16:28 +0000
+++ debian/maas-common.postrm	2013-03-20 01:26:21 +0000
@@ -7,6 +7,7 @@
                 # Deleting user/group
                 if getent passwd maas >/dev/null; then
                         deluser maas || true
+			delgroup maas || true
                 fi
 esac
 

=== modified file 'debian/maas-dhcp.postinst'
--- debian/maas-dhcp.postinst	2012-10-02 18:42:28 +0000
+++ debian/maas-dhcp.postinst	2013-03-20 01:26:21 +0000
@@ -1,22 +1,15 @@
-#!/bin/sh -e
-
-. /usr/share/debconf/confmodule
-db_version 2.0
-
-stop_isc_dhcp_server(){
-    invoke-rc.d isc-dhcp-server stop
-}
-
+#!/bin/sh
+
+set -e
 
 if [ "$1" = "configure" ]; then
-   stop_isc_dhcp_server
+   invoke-rc.d isc-dhcp-server stop
 
    dhcpd_prof="/etc/apparmor.d/usr.sbin.dhcpd"
-   if [ -f "${dhcpd_prof}" ] &&
-      command -v apparmor_parser >/dev/null 2>&1; then
+   if [ -f "${dhcpd_prof}" ] && command -v apparmor_parser >/dev/null 2>&1
+   then
       apparmor_parser --replace --write-cache --skip-read-cache "${dhcpd_prof}"
    fi
 fi
 
 #DEBHELPER#
-exit 0

=== modified file 'debian/maas-dhcp.postrm'
--- debian/maas-dhcp.postrm	2012-09-14 18:06:57 +0000
+++ debian/maas-dhcp.postrm	2013-03-20 01:26:21 +0000
@@ -1,6 +1,8 @@
-#!/bin/sh -e
-
-if [ "$1" = "remove" -o "$1" = "purge" ]; then
+#!/bin/sh
+
+set -e
+
+if [ "$1" = "remove" ] || [ "$1" = "purge" ]; then
    dhcpd_prof="/etc/apparmor.d/usr.sbin.dhcpd"
    if [ -f "${dhcpd_prof}" ] &&
       command -v apparmor_parser >/dev/null 2>&1; then

=== modified file 'debian/maas-dns.postinst'
--- debian/maas-dns.postinst	2012-12-03 13:13:07 +0000
+++ debian/maas-dns.postinst	2013-03-20 01:26:21 +0000
@@ -1,6 +1,8 @@
-#!/bin/sh -e
-
-if ([ "$1" = "configure" ] && [ -z "$2" ]) || [ "$1" = "reconfigure" ] || [ -n "$DEBCONF_RECONFIGURE" ]; then
+#!/bin/sh
+
+set -e
+
+if ([ "$1" = "configure" ] && [ -z "$2" ]) || [ -n "$DEBCONF_RECONFIGURE" ]; then
     # If /etc/bind/maas is empty, set_up_dns.
     if [ ! "$(ls -A /etc/bind/maas)" ]; then
         maas set_up_dns
@@ -28,10 +30,7 @@
 
     maas write_dns_config
 
-    if [ -x /usr/sbin/invoke-rc.d ]; then
-        invoke-rc.d bind9 restart || true
-    fi
+    invoke-rc.d bind9 restart || true
 fi
 
 #DEBHELPER#
-exit 0

=== modified file 'debian/maas-dns.postrm'
--- debian/maas-dns.postrm	2012-08-10 23:50:38 +0000
+++ debian/maas-dns.postrm	2013-03-20 01:26:21 +0000
@@ -1,4 +1,6 @@
-#!/bin/sh -e
+#!/bin/sh
+
+set -e
 
 if [ "$1" = "remove" ]; then
 	if [ -f /etc/bind/named.conf.local ]; then
@@ -19,6 +21,3 @@
 fi
 
 #DEBHELPER#
-
-exit 0
-

=== modified file 'debian/maas-region-controller.config'
--- debian/maas-region-controller.config	2012-10-11 18:50:41 +0000
+++ debian/maas-region-controller.config	2013-03-20 01:26:21 +0000
@@ -1,4 +1,6 @@
-#!/bin/sh -e
+#!/bin/sh
+
+set -e
 
 . /usr/share/debconf/confmodule
 db_version 2.0
@@ -21,7 +23,7 @@
 	. /usr/share/dbconfig-common/dpkg/config.pgsql
 fi
 
-if ([ "$1" = "configure" ] && [ -z "$2" ]); then
+if [ "$1" = "configure" ] && [ -z "$2" ]; then
 	# Hide maas/dbconfig-install question by setting default.
 	set_question maas-region-controller/dbconfig-install true
 	set_question maas-region-controller/pgsql/app-pass ""
@@ -34,9 +36,7 @@
 
 elif [ "$1" = "reconfigure" ] || [ -n "$DEBCONF_RECONFIGURE" ]; then
 	db_get maas/default-maas-url || true
-	if [ -n "$RET" ]; then
-		db_set maas/default-maas-url "$RET"
-	else
+	if [ -z "$RET" ]; then
 		ipaddr=$(awk '$1 == "DEFAULT_MAAS_URL" { split($0,array,"/")} END{print array[3] }' /etc/maas/maas_local_settings.py)
 		db_set maas/default-maas-url "$ipaddr"
 	fi

=== modified file 'debian/maas-region-controller.postinst'
--- debian/maas-region-controller.postinst	2012-11-06 16:41:05 +0000
+++ debian/maas-region-controller.postinst	2013-03-20 01:26:21 +0000
@@ -1,4 +1,6 @@
-#!/bin/sh -e
+#!/bin/sh
+
+set -e
 
 . /usr/share/debconf/confmodule
 db_version 2.0
@@ -13,36 +15,16 @@
 	maas migrate metadataserver --noinput
 }
 
-restart_apache2(){
-	if [ -x /usr/sbin/invoke-rc.d ]; then
-		invoke-rc.d apache2 restart || true
-	else
-		/etc/init.d/apache2 restart || true
-	fi
-}
-
 restart_rabbitmq(){
-	if [ -x /usr/sbin/invoke-rc.d ]; then
-		invoke-rc.d rabbitmq-server restart || true
-	else
-		/etc/init.d/rabbitmq-server restart || true
-	fi
+	invoke-rc.d rabbitmq-server restart || true
 }
 
 restart_postgresql(){
-	if [ -x /usr/sbin/invoke-rc.d ]; then
-		invoke-rc.d --force postgresql restart || true
-	else
-		/etc/init.d/postgresql restart || true
-	fi
+	invoke-rc.d --force postgresql restart || true
 }
 
 restart_squid_deb_proxy() {
-	if [ -x /usr/sbin/invoke-rc.d ]; then
-		invoke-rc.d squid-deb-proxy restart || true
-	else
-		/etc/init.d/squid-deb-proxy restart || true
-	fi
+	invoke-rc.d squid-deb-proxy restart || true
 }
 
 configure_maas_txlongpoll_rabbitmq_user() {
@@ -191,17 +173,13 @@
 	mkdir -p /var/log/maas/rsyslog
 	chown -R syslog:syslog /var/log/maas/rsyslog
 	# Make sure rsyslog reads our config
-	if [ -x /usr/sbin/invoke-rc.d ]; then
-		invoke-rc.d rsyslog restart
-	fi
+	invoke-rc.d rsyslog restart
 
 	#########################################################
 	################### Squid-deb-proxy  ####################
 	#########################################################
 	# Make sure squid-deb-proxy reads our config (99-maas)
-	if [ -x /usr/sbin/invoke-rc.d ]; then
-		invoke-rc.d squid-deb-proxy restart
-	fi
+	invoke-rc.d squid-deb-proxy restart
 
 	#########################################################
 	########## Configure longpoll rabbitmq config ###########
@@ -239,11 +217,10 @@
 	db_input high maas/installation-note || true
 	db_go
 
-elif [ "$1" = "reconfigure" ] || [ -n "$DEBCONF_RECONFIGURE" ]; then
+elif [ -n "$DEBCONF_RECONFIGURE" ]; then
 	# Set the IP address of the interface with default route
 	db_get maas/default-maas-url
 	ipaddr="$RET"
-	db_set maas/default-maas-url "$ipaddr"
 	if [ -n "$ipaddr" ]; then
 		configure_maas_default_url "$ipaddr"
 		configure_maas_squid_deb_proxy "$ipaddr"
@@ -252,9 +229,7 @@
 
 elif [ "$1" = "configure" ] && dpkg --compare-versions "$2" gt 0.1+bzr266+dfsg-0ubuntu1; then
 	# If upgrading to any later package version, then upgrade db.
-	if [ -x /usr/sbin/invoke-rc.d ]; then
-		invoke-rc.d apache2 stop || true
-	fi
+	invoke-rc.d apache2 stop || true
 
 	# make sure postgresql is running
 	restart_postgresql
@@ -284,12 +259,10 @@
 
 fi
 
-restart_apache2
+invoke-rc.d apache2 restart || true
 
 restart_squid_deb_proxy
 
 db_stop
 
 #DEBHELPER#
-
-exit 0

=== modified file 'debian/maas-region-controller.postrm'
--- debian/maas-region-controller.postrm	2012-11-06 16:41:05 +0000
+++ debian/maas-region-controller.postrm	2013-03-20 01:26:21 +0000
@@ -22,8 +22,6 @@
 		# Restarting apache2
 		if [ -x /usr/sbin/invoke-rc.d ]; then
 			invoke-rc.d apache2 restart || true
-		else
-			/etc/init.d/apache2 restart || true
 		fi
 
 		# Delete symlink
@@ -48,5 +46,3 @@
 #DEBHELPER#
 
 db_stop
-
-exit 0

=== modified file 'debian/maas-region-controller.prerm'
--- debian/maas-region-controller.prerm	2012-09-28 02:40:23 +0000
+++ debian/maas-region-controller.prerm	2013-03-20 01:26:21 +0000
@@ -6,13 +6,7 @@
 
 #DEBHELPER#
 
-if [ -x /usr/sbin/invoke-rc.d ]; then
-	invoke-rc.d apache2 stop || true
-else
-	/etc/init.d/apache2 stop || true
-fi
+invoke-rc.d apache2 stop || true
 
 . /usr/share/dbconfig-common/dpkg/prerm.pgsql
 dbc_go maas-region-controller $@
-
-exit 0