Hi Tim, all!
I checked your patch, comments are inserted below.
I tested the RPM commands which you added on a SuSE 9.3 installation,
and they worked fine.
I did not try to verify your complete patch, because I lack a test
machine (physical or virtual) on which to install a RPM that might
trigger the check.
tim@stripped wrote:
> Below is the list of changes that have just been committed into a local
> 5.0 repository of tsmith. When tsmith does a push these changes
> will be propagated to the main repository and, within 24 hours after the
> push, to the public repository.
> For information on how to access the public repository
> see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
>
> ChangeSet@stripped, 2008-02-15 02:30:12-07:00, tsmith@stripped +1 -0
> Bug #28555 Upgrading MySQL Fails to shut down old server and kills socket file
>
> Check for an existing MySQL server package from a different vendor or
> major MySQL version. In such a case, refuse to install the server and
> recommend how to safely remove the old packages before installing the
> new ones.
>
> support-files/mysql.spec.sh@stripped, 2008-02-15 02:30:09-07:00,
> tsmith@stripped +61 -1
> Add to the %pre server scriptlet checks to ensure that we're not
> upgrading from another vendor's package, or that this is not a
> major version upgrade. If an automatic upgrade isn't safe, print
> basic instructions on how to do a manual upgrade, and bail out.
>
> diff -Nrup a/support-files/mysql.spec.sh b/support-files/mysql.spec.sh
> --- a/support-files/mysql.spec.sh 2007-11-16 09:36:11 -07:00
> +++ b/support-files/mysql.spec.sh 2008-02-15 02:30:09 -07:00
> @@ -15,6 +15,7 @@
> # MA 02110-1301 USA.
>
> %define mysql_version @VERSION@
> +%define mysql_vendor MySQL AB
While that is correct, it still leaves the risk of somebody outside
MySQL just using the spec file without changing this setting.
If we really want to be safe, we might set it to "unknown" here and then
override it when we start the RPM build.
This would require additional tool changes (build scripts).
>
> # use "rpmbuild --with static" or "rpm --define '_with_static 1'" (for RPM 3.x)
> # to enable static linking (off by default)
> @@ -69,7 +70,7 @@ License: %{license}
>
> Source: http://www.mysql.com/Downloads/MySQL-@MYSQL_BASE_VERSION@/mysql-%{mysql_version}.tar.gz
> URL: http://www.mysql.com/
> Packager: MySQL Production Engineering Team <build@stripped>
> -Vendor: MySQL AB
> +Vendor: %{mysql_vendor}
> Provides: msqlormysql MySQL-server mysql
> BuildRequires: ncurses-devel
> Obsoletes: mysql
ok.
> @@ -420,6 +421,65 @@ touch $RBR%{_sysconfdir}/my.cnf
> touch $RBR%{_sysconfdir}/mysqlmanager.passwd
>
> %pre server
> +# Check if we can safely upgrade. An upgrade is only safe if it's from one
> +# of our RPMs in the same version family.
> +
> +installed=`rpm -q --whatprovides mysql-server 2> /dev/null`
> +if [ $? -eq 0 -a -n "$installed" ]; then
> + myversion='%{mysql_version}'
> + myvendor='%{mysql_vendor}'
> + vendor=`rpm -q --queryformat='%{VENDOR}' "$installed" 2>&1`
> + version=`rpm -q --queryformat='%{VERSION}' "$installed" 2>&1`
The above commands work fine on SuSE 9.3 with MySQL 5.1.23-rc installed.
> +
> + family=`echo $version | sed -e 's,\([0-9][0-9]*\.[0-9][0-9]*\)\..*,\1,'`
> + new_family=`echo $myversion | sed -e 's,\([0-9][0-9]*\.[0-9][0-9]*\)\..*,\1,'`
You allow for further growth, supporting multi-digit major and minor
versions. I'm not sure that is needed, but it should not harm.
Still, I propose some changes:
- Anchor the pattern at the beginning of the string.
- Explicitly extend it to the end (yes, matching is greedy, this is more
for documentation).
- Suppress output in case of non-match,
later check for non-empty (this may be considered paranoid).
- Restrict the very first digit to the 1..9 range.
- For documentation, I would rename "family" to "old_family", showing
the difference to "new_family".
(this causes changes further down - not shown, for clarity).
So my proposal is:
+
+ old_family=`echo $version | sed -n -e
's,^\([1-9][0-9]*\.[0-9][0-9]*\)\..*$,\1,p'`
+ new_family=`echo $myversion | sed -n -e
's,^\([1-9][0-9]*\.[0-9][0-9]*\)\..*$,\1,p'`
Yes, I know these checks on "new_family" may be considered overkill,
because that is derived from a constant string and not from program
output. Maybe I prefer symmetry too much ...
> +
> + vendor_error=
> + if [ "$vendor" != "$myvendor" ]; then
> + vendor_error="
> +The current MySQL server package is provided by a different
> +vendor ($vendor) than $myvendor.
> +"
> + fi
> +
> + family_error=
> + if [ "$family" != "$new_family" ]; then
Consequence of the above proposal: Replace the last line by
+ if [ "$old_family" != "$new_family"
+ -o -z "$old_family" -o -z "$new_family" ]; then
> + family_error="
> +Upgrading directly from MySQL $family to MySQL $new_family may not
> +be safe in all cases.
> +"
> + fi
> +
> + if [ -n "$vendor_error$family_error" ]; then
This may be a style issue ...
I would not combine these two strings into one, rather check them
separate, but I am sure the above works.
But *if* you prefer concatenating them (both here and in the message
below), then I would rather do that when building them:
error_string=""
if [ vendor-difference ]; then
error_string="$error_string
FOO
"
fi
if [ family-difference ]; then
error_string="$error_string
BAR
"
if [ -n "$error_string" ]; then
One of my reasons is that this approach will make it easier to extend
the checks, should we add more later.
> + cat <<HERE >&2
> +
> +******************************************************************
> +A MySQL server package ($installed) is installed.
> +$vendor_error$family_error
> +A manual upgrade is required. A recommended procedure is:
^^^
Isn't it "The" recommended procedure ? Do we have several ?
> +
> +- Ensure that you have a complete, working backup of your data and my.cnf
> + files
> +- Shut down the MySQL server cleanly
> +- Remove the existing MySQL packages. Usually this command will
> + list the packages you should remove:
> + rpm -qa | grep -i '^mysql-'
> +
> + You may choose to use 'rpm --nodeps -ev <package-name>' to remove
> + the package which contains the mysqlclient shared library. The
> + library will be reinstalled by the MySQL-shared-compat package.
> +- Install the new MySQL packages supplied by $myvendor
> +- Ensure that the MySQL server is started
> +- Run the 'mysql_upgrade' program
I am torn between two positions:
- Keep it small.
- Provide as many hints as possible.
For "small", your text is fine.
For "many hints", we would need to give separate explanations for
"different vendor" (directory structure may differ, ...) and
"different family" (run 'mysql_upgrade').
Also, a different vendor might provide RPMs named 'mysql-*' for which we
have no equivalent (say, a tutorial, a GUI, ...), and we would not in
general propose to uninstall these.
A compromise might be to add such hints to the "*_error" strings when
setting them.
As I said: Torn between two positions ...
> +
> +This is a brief description of the upgrade process. Important details
> +can be found in the MySQL manual, in the Upgrading section.
> +******************************************************************
> +HERE
> + exit 1
> + fi
> +fi
> +
> # Shut down a previously installed server first
> if test -x %{_sysconfdir}/init.d/mysql
> then
>
--
Joerg Bruehe, Senior Production Engineer
MySQL AB, www.mysql.com
Office: (+49 30) 417 01 487 VoIP: 4464@stripped
