Bug#853008: [debian-mysql] Bug#853008: mysql-server-5.7: purge could delete mariadb-server files with inadequate warning

2018-01-31 Thread Lars Tangvald 



On 01/31/2018 02:19 PM, Olaf van der Spek wrote:

Hi,


Anyone else have any good ideas on how to handle this?

I do. The solution is quite simple: do not, ever, remove user data / databases.

It makes everything so much simpler, both on the user side and on the
dev side. No weird questions when installing, no databases gone by
accident..

The problem is that MariaDB and MySQL use the same data location, so 
you'd instead get weird errors when trying to move to a variant/version 
that can't handle the old data (MySQL does not support migration from 
MariaDB, and MariaDB supports specific versions of MySQL). It's 
reasonable for a user to expect that purging one package would let them 
install the other without issues.


There is work going on to deal with this, which should both let users go 
back and forth between incompatible versions and make sure a package can 
only delete its own data.


--
Lars

Bug#853008: mysql-server-5.7: purge could delete mariadb-server files with inadequate warning

2018-01-31 Thread Olaf van der Spek 
Hi,

> Anyone else have any good ideas on how to handle this?

I do. The solution is quite simple: do not, ever, remove user data / databases.

It makes everything so much simpler, both on the user side and on the
dev side. No weird questions when installing, no databases gone by
accident..

-- 
Olaf

Bug#853008: [debian-mysql] Bug#853008: Bug#853008: Bug#853008: mysql-server-5.7: purge could delete mariadb-server files with inadequate warning

2017-02-21 Thread Julian Gilbey 
On Tue, Feb 21, 2017 at 02:10:22PM +0100, Lars Tangvald wrote:
> On 02/21/2017 01:59 PM, Julian Gilbey wrote:
> > On Tue, Feb 21, 2017 at 01:27:44PM +0100, Lars Tangvald wrote:
> > > I've looked at it some more, and I'm hesitant about including such a big
> > > patch for a pretty rare scenario, which in the worst case does ask the
> > > following:
> > >   The /var/lib/mysql directory which contains the MySQL databases is about
> > >   to be removed.
> > >   .
> > >   This will also erase all data in /var/lib/mysql-files as well as
> > >   /var/lib/mysql-keyring.
> > >   .
> > >   If you're removing the MySQL package in order to later install a more
> > >   recent version or if a different mysql-server package is already
> > >   using it, the data should be kept.
> > > 
> > > My suggestion is that for now, we revert the patch that enabled the
> > > purge-remove (meaning postrm will never delete anything or ask to do so) 
> > > and
> > > instead work on separating the data for the different packages so that
> > > MariaDB and MySQL data isn't in the same location.
> > Hi Lars,
> > 
> > Does that mean that the whole remove section just gets removed from
> > the postrm, or that you put back the test for /usr/sbin/mysqld existing?
> > 
> > Best wishes,
> > 
> > Julian
> > 
> Only reverting the commit would put it back, which I guess would be best
> since I think it can trigger if you first do apt-get remove and then apt-get
> purge (without installing a different variant between).

As in commit 554ecf472109f2b2e2fdfa0aaa0302cf2247ac1e - yes, that
would make some sense.

However, this does not address other serious issues which are
addressed in the patch, and which will affect even users who say "No"
to the debconf purge question mentioned above:

* Purging old removed mysql-* packages stops our server, potentially
  interferes with our data and does other unwanted things (such as
  messing with our init.d scripts).

* The current postrm has some of the same issues remaining, so that
  future mysql-* packages will be damaged by purging mysql-5.7 in its
  current form.

Neither of these issues will be fixed by splitting the mysql/mariadb
data to a different location during the next release cycle, and will
potentially affect users of Debian 9.

I realise that the patch is big, which is unideal.  I wish I had
spotted the issues earlier in the release cycle, rather than now.  But
that's life.

Best wishes,

   Julian

Bug#853008: [debian-mysql] Bug#853008: Bug#853008: Bug#853008: mysql-server-5.7: purge could delete mariadb-server files with inadequate warning

2017-02-21 Thread Lars Tangvald 



On 02/21/2017 01:59 PM, Julian Gilbey wrote:

On Tue, Feb 21, 2017 at 01:27:44PM +0100, Lars Tangvald wrote:

I've looked at it some more, and I'm hesitant about including such a big
patch for a pretty rare scenario, which in the worst case does ask the
following:
  The /var/lib/mysql directory which contains the MySQL databases is about
  to be removed.
  .
  This will also erase all data in /var/lib/mysql-files as well as
  /var/lib/mysql-keyring.
  .
  If you're removing the MySQL package in order to later install a more
  recent version or if a different mysql-server package is already
  using it, the data should be kept.

