Make WordPress Core

Opened 14 years ago

Closed 14 years ago

Last modified 14 years ago

#11464 closed defect (bug) (fixed)

MySQL 4.1.2 and automatic update...

Reported by: nickbohle's profile nickbohle Owned by:
Milestone: 2.9 Priority: high
Severity: blocker Version: 2.9
Component: Upgrade/Install Keywords: has-patch needs-testing
Focuses: Cc:

Description

Dear all,

I've just tried to update to WP 2.9 RC1 via the beta tester plug-in and the automatic update.

Everything worked like a charm (files downloaded, installled, etc.), until I had to upgrade my database.

Until?

// Make sure the server has MySQL 4.1.2 
 	1054	        if ( version_compare($this->db_version(), '4.1.2', '<') ) 
 	1055	            return new WP_Error('database_version',sprintf(__('<strong>ERROR</strong>: WordPress %s requires MySQL 4.1.2 or higher'), $wp_version));


I know! By using the beta tester plugin, I totally forgot about the database requirements. My fault. And I fixed by downgrading...

But what is the reaction of the average WP user when he updates WP to 2.9 via the automatic update, he starts the database update and then, his database upgrade will not work due to an old mysql version. Result: he will be stuck with no connection to the back-end.

So, my question:

Why is there no MySQL (and PHP) version check BEFORE the automatic updates starts. A warning about an outdated mysql verion would be more useful BEFORE I try to install WP 2.9


Attachments (10)

11464.diff (1.1 KB) - added by westi 14 years ago.
A possible fix - as yet untested
11464.2.diff (1.2 KB) - added by Denis-de-Bernardy 14 years ago.
highlight the versions
11464.4.diff (1.2 KB) - added by Denis-de-Bernardy 14 years ago.
woops
11464.5.diff (1.2 KB) - added by Denis-de-Bernardy 14 years ago.
fix the typo as well
11464.6.diff (1.4 KB) - added by nacin 14 years ago.
Checks PHP and MySQL separately, only shows errors it needs to show.
11464.7.diff (1.8 KB) - added by caesarsgrunt 14 years ago.
11464.8.diff (1.6 KB) - added by nacin 14 years ago.
11464.9.diff (1.9 KB) - added by ryan 14 years ago.
Future proof translations
11464.9.2.diff (2.4 KB) - added by nacin 14 years ago.
11464.10.diff (3.6 KB) - added by nacin 14 years ago.

Download all attachments as: .zip

Change History (52)

#1 @nickbohle
14 years ago

  • Component changed from General to Upgrade/Install

#2 @westi
14 years ago

I'm not sure there is something easy we can do here.

We could put a check into wp-admin/includes/update-core.php as that get copied across and does the core of the work but that check would need to be ignorable in the case that the user is using a replacement db class which has different requirements.

I'm going to throw up a patch which adds a check and see what people think.

@westi
14 years ago

A possible fix - as yet untested

#3 @Denis-de-Bernardy
14 years ago

westi beat me to create the patch -- haven't tested, but I'm pretty certain that it'll work indeed.

#4 follow-up: @Denis-de-Bernardy
14 years ago

while we're at it, we might as well check the php version too.

#5 in reply to: ↑ 4 @westi
14 years ago

Replying to Denis-de-Bernardy:

while we're at it, we might as well check the php version too.

You mean like the patch does ;-)

#6 @westi
14 years ago

  • Priority changed from normal to high
  • Severity changed from normal to blocker
  • Type changed from enhancement to defect (bug)

Updating the priority/severity on this - I think we should resolve this prior to release.

#7 @scribu
14 years ago

Patch looks good. Just a few minor corrections to the text:

"The update cannot be installed because you do not have the correct PHP (4.3 or newer) and MySQL versions (4.1.2 or newer)"

#8 @nickbohle
14 years ago

Thank you guys for the fast response!

I would suggest to split the testing for the correct PHP / MySQL version:

Test PHP --> if not okay:

"The update cannot be installed because you do not have the correct PHP version (4.3 or newer)"

if okay --> proceed

Test MySQL --> if not okay:

"The update cannot be installed because you do not have the correct MySQL version (4.1.2 or newer)"

if okay ---> proceed

This would point the user into the right direction.

And don't forget localization.

#9 @Denis-de-Bernardy
14 years ago

Replying to westi:

Replying to Denis-de-Bernardy:

while we're at it, we might as well check the php version too.

You mean like the patch does ;-)

Exactly. I had stopped reading it upon seeing update-core.php was the file getting patched. :-)

@Denis-de-Bernardy
14 years ago

highlight the versions

#10 @Denis-de-Bernardy
14 years ago

This would point the user into the right direction.

please try the new one.

@Denis-de-Bernardy
14 years ago

woops

@Denis-de-Bernardy
14 years ago

fix the typo as well

#11 follow-up: @nacin
14 years ago

There's no need to provide them four version numbers (current and required for both PHP and MySQL). First, since you're checking both at once and throwing an error if either is not sufficient, then it might say "PHP (4.3.1 vs 4.3 or newer)", which is just confusing.

Second, how much of a point in the right direction does a user need? An end-user will be contacting their host anyway, saying they can't run WordPress and this is the error they're getting. A web developer or sys admin will know exactly what needs to be done. I find this similar to upload file size errors -- we don't bother telling them about post_max_size, max_upload_filesize, .htaccess, or php.ini for these reasons.

At the very least, perhaps we can make it more understandable by breaking it up and only displaying what the real error is.

#12 in reply to: ↑ 11 @nickbohle
14 years ago

Replying to nacin:

At the very least, perhaps we can make it more understandable by breaking it up and only displaying what the real error is.

I second this. Let's just say:

