On Wed, Jan 25, 2017 at 12:45:19PM +0200, Otto Kekäläinen wrote:
> I am happy to accept patches though for this issue if somebody else
> can better reproduce the problem and then verify that the patches work
> when written.
There is the major difficulty of fixing a broken package's postrm once
the package is no longer installed.
I think the only way to fix this retrospectively, then, is to edit the
other package's postrm script in the postinst. This is generally bad
form, but this is the same package series maintained by the same
maintainers, so it is probably not a disaster. (I couldn't find
anything about this in Debian Policy.) It is certainly a really good
thing to fix it moving forwards.
Also, the same issue partly applies (I've just noticed) to some of the
mysql-server-* packages. mysql-server-5.5 has it right: the
update-rc.d remove call is protected, but mysql-server-5.6 has it
wrong again, and also has other problematic parts. The changes which
I suggest in my attached patch could presumably be applied there too,
but I haven't tested that. (For the mysql-server-5.[67] packages,
debhelper would also have to be prevented from automatically including
most of the postrm snippets, and they would instead have to be
included manually, protected to only run when this was the last
instance of a mysql server.)
I have tested the attached patch on a machine which had the following
upgrade path:
mysql-server-5.5 -> mysql-server-5.6 -> mariadb-server-10.0 ->
mariadb-server-10.1 (10.1.20-3) -> mariadb-server-10.1 (10.1.21-1.1
with my patch installed)
then purging all of the old servers (the first three in the list
above) did not cause a problem. (And wow, building mariadb-10.1 takes
ages!)
The attached patch is against 10.1.21-1. The changes are as follows
(a couple of which are not necessary for this bug, but tidy up the
scripts or fix other small bugs); the ones with a double star are
essential for this bugfix:
debian/changelog
* add new entry for this patch
debian/mariadb-server-10.1.lintian-overrides
* update to match rest of patch
debian/mariadb-server-10.1.postinst
* remove the invoke() function, just call invoke-rc.d directly
(rather than "if [ -x ... ]", as invoke-rc.d is now in an essential
package, and has been essential for a while)
* move the code from after the "case "$1" in ... block to within the
configure part of it, as it should have been anyway
** add code (somewhat tricky, modify with great care!) to mangle the
postrm of any remaining old server packages; at present, it will
touch any mysql-server-5.*.postrm, including any that aren't yet
in Debian. If mysql-server-5.8 is fixed, then this could be
restricted to only touching mysql-server-5.[1-7].postrm and
mariadb-server-10.0.postrm
debian/mariadb-server-10.1.postrm
** remove stop_server code completely; *our* server is not running by
the time this script is called (our server was stopped in the
prerm), and if a mysqld process is running, it belongs to a
different mysql-server package (this partly prevents the bug from
occurring in future packages)
* fix a typo (sever -> server) ;-)
** include the relevant debhelper snippets manually where appropriate
in the postrm (so the update-rc.d snippet is protected and only
run if this is the final mysql server package to be purged)
** move the automatically installed #DEBHELPER# snippets after exit 0,
so that any future snippets added are obvious and can be manually
copied into the earlier part of the postrm (alternatively, the
#DEBHELPER# could just be removed, but this might lead to
unexpected bugs in the future if changes in debhelper are not
noticed)
debian/mariadb-server-10.1.preinst
* call invoke-rc.d directly
I hope this is of significant benefit to all!
Julian
diff -Nru debian-10.1.21-1/debian/changelog debian-10.1.21-1.1/debian/changelog
--- debian-10.1.21-1/debian/changelog 2017-01-19 09:33:01.000000000 +0000
+++ debian-10.1.21-1.1/debian/changelog 2017-01-25 12:28:41.000000000 +0000
@@ -1,3 +1,10 @@
+mariadb-10.1 (10.1.21-1.1) UNRELEASED; urgency=medium
+
+ * Non-maintainer upload.
+ * Fix purge of old mariadb-server bug (Closes: #852495)
+
+ -- Julian Gilbey <[email protected]> Wed, 25 Jan 2017 12:28:41 +0000
+
mariadb-10.1 (10.1.21-1) unstable; urgency=low
[ Otto Kekäläinen ]
diff -Nru debian-10.1.21-1/debian/mariadb-server-10.1.lintian-overrides debian-10.1.21-1.1/debian/mariadb-server-10.1.lintian-overrides
--- debian-10.1.21-1/debian/mariadb-server-10.1.lintian-overrides 2016-12-20 20:58:40.000000000 +0000
+++ debian-10.1.21-1.1/debian/mariadb-server-10.1.lintian-overrides 2017-01-25 17:07:28.956664190 +0000
@@ -1,9 +1,7 @@
# ash's buildin has no "-e" so use /bin/echo
-mariadb-server-10.1: command-with-path-in-maintainer-script postinst:154 /bin/echo
-# OK, path /usr/sbin/invoke-rc.d is only used in check, executes are run without the path
-mariadb-server-10.1: command-with-path-in-maintainer-script postinst:17 /usr/sbin/invoke-rc.d
-mariadb-server-10.1: command-with-path-in-maintainer-script postrm:15 /usr/sbin/invoke-rc.d
-mariadb-server-10.1: command-with-path-in-maintainer-script preinst:27 /usr/sbin/invoke-rc.d
+mariadb-server-10.1: command-with-path-in-maintainer-script postinst:145 /bin/echo
+# the second update-rc.d is informational only: see postrm for details
+mariadb-server-10.1: duplicate-updaterc.d-calls-in-postrm mysql
# False positive: unfortified calls have already been fully validated at compile-time
# See full research at https://jira.mariadb.org/browse/MDEV-8377
mariadb-server-10.1: hardening-no-fortify-functions usr/lib/*/mariadb18/plugin/auth_pam.so
diff -Nru debian-10.1.21-1/debian/mariadb-server-10.1.postinst debian-10.1.21-1.1/debian/mariadb-server-10.1.postinst
--- debian-10.1.21-1/debian/mariadb-server-10.1.postinst 2017-01-19 09:33:01.000000000 +0000
+++ debian-10.1.21-1.1/debian/mariadb-server-10.1.postinst 2017-01-25 17:23:08.068768679 +0000
@@ -13,21 +13,15 @@
# the install, rather than failing silently and leaving a broken install.
set -o pipefail
-invoke() {
- if [ -x /usr/sbin/invoke-rc.d ]; then
- invoke-rc.d mysql $1
- else
- /etc/init.d/mysql $1
- fi
-}
-
MYSQL_BOOTSTRAP="/usr/sbin/mysqld --bootstrap --user=mysql --disable-log-bin --skip-grant-tables --default-storage-engine=myisam"
# This is necessary because mysql_install_db removes the pid file in /var/run
# and because changed configuration options should take effect immediately.
# In case the server wasn't running at all it should be ok if the stop
# script fails. I can't tell at this point because of the cleaned /var/run.
-set +e; invoke stop; set -e
+set +e
+invoke-rc.d mysql stop
+set -e
case "$1" in
configure)
@@ -157,6 +151,28 @@
# NOTE: $MYSQL_BOOTSTRAP requires one SQL statement per line, semicolon at the end.
echo "$password_column_fix_query" | $MYSQL_BOOTSTRAP 2>&1 | $ERR_LOGGER
+ db_stop # in case invoke failes
+
+ # If we upgrade from MySQL mysql.service may be masked, which also
+ # means init.d script is disabled. Unmask mysql service explicitely.
+ # Check first that the command exists, to avoid emitting any warning messages.
+ if [ -x "$(command -v deb-systemd-helper)" ]; then
+ deb-systemd-helper unmask mysql.service > /dev/null
+ fi
+
+ # Fix broken postrms in mysql-server-5.* and mariadb-server-10.0 packages
+ # to prevent purging these packages from breaking our package. (See #852495)
+ # We comment out all of the commands which assume that there is no other
+ # mysql server installed.
+ # (Because of the Conflicts in the control file for this package, they can
+ # only possibly be in a configuration-only state at this point. And this
+ # cannot harm even if the system is in a very broken state and we are being
+ # configured in spite of those packages being in a different state.)
+ olds=$(ls /var/lib/dpkg/info/mysql-server-5.*.postrm /var/lib/dpkg/info/mariadb-server-10.0.postrm 2>/dev/null)
+ if [ -n "$olds" ]; then
+ perl -i -pe 's/stop_server(?=\s|$)/# stop_server/; s/^(\s*)((?:update|invoke)-rc\.d.*$)/$1# $2\n$1true/; s/^(\s*)(deb-systemd-helper.*$)/$1# $2\n$1true/; s%rm -f "/etc/apparmor.d%# rm -f "/etc/apparmor.d%' $olds
+ fi
+
;;
abort-upgrade|abort-remove|abort-configure)
@@ -168,15 +184,6 @@
;;
esac
-db_stop # in case invoke failes
-
-# If we upgrade from MySQL mysql.service may be masked, which also
-# means init.d script is disabled. Unmask mysql service explicitely.
-# Check first that the command exists, to avoid emitting any warning messages.
-if [ -x "$(command -v deb-systemd-helper)" ]; then
- deb-systemd-helper unmask mysql.service > /dev/null
-fi
-
#DEBHELPER#
exit 0
diff -Nru debian-10.1.21-1/debian/mariadb-server-10.1.postrm debian-10.1.21-1.1/debian/mariadb-server-10.1.postrm
--- debian-10.1.21-1/debian/mariadb-server-10.1.postrm 2016-12-20 20:58:40.000000000 +0000
+++ debian-10.1.21-1.1/debian/mariadb-server-10.1.postrm 2017-01-25 12:28:41.000000000 +0000
@@ -5,34 +5,10 @@
if [ -n "$DEBIAN_SCRIPT_DEBUG" ]; then set -v -x; DEBIAN_SCRIPT_TRACE=1; fi
${DEBIAN_SCRIPT_TRACE:+ echo "#42#DEBUG# RUNNING $0 $*" 1>&2 }
-MYADMIN="/usr/bin/mysqladmin --defaults-file=/etc/mysql/debian.cnf"
-
-# Try to stop the server in a sane way. If it does not success let the admin
-# do it himself. No database directories should be removed while the server
-# is running!
-stop_server() {
- set +e
- if [ -x /usr/sbin/invoke-rc.d ]; then
- invoke-rc.d mysql stop
- else
- /etc/init.d/mysql stop
- fi
- errno=$?
- set -e
-
- if [ "$?" != 0 ]; then
- echo "Trying to stop the MySQL server resulted in exitcode $?." 1>&2
- echo "Stop it yourself and try again!" 1>&2
- exit 1
- fi
-}
-
case "$1" in
purge|remove|upgrade|failed-upgrade|abort-install|abort-upgrade|disappear)
- if [ -n "`$MYADMIN ping 2>/dev/null`" ]; then
- stop_server
- sleep 2
- fi
+ # We handle purge actions below. The server has already been stopped
+ # if necessary by the prerm.
;;
*)
echo "postrm called with unknown argument '$1'" 1>&2
@@ -41,7 +17,7 @@
esac
#
-# - Do NOT purge logs or data if another mysql-sever* package is installed (#307473)
+# - Do NOT purge logs or data if another mysql-server* package is installed (#307473)
# - Remove the mysql user only after all his owned files are purged.
#
if [ "$1" = "purge" -a ! \( -x /usr/sbin/mysqld -o -L /usr/sbin/mysqld \) ]; then
@@ -71,8 +47,34 @@
userdel mysql || true
fi
+ # The following would have been automatically added by dh_installinit
+ # It is placed here to ensure that it is only run when there is no other
+ # mysql server package running. (See #852495)
+ if [ "$1" = "purge" ] ; then
+ update-rc.d mysql remove >/dev/null
+ fi
+
fi
-#DEBHELPER#
+# The following would have been automatically added by dh_installdebconf
+if [ "$1" = purge ] && [ -e /usr/share/debconf/confmodule ]; then
+ . /usr/share/debconf/confmodule
+ db_purge
+fi
+# The following would have been automatically added by dh_installinit
+# and is safe to run in any event
+# In case this system is running systemd, we make systemd reload the unit files
+# to pick up changes.
+if [ -d /run/systemd/system ] ; then
+ systemctl --system daemon-reload >/dev/null || true
+fi
+# End what would have been automatically added by dh_installinit
+
exit 0
+
+# We add the debhelper snippets here, after the exit 0, so that they can
+# be visually checked in the future to ensure that nothing has been left
+# out above.
+
+#DEBHELPER#
diff -Nru debian-10.1.21-1/debian/mariadb-server-10.1.preinst debian-10.1.21-1.1/debian/mariadb-server-10.1.preinst
--- debian-10.1.21-1/debian/mariadb-server-10.1.preinst 2017-01-15 22:28:37.000000000 +0000
+++ debian-10.1.21-1.1/debian/mariadb-server-10.1.preinst 2017-01-25 12:28:41.000000000 +0000
@@ -24,12 +24,7 @@
if [ ! -x /etc/init.d/mysql ]; then return; fi
set +e
- if [ -x /usr/sbin/invoke-rc.d ]; then
- cmd="invoke-rc.d mysql stop"
- else
- cmd="/etc/init.d/mysql stop"
- fi
- $cmd
+ invoke-rc.d mysql stop
errno=$?
set -e