My suggestion is that for now, we revert the patch that enabled the
purge-remove (meaning postrm will never delete anything or ask to do so) and
instead work on separating the data for the different packages so that
MariaDB and MySQL data isn't in the same location.

Hi Lars,

Does that mean that the whole remove section just gets removed from
the postrm, or that you put back the test for /usr/sbin/mysqld existing?

Best wishes,

Julian

Only reverting the commit would put it back, which I guess would be best 
since I think it can trigger if you first do apt-get remove and then 
apt-get purge (without installing a different variant between).


--
Lars

Bug#853008: [debian-mysql] Bug#853008: Bug#853008: Bug#853008: mysql-server-5.7: purge could delete mariadb-server files with inadequate warning

2017-02-21 Thread Julian Gilbey 
On Tue, Feb 21, 2017 at 01:27:44PM +0100, Lars Tangvald wrote:
> I've looked at it some more, and I'm hesitant about including such a big
> patch for a pretty rare scenario, which in the worst case does ask the
> following:
>  The /var/lib/mysql directory which contains the MySQL databases is about
>  to be removed.
>  .
>  This will also erase all data in /var/lib/mysql-files as well as
>  /var/lib/mysql-keyring.
>  .
>  If you're removing the MySQL package in order to later install a more
>  recent version or if a different mysql-server package is already
>  using it, the data should be kept.
> 
> My suggestion is that for now, we revert the patch that enabled the
> purge-remove (meaning postrm will never delete anything or ask to do so) and
> instead work on separating the data for the different packages so that
> MariaDB and MySQL data isn't in the same location.

Hi Lars,

Does that mean that the whole remove section just gets removed from
the postrm, or that you put back the test for /usr/sbin/mysqld existing?

Best wishes,

   Julian

Bug#853008: [debian-mysql] Bug#853008: Bug#853008: mysql-server-5.7: purge could delete mariadb-server files with inadequate warning

2017-01-31 Thread Lars Tangvald 

- j...@debian.org wrote:

> On Mon, Jan 30, 2017 at 06:38:16PM +, Robie Basak wrote:
> > > So how about this, just a sketch at the moment rather than a full
> > > patch?
> > 
> > Your sketch seems good to me, assuming that "dpkg-query --search"
> is
> > permitted from maintainer scripts (I know there are some
> re-entrancy
> > problems with some particular types of dpkg-related invocations?)
> > [...]
> > 
> > I wondered this too. But without thinking it through in more
> detail,
> > it'd certainly be better than what we have now.
> 
> Please find attached a patch against the current
> mysql-5.7/debian/master branch on alioth.  I have also updated the
> github branch for mariadb-10.1 to address the same issue (the one
> with
> a pull request to otto/mariadb-10.1).
> 
> It's been tested for upgrades, clean install, remove and purge.
> 
> Best wishes,
> 
>Julian


Thanks!

I'll look it over and test a little myself.

--
Lars

Bug#853008: [debian-mysql] Bug#853008: Bug#853008: mysql-server-5.7: purge could delete mariadb-server files with inadequate warning

2017-01-31 Thread Julian Gilbey 
On Mon, Jan 30, 2017 at 06:38:16PM +, Robie Basak wrote:
> > So how about this, just a sketch at the moment rather than a full
> > patch?
> 
> Your sketch seems good to me, assuming that "dpkg-query --search" is
> permitted from maintainer scripts (I know there are some re-entrancy
> problems with some particular types of dpkg-related invocations?)
> [...]
> 
> I wondered this too. But without thinking it through in more detail,
> it'd certainly be better than what we have now.

Please find attached a patch against the current
mysql-5.7/debian/master branch on alioth.  I have also updated the
github branch for mariadb-10.1 to address the same issue (the one with
a pull request to otto/mariadb-10.1).

It's been tested for upgrades, clean install, remove and purge.

Best wishes,

   Julian