Test PHP --> if not okay:

"The update cannot be installed because your web host is running an outdated PHP version"

if okay --> proceed

Test MySQL --> if not okay:

"The update cannot be installed because your web host is running an outdated MySQL version"

if okay ---> proceed

Regards,
Nick

#13 @nacin
14 years ago

What you don't want is to test PHP, and find that PHP is too low, so they contact their host, then they come back, and find that their MySQL is too low.

#14 @azaozz
14 years ago

Splitting the test in two parts and showing the right error message(s) makes sense. Perhaps we should show both results when there's an error:

PHP version 4.3 or newer: PASS
MySQL version 4.1.2 or newer: FAIL

#15 follow-up: @nacin
14 years ago

Patch attached that incorporates I think everyone's ideas here. Untested.

#16 in reply to: ↑ 15 @Denis-de-Bernardy
14 years ago

Replying to nacin:

Patch attached that incorporates I think everyone's ideas here. Untested.

You are running version %2$s will return an insufficient number of arguments from sprintf. It should be %s instead.

#17 @nacin
14 years ago

Artifact from when I had $wp_version in there. Then I realized that would only return the current version, so I stripped it. Forgot to update that part, obviously.

Patch replaced.

@nacin
14 years ago

Checks PHP and MySQL separately, only shows errors it needs to show.

#18 @caesarsgrunt
14 years ago

Patch 7 shows a combined error notice if both errors apply. More user-friendly.

@nacin
14 years ago

#19 @nacin
14 years ago

  • Keywords has-patch needs-testing added; MySQL version update removed

Many patches here, though we need testing.

I just added patch 8 which cleans up patch 7 a bit.

#20 @ryan
14 years ago

(In [12422]) Check MySQL and PHP versions when auto upgrading. Props westi, Denis-de-Bernardy, nacin, caesarsgrunt. see #11464

#21 @ryan
14 years ago

(In [12423]) Check MySQL and PHP versions when auto upgrading. Props westi, Denis-de-Bernardy, nacin, caesarsgrunt. see #11464 for 2.9

#22 @ryan
14 years ago

Updated the nightly builds so that have the new checks.

#23 @TobiasBg
14 years ago

Severe typo here:

$mysql_version = $wpdb->db_version;

needs to be

$mysql_version = $wpdb->db_version();

Just ran into it, when trying to update to latest nightly build as announced by ryan.

#25 @nacin
14 years ago

Oops, that was me. :)

#26 @ryan
14 years ago

Updating nightlies. Wait a few minutes and try again.

A good way to force the DB check for testing is to edit your wp-db.php and hack db_version() to return '4.1.0'.

#27 @ryan
14 years ago

That looks better. Working here.

#28 @nickbohle
14 years ago

Works like a charm!

#29 @ryan
14 years ago

  • Resolution set to fixed
  • Status changed from new to closed

Thanks everyone. Resolving as fixed.

#30 @ryan
14 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

westi brought up an issue with translations. Since the strings will only be in the catalog for the new version of WP, these won't translate. We could do hard translations here, or we could live with it for now knowing that once 2.9 is out future upgrades will have the translations. To future proof those translations, we'd have to move the required versions out of the strings and sprintf them in.

@ryan
14 years ago

Future proof translations

@nacin
14 years ago

@nacin
14 years ago

#31 @ryan
14 years ago

(In [12432]) Add required php and mysql versions to version.php. Future proof required version check strings. Props nacin. see #11464

#32 @ryan
14 years ago

(In [12433]) Add required php and mysql versions to version.php. Future proof required version check strings. Props nacin. see #11464 for 2.9

#33 @nacin
14 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

I guess we can reopen if there are further issues. Looks good.

#34 follow-up: @Denis-de-Bernardy
14 years ago

could r12343 potentially update the wp_db_version option in weird circumstances?

#35 in reply to: ↑ 34 ; follow-up: @nacin
14 years ago

Replying to Denis-de-Bernardy:

could r12343 potentially update the wp_db_version option in weird circumstances?

It's not including version.php in global scope. Is that what you mean?

#36 in reply to: ↑ 35 @Denis-de-Bernardy
14 years ago

Replying to nacin:

Replying to Denis-de-Bernardy:

could r12343 potentially update the wp_db_version option in weird circumstances?

It's not including version.php in global scope. Is that what you mean?

Pretty much so, yes. Phrased like that, it makes much more sense, and my worries were unjustified. :-)

#37 @ryan
14 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

The include of version.php is not working on some of my testbeds. It gets the wrong path. I think to include something we have to copy it into place first.

#38 @ryan
14 years ago

(In [12450]) Hard code required version in update-core.php. see #11464

#39 @ryan
14 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

[12451] for 2.9.

I hard-coded the versions in update-core.php and removed the version.php include. We can fix it more elegantly in 3.0.

#40 @ryan
14 years ago

I forgot to capture the error, but the include was missing the beginning of the path. It was relative to the web root instead of ABSPATH. WP_Filesystem::exists() has ways of determining the root for $from that a straight include doesn't have. To include version.php we'll probably have to copy it into place first, like we do with update-core.php. We can consider that for 3.0.

#41 @caesarsgrunt
14 years ago

The path tarted with a "wordpress" dir, which doesn't exist...

#42 @dd32
14 years ago

t was relative to the web root instead of ABSPATH. WP_Filesystem::exists() has ways of determining the root for $from that a straight include doesn't have.

$from was a absolute directory, on the FTP Server. On Windows & PHP-Su environments, it would've been absolute to the filesystems root coindicentally.

Ie. If the FTP root is /home/dd32/ then the $from in that case is /public_html/dd32.id.au/wordpress/.....

Note: See TracTickets for help on using tickets.