diff --git a/debian/mysql-server-5.7.lintian-overrides b/debian/mysql-server-5.7.lintian-overrides
index f1672c49..c5732994 100644
--- a/debian/mysql-server-5.7.lintian-overrides
+++ b/debian/mysql-server-5.7.lintian-overrides
@@ -1,3 +1,5 @@
 # These long lines reproduce actual output and to reformat them
 # would damage the integrity of the man page.
 manpage-has-errors-from-man usr/share/man/man1/mysqlbinlog.1.gz 1893: warning [p 13, 5.3i, div `3tbd3,2', 0.8i]: can't break line
+# the second update-rc.d is informational only: see postrm for details
+mysql-server-5.7: duplicate-updaterc.d-calls-in-postrm mysql
diff --git a/debian/mysql-server-5.7.postinst b/debian/mysql-server-5.7.postinst
index 54adbd09..e7e7f823 100755
--- a/debian/mysql-server-5.7.postinst
+++ b/debian/mysql-server-5.7.postinst
@@ -22,27 +22,8 @@ run_init_sql() {
   return $result
 }
 
-# To avoid having hardcoded paths in the script, we do a search on the path, as suggested at:
-# https://www.debian.org/doc/manuals/developers-reference/ch06.en.html#bpp-debian-maint-scripts
-pathfind() {
-  OLDIFS="$IFS"
-  IFS=:
-  for p in $PATH; do
-if [ -x "$p/$*" ]; then
-  IFS="$OLDIFS"
-  return 0
-fi
-  done
-  IFS="$OLDIFS"
-  return 1
-}
-
 invoke() {
-  if pathfind invoke-rc.d; then
-invoke-rc.d mysql $1
-  else
-/etc/init.d/mysql $1
-  fi
+  invoke-rc.d mysql $1
 }
 
 # Check if server is able to start. If it fails we abort early and refer
@@ -315,6 +296,27 @@ if [ "$1" = "configure" ]; then
   invoke start
 fi
   fi
+
+  # Fix broken postrms in mysql-server-5.[1-6] and mariadb-server-10.[01]
+  # packages to prevent purging these packages from breaking our package.
+  # (See #852495)  It is unlikely that a user would have mariadb-server
+  # postrm files lying around, but we aim to be safe.
+
+  # 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.[1-6].postrm /var/lib/dpkg/info/mariadb-server-10.0.postrm 2>/dev/null || true)
+  if [ -f /var/lib/dpkg/info/mariadb-server-10.1.postrm ]; then
+  if ! grep -q fullpurge /var/lib/dpkg/info/mariadb-server-10.1.postrm; then
+	  olds="$olds /var/lib/dpkg/info/mariadb-server-10.1.postrm"
+  fi
+  fi
+  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
 fi
 
 # forget we ever saw the password.  don't use reset to keep the seen status
diff --git a/debian/mysql-server-5.7.postrm b/debian/mysql-server-5.7.postrm
index 49d45f1b..a106ec29 100755
--- a/debian/mysql-server-5.7.postrm
+++ b/debian/mysql-server-5.7.postrm
@@ -11,49 +11,33 @@ if [ -n "$DEBIAN_SCRIPT_DEBUG" ]; then set -v -x; DEBIAN_SCRIPT_TRACE=1; fi
 ${DEBIAN_SCRIPT_TRACE:+ echo "#42#DEBUG# RUNNING $0 $*" 1>&2 }
 
 mysql_cfgdir=/etc/mysql
-MYADMIN="/usr/bin/mysqladmin --defaults-file=/etc/mysql/debian.cnf"
-
-# To avoid having hardcoded paths in the script, we do a search on the path, as suggested at:
-# https://www.debian.org/doc/manuals/developers-reference/ch06.en.html#bpp-debian-maint-scripts
-pathfind() {
-  OLDIFS="$IFS"
-  IFS=:
-  for p in $PATH; do
-if [ -x "$p/$*" ]; then
-  IFS="$OLDIFS"
-  return 0
-fi
-  done
-  IFS="$OLDIFS"
-  return 1
-}
-
-# 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 pathfind 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 aga

Bug#853008: mysql-server-5.7: purge could delete mariadb-server files with inadequate warning

2017-01-31 Thread Andreas Beckmann 
On Mon, 30 Jan 2017 09:28:52 + Robie Basak 
wrote:
> I think the root cause here is that both MySQL and MariaDB packaging
> "own" /var/lib/mysql. This causes confusion because even though the
> packages Conflict, one can still be purged while the other is installed.
>
> I think that sharing /var/lib/mysql in this way leads to a slew of bugs,
> and we should fix it in the long term so that the packaging doesn't do
> this.
>
> This is https://launchpad.net/bugs/1490071 and
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=841345
>
> Unfortunately fixing this properly is quite involved. I've had time to
> think up a solution, but not had time to code it up yet, and I don't
> expect to have the necessary time in the next few months.

I don't know what solution you had in mind, but for solving the shared
ownership of /var/lib/mysql I'd suggest to create a new package, e.g.
"var-lib-mysql", from src:mysql-defaults.
* All server packages (but not server-core packages) will depend on this
new package.
* The new package should have Breaks against all server packages that
existed in the past and didn't conform to this new scheme.
* The new package will be handling the removal of the database directory
upon purge. No other place should care about this in the future. So the
purge logic is in one place only and does need to be kept in sync across
several packages.
* Abusing mysql-common for this task is not a good idea since
mysql-common is being depended upon by client packages.

It may be a bit late to get this into stretch, but not impossible, since
this is "fallout" from the mysql->mariadb transition. But it needs to be
acted quickly, with an unblock filed ideally before the freeze on Feb 5th.
It will not help the jessie->stretch upgrades, but it will make the
stretch->buster upgrades more smooth. And if it's only
mariadb-server-10.1 -> mariadb-server-10.2


In case you need to deactivate the postrm purge action of a removed but
not purged old m*-x.y-server package, you could try something like

sed -i '2iif [ "$1" = "purge" ]; then exit 0; fi  # old postrm script
deactivated by $PKG to prevent postrm purge messing around with
/var/lib/mysql, see #853008 for details' $OLD_OTHER_POSTRM

(unwrap, must be a single line)


Andreas

Bug#853008: [debian-mysql] Bug#853008: Bug#853008: mysql-server-5.7: purge could delete mariadb-server files with inadequate warning

2017-01-30 Thread Julian Gilbey 
On Mon, Jan 30, 2017 at 06:38:16PM +, Robie Basak wrote:
> > So how about this, just a sketch at the moment rather than a full
> > patch?
> 
> Your sketch seems good to me, assuming that "dpkg-query --search" is
> permitted from maintainer scripts (I know there are some re-entrancy
> problems with some particular types of dpkg-related invocations?)

AFAIK it is safe to run dpkg-query, as it only queries the dpkg
database without performing any actions: it can be run as an
unprivileged user, for example.  (It is equivalent to "dpkg -S", btw.)
It is already used in numerous maintainer scripts, as a quick "grep
dpkg-query /var/lib/dpkg/info/*.p* shows.

With a little more checking, it turns out that the dpkg-query command
can be simplified, as it doesn't do a pattern search without an
explcit request, so the following should also work:

daemonpackage=$(dpkg-query -S /usr/sbin/mysqld 2>/dev/null | cut -d: -f1)

The "|| true" is probably also unnecessary, as the pipeline will
succeed unless the "cut" command fails, which is highly unlikely.

> [...]
> 
> I wondered this too. But without thinking it through in more detail,
> it'd certainly be better than what we have now.

Fair enough.  If implementing it, it should probably be done together
with the patches I sent to the other bug report, and applied to both
the mysql-server and mariadb-server packages.

Best wishes,

   Julian

Bug#853008: [debian-mysql] Bug#853008: Bug#853008: Bug#853008: mysql-server-5.7: purge could delete mariadb-server files with inadequate warning

2017-01-30 Thread Julian Gilbey 
On Mon, Jan 30, 2017 at 06:51:28PM +, Robie Basak wrote:
> On Mon, Jan 30, 2017 at 06:38:16PM +, Robie Basak wrote:
> > manage /var/lib/mysql manually. So perhaps it is indeed entirely
> > inappropriate to put purging code in the *-core package postrm.
> 
> I keep saying "purging code", but what I really mean is "code that
> touches/deletes /var/lib/mysql on purge. It is of course entirely
> appropriate to put code that handles the purge step in a postrm for any
> package :-)

I read what you wrote with exactly that understanding ;-)

   Julian

Bug#853008: [debian-mysql] Bug#853008: Bug#853008: Bug#853008: mysql-server-5.7: purge could delete mariadb-server files with inadequate warning

2017-01-30 Thread Robie Basak 
On Mon, Jan 30, 2017 at 06:38:16PM +, Robie Basak wrote:
> manage /var/lib/mysql manually. So perhaps it is indeed entirely
> inappropriate to put purging code in the *-core package postrm.

I keep saying "purging code", but what I really mean is "code that
touches/deletes /var/lib/mysql on purge. It is of course entirely
appropriate to put code that handles the purge step in a postrm for any
package :-)


signature.asc
Description: PGP signature

Bug#853008: [debian-mysql] Bug#853008: Bug#853008: mysql-server-5.7: purge could delete mariadb-server files with inadequate warning

2017-01-30 Thread Robie Basak 
On Mon, Jan 30, 2017 at 06:02:30PM +, Julian Gilbey wrote:
> There is an issue with this: the postrm's in mysql-server-5.7 and
> mariadb-server-10.1 do significantly more than just removing
> /var/lib/mysql when purging - see my (now broken because of this!)
> patches on bug#852495: they also do all of the debhelper fragments
> which should only be performed on the purge of the final mysqld
> instance.  These clearly don't belong in the server-core packages, and
> it would be strange (and perhaps wrong) to shift them there.

You reminded me that the *-core packages are supposed to provide the
binaries without any "smarts". So a user might expect to use them and
manage /var/lib/mysql manually. So perhaps it is indeed entirely
inappropriate to put purging code in the *-core package postrm.

> So how about this, just a sketch at the moment rather than a full
> patch?

Your sketch seems good to me, assuming that "dpkg-query --search" is
permitted from maintainer scripts (I know there are some re-entrancy
problems with some particular types of dpkg-related invocations?)

> I *think* this covers the cases, but I'm really not certain.

I'm also not certain.

> a possibility that someone could be purging the old mysql-* packages
> while installing a newer replacement, and I'm not sure how aptitude
> would handle that one because of the somewhat complex
> Breaks/Conflicts/Replaces settings in the various control files.  But
> a sysadmin who tries to purge an old mysql-server-providing package
> simultaneously to installing a new one is potentially asking for
> things to be deleted, so I'm not overly concerned about this.  They
> would still be asked about deleting data in this case, so there is a
> safety net.  (But they could conceivably lose their apparmor settings
> and so on.)

I wondered this too. But without thinking it through in more detail,
it'd certainly be better than what we have now.

Robie


signature.asc
Description: PGP signature

Bug#853008: [debian-mysql] Bug#853008: Bug#853008: mysql-server-5.7: purge could delete mariadb-server files with inadequate warning

2017-01-30 Thread Julian Gilbey 
On Mon, Jan 30, 2017 at 10:00:20AM +, Robie Basak wrote:
> On Mon, Jan 30, 2017 at 10:45:44AM +0100, Lars Tangvald wrote:
> > I think an ok short-term solution is to make a .postrm script for
> > mysql-server-core, and move the delete logic there with the check on
> > /usr/sbin/mysqld restored, for both MariaDB and MySQL. Then we don't need to
> > check on any specific packages, since we'd know any existing mysqld binary
> > doesn't belong to the package being purged. If the user has installed a
> > MariaDB, the deletion would be handled by mariadb-server-core being purged.
> 
> I follow why that would work now, thanks. I don't see any problem with
> doing this until we have better handling of /var/lib/mysql between
> variants. So +1, unless someone else raises something.

There is an issue with this: the postrm's in mysql-server-5.7 and
mariadb-server-10.1 do significantly more than just removing
/var/lib/mysql when purging - see my (now broken because of this!)
patches on bug#852495: they also do all of the debhelper fragments
which should only be performed on the purge of the final mysqld
instance.  These clearly don't belong in the server-core packages, and
it would be strange (and perhaps wrong) to shift them there.

So how about this, just a sketch at the moment rather than a full
patch?

* Leave the {mysql,mariadb}-server-core-* packages as they are.

* In the postrm of the {mysql,mariadb}-server-X.Y packages, have the
  following.  We do not do a full purge if /usr/sbin/mysqld is a link,
  as we know nothing about this setup; we only know /usr/sbin/mysqld
  being a regular executable.

# Do we (offer to) do a full purge, or is there another mysql server
# present?  This includes within it a check for "$1" = purge.
fullpurge=no
if [ "$1" = purge ]; then
  if [ ! \( -x /usr/sbin/mysqld -o -L /usr/sbin/mysqld \) ]; then
fullpurge=yes
  elif [ -x /usr/sbin/mysqld ]; then
# Does this /usr/sbin/mysqld belong to our server-core package?
# If so, we can offer to do a full purge, otherwise we do not.

# Use the DPKG_MAINTSCRIPT_PACKAGE to avoid hardcoding the package
# name; this reduces the chances of future errors creeping in.
# If we're being run outside of a dpkg run, then this is safe.
package=${DPKG_MAINTSCRIPT_PACKAGE}
corepackage=$(echo "$package" | sed -e 's/server-/server-core-/')
daemonpackage=$(dpkg-query --search /usr/sbin/mysqld 2>/dev/null | grep ' 
/usr/sbin/mysqld$' | cut -d: -f1 || true)
if [ "$corepackage" = "$daemonpackage" -a -n "$corepackage" ]; then
  fullpurge=yes
fi
  fi
fi

Then in the rest of the postrm, check
  if [ $fullpurge = yes ]; then ...
instead of
  if [ "$1" = purge ]; then ...
when deciding whether to perform a purge action which should only be
done if this is the final mysqld-providing package.


I *think* this covers the cases, but I'm really not certain.  There is
a possibility that someone could be purging the old mysql-* packages
while installing a newer replacement, and I'm not sure how aptitude
would handle that one because of the somewhat complex
Breaks/Conflicts/Replaces settings in the various control files.  But
a sysadmin who tries to purge an old mysql-server-providing package
simultaneously to installing a new one is potentially asking for
things to be deleted, so I'm not overly concerned about this.  They
would still be asked about deleting data in this case, so there is a
safety net.  (But they could conceivably lose their apparmor settings
and so on.)

Best wishes,

   Julian

Bug#853008: [debian-mysql] Bug#853008: Bug#853008: mysql-server-5.7: purge could delete mariadb-server files with inadequate warning

2017-01-30 Thread Robie Basak 
On Mon, Jan 30, 2017 at 10:45:44AM +0100, Lars Tangvald wrote:
> I think an ok short-term solution is to make a .postrm script for
> mysql-server-core, and move the delete logic there with the check on
> /usr/sbin/mysqld restored, for both MariaDB and MySQL. Then we don't need to
> check on any specific packages, since we'd know any existing mysqld binary
> doesn't belong to the package being purged. If the user has installed a
> MariaDB, the deletion would be handled by mariadb-server-core being purged.

I follow why that would work now, thanks. I don't see any problem with
doing this until we have better handling of /var/lib/mysql between
variants. So +1, unless someone else raises something.


signature.asc
Description: PGP signature

Bug#853008: [debian-mysql] Bug#853008: Bug#853008: mysql-server-5.7: purge could delete mariadb-server files with inadequate warning

2017-01-30 Thread Lars Tangvald 



On 01/30/2017 10:28 AM, Robie Basak wrote:

Hi Julian,

Thank you for reporting this.

On Mon, Jan 30, 2017 at 09:24:46AM +0100, Lars Tangvald wrote:

Anyone else have any good ideas on how to handle this?

I think the root cause here is that both MySQL and MariaDB packaging
"own" /var/lib/mysql. This causes confusion because even though the
packages Conflict, one can still be purged while the other is installed.

I think that sharing /var/lib/mysql in this way leads to a slew of bugs,
and we should fix it in the long term so that the packaging doesn't do
this.

This is https://launchpad.net/bugs/1490071 and
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=841345

Unfortunately fixing this properly is quite involved. I've had time to
think up a solution, but not had time to code it up yet, and I don't
expect to have the necessary time in the next few months.

Am I right in thinking that this could be triggered by installing MySQL
in jessie, upgrading to stretch, installing MariaDB if it isn't already,
and then purging MySQL? If so I agree with "serious" and so we should
find some stop-gap solution in the short term until the full fix above
can be implemented.

Could we work on unifying MySQL and MariaDB's handling of this? Use the
same debconf key and present the question for /var/lib/mysql in general,
making it clear that it applies to both MySQL and MariaDB. Then on
purge, if the answer was "yes, purge", then ask again to confirm that
the user does actually want it deleted both for MySQL and MariaDB (not
sure we can ask debconf questions in postrm though?). A goal should be
that all the code handling creation and deletion of /var/lib/mysql
should be common between MySQL and MariaDB. Either leave comments in the
maintainer scripts, or put the code in mysql-common from
src:mysql-defaults and Pre-Depend on it.

Would this be acceptable, both from technical implementation and UX
perspectives? Does Debian and/or stretch have string translation
deadlines or freezes?

Additionally we could have some hacks to try to determine if the
counterpart variant is installed and not do anything if it is. I think
hacks are fine in the short term because we have an open bug for a
proper solution, and the seriousness of this issue (if as above)
warrants it.

Opinions appreciated.

Robie
The trigger would basically just be to install and remove 
mysql-server-5.7 (which also happens when replacing it with a 
conflicting package), then install any other variant, then purge 
mysql-server-5.7.
I think an ok short-term solution is to make a .postrm script for 
mysql-server-core, and move the delete logic there with the check on 
/usr/sbin/mysqld restored, for both MariaDB and MySQL. Then we don't 
need to check on any specific packages, since we'd know any existing 
mysqld binary doesn't belong to the package being purged. If the user 
has installed a MariaDB, the deletion would be handled by 
mariadb-server-core being purged.


Looking at the debconf manual, only postinst is mentioned as a place not 
to use it (and it actually uses postrm and asking about deleting 
something as a usage example).


--
Lars

Bug#853008: [debian-mysql] Bug#853008: Bug#853008: mysql-server-5.7: purge could delete mariadb-server files with inadequate warning

2017-01-30 Thread Robie Basak 
Hi Julian,

Thank you for reporting this.

On Mon, Jan 30, 2017 at 09:24:46AM +0100, Lars Tangvald wrote:
> Anyone else have any good ideas on how to handle this?

I think the root cause here is that both MySQL and MariaDB packaging
"own" /var/lib/mysql. This causes confusion because even though the
packages Conflict, one can still be purged while the other is installed.

I think that sharing /var/lib/mysql in this way leads to a slew of bugs,
and we should fix it in the long term so that the packaging doesn't do
this.

This is https://launchpad.net/bugs/1490071 and
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=841345

Unfortunately fixing this properly is quite involved. I've had time to
think up a solution, but not had time to code it up yet, and I don't
expect to have the necessary time in the next few months.

Am I right in thinking that this could be triggered by installing MySQL
in jessie, upgrading to stretch, installing MariaDB if it isn't already,
and then purging MySQL? If so I agree with "serious" and so we should
find some stop-gap solution in the short term until the full fix above
can be implemented.

Could we work on unifying MySQL and MariaDB's handling of this? Use the
same debconf key and present the question for /var/lib/mysql in general,
making it clear that it applies to both MySQL and MariaDB. Then on
purge, if the answer was "yes, purge", then ask again to confirm that
the user does actually want it deleted both for MySQL and MariaDB (not
sure we can ask debconf questions in postrm though?). A goal should be
that all the code handling creation and deletion of /var/lib/mysql
should be common between MySQL and MariaDB. Either leave comments in the
maintainer scripts, or put the code in mysql-common from
src:mysql-defaults and Pre-Depend on it.

Would this be acceptable, both from technical implementation and UX
perspectives? Does Debian and/or stretch have string translation
deadlines or freezes?

Additionally we could have some hacks to try to determine if the
counterpart variant is installed and not do anything if it is. I think
hacks are fine in the short term because we have an open bug for a
proper solution, and the seriousness of this issue (if as above)
warrants it.

Opinions appreciated.

Robie


signature.asc
Description: PGP signature

Bug#853008: [debian-mysql] Bug#853008: mysql-server-5.7: purge could delete mariadb-server files with inadequate warning

2017-01-30 Thread Julian Gilbey 
On Mon, Jan 30, 2017 at 09:24:46AM +0100, Lars Tangvald wrote:
> 
> The problem with the old code was that at the point it was run,
> usr/sbin/mysqld  _always_ existed, so it was just a big chunk of dead code.
> The code was just copied from the old packages that had a different
> structure. I think the code would need to be in the maintainer script for
> the package that has the server binary, so that it would run correctly after
> the package's own binary is removed, but that's -server-core, which doesn't
> run any postrm (we could add it, but part of the point of the core packages
> is that they don't run anything).

Oh, I didn't realise that there was this split between -server and
-server-core.

> Anyone else have any good ideas on how to handle this?

Yes, but I don't have time to write it now - will have a go later in
the day.

   Julian

Bug#853008: [debian-mysql] Bug#853008: mysql-server-5.7: purge could delete mariadb-server files with inadequate warning

2017-01-30 Thread Lars Tangvald 



On 01/29/2017 01:13 PM, Julian Gilbey wrote:

On Sat, Jan 28, 2017 at 09:21:13PM +, Julian Gilbey wrote:

Package: mysql-server-5.7
Version: 5.7.16-2
Severity: serious

Hello!

I'm really confused by the change in the postrm introduced in response
to LP: #1602945, and I simply do not understand the rationale of the
original bug report, and the comment there (and in the git commit log)
that "Remove the check on the server binary, since it shouldn't be
possible for another package to own that file anyway" is clearly
incorrect: during a postrm remove|purge run, if that file exists -
which it may well do, it will certainly belong to a different package,
such as mysql-server-5.8 or mariadb-server-10.1.

And looking at the existing postrms, I see that this is effectively
just Debian bug #307473 reopened.  That was regarded as critical!

The problem with the old code was that at the point it was run, 
usr/sbin/mysqld  _always_ existed, so it was just a big chunk of dead 
code. The code was just copied from the old packages that had a 
different structure. I think the code would need to be in the maintainer 
script for the package that has the server binary, so that it would run 
correctly after the package's own binary is removed, but that's 
-server-core, which doesn't run any postrm (we could add it, but part of 
the point of the core packages is that they don't run anything).


Anyone else have any good ideas on how to handle this?

--
Lars

Best wishes,

Julian

___
pkg-mysql-maint mailing list
pkg-mysql-ma...@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-mysql-maint

Bug#853008: mysql-server-5.7: purge could delete mariadb-server files with inadequate warning

2017-01-29 Thread Julian Gilbey 
On Sat, Jan 28, 2017 at 09:21:13PM +, Julian Gilbey wrote:
> Package: mysql-server-5.7
> Version: 5.7.16-2
> Severity: serious
> 
> Hello!
> 
> I'm really confused by the change in the postrm introduced in response
> to LP: #1602945, and I simply do not understand the rationale of the
> original bug report, and the comment there (and in the git commit log)
> that "Remove the check on the server binary, since it shouldn't be
> possible for another package to own that file anyway" is clearly
> incorrect: during a postrm remove|purge run, if that file exists -
> which it may well do, it will certainly belong to a different package,
> such as mysql-server-5.8 or mariadb-server-10.1.

And looking at the existing postrms, I see that this is effectively
just Debian bug #307473 reopened.  That was regarded as critical!

Best wishes,

   Julian

Bug#852008: Bug#853008: mysql-server-5.7: purge could delete mariadb-server files with inadequate warning

2017-01-29 Thread Julian Gilbey 
On Sat, Jan 28, 2017 at 09:21:13PM +, Julian Gilbey wrote:
> Package: mysql-server-5.7
> Version: 5.7.16-2
> Severity: serious
> 
> Hello!
> 
> I'm really confused by the change in the postrm introduced in response
> to LP: #1602945, and I simply do not understand the rationale of the
> original bug report, and the comment there (and in the git commit log)
> that "Remove the check on the server binary, since it shouldn't be
> possible for another package to own that file anyway" is clearly
> incorrect: during a postrm remove|purge run, if that file exists -
> which it may well do, it will certainly belong to a different package,
> such as mysql-server-5.8 or mariadb-server-10.1.

And looking at the existing postrms, I see that this is effectively
just Debian bug #307473 reopened.  That was regarded as critical!

Best wishes,

   Julian

Bug#853008: mysql-server-5.7: purge could delete mariadb-server files with inadequate warning

2017-01-28 Thread Julian Gilbey 
Package: mysql-server-5.7
Version: 5.7.16-2
Severity: serious

Hello!

I'm really confused by the change in the postrm introduced in response
to LP: #1602945, and I simply do not understand the rationale of the
original bug report, and the comment there (and in the git commit log)
that "Remove the check on the server binary, since it shouldn't be
possible for another package to own that file anyway" is clearly
incorrect: during a postrm remove|purge run, if that file exists -
which it may well do, it will certainly belong to a different package,
such as mysql-server-5.8 or mariadb-server-10.1.

Regarding upgrading, I have upgraded from mysql-5.6 to mariadb-10.0
with no noticeable problems whatsoever.  I have done a file comparison
looking for differences between /var/lib/mysql following this
mysql->mariadb conversion and a clean mariadb installation, and the
only ones I could see are:

mysql/ndb_binlog_index.*
mysql/slave_master_info.*
mysql/slave_relay_log_info.*
mysql/slave_worker_info.*
auto.cnf

If it is really the case the mariadb needs a different set of files
from mysql because of incompatibilities, then they should not share
the same /var/lib/mysql or it should be addressed in some other way:
mariadb could use /var/lib/mariadb instead, for instance, or the
mariadb installation could run some "clean up old /var/lib/mysql
directory from mysql instance" (which I think it already does).

So by removing the check on the existence of /usr/sbin/mysqld, you
leave sysadmins with the potential to inadvertantly wipe their entire
running mysql instance's database, for example if they purge an old
mysql-server-5.7 after installing mysql-server-5.8 at some point in
the future!

So as long as the various mysql/mariadb servers share the
/var/lib/mysql directory, the check for /usr/sbin/mysqld must be
reinstated: I cannot think of any situation in which rm -rf
/var/lib/mysql would be desired if /usr/sbin/mysqld is present, as
this means that there is some other package with responsibility for
/var/lib/mysql installed, and therefore this would fail the Debian
policy directive of not interferiyng with other packages' data.  (And
that is why I have labelled this bug as severity "serious".)  If a
sysadmin really wants to delete the directory in this situation, they
are welcome to do so manually.

The patch for this is simple: simply revert commit
554ecf472109f2b2e2fdfa0aaa0302cf2247ac1e

This would reopen the LP bug report, and I would ask the original
reporter there to explain the circumstances in which there are
problems, so that those can be addressed appropriately.

In addition, the current postrm_remove_databases debconf
question should be enhanced to explicitly mention mariadb.  I would
also suggest being even more explicit: instead of "the data should be
kept", say "do NOT remove the databases, and say NO to this question."

Best wishes,

   Julian