WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#11464 closed defect (bug) (fixed)

MySQL 4.1.2 and automatic update...

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

Download all attachments as: .zip

Change History (52)

comment:1 @nickbohle6 years ago

  • Component changed from General to Upgrade/Install

comment:2 @westi6 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.

@westi6 years ago

A possible fix - as yet untested

comment:3 @Denis-de-Bernardy6 years ago

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

comment:4 follow-up: @Denis-de-Bernardy6 years ago

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

comment:5 in reply to: ↑ 4 @westi6 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 ;-)

comment:6 @westi6 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.

comment:7 @scribu6 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)"

comment:8 @nickbohle6 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.

comment:9 @Denis-de-Bernardy6 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-Bernardy6 years ago

highlight the versions

comment:10 @Denis-de-Bernardy6 years ago

This would point the user into the right direction.

please try the new one.

@Denis-de-Bernardy6 years ago

woops

@Denis-de-Bernardy6 years ago

fix the typo as well

comment:11 follow-up: @nacin6 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.

comment:12 in reply to: ↑ 11 @nickbohle6 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

comment:13 @nacin6 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.

comment:14 @azaozz6 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

comment:15 follow-up: @nacin6 years ago

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

comment:16 in reply to: ↑ 15 @Denis-de-Bernardy6 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.

comment:17 @nacin6 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.

@nacin6 years ago

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

@caesarsgrunt6 years ago

comment:18 @caesarsgrunt6 years ago

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

@nacin6 years ago

comment:19 @nacin6 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.

comment:20 @ryan6 years ago

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

comment:21 @ryan6 years ago

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

comment:22 @ryan6 years ago

Updated the nightly builds so that have the new checks.

comment:23 @TobiasBg6 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.

comment:25 @nacin6 years ago

Oops, that was me. :)

comment:26 @ryan6 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'.

comment:27 @ryan6 years ago

That looks better. Working here.

comment:28 @nickbohle6 years ago

Works like a charm!

comment:29 @ryan6 years ago

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

Thanks everyone. Resolving as fixed.

comment:30 @ryan6 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.

@ryan6 years ago

Future proof translations

@nacin6 years ago

@nacin6 years ago

comment:31 @ryan6 years ago

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

comment:32 @ryan6 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

comment:33 @nacin6 years ago

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

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

comment:34 follow-up: @Denis-de-Bernardy6 years ago

could r12343 potentially update the wp_db_version option in weird circumstances?

comment:35 in reply to: ↑ 34 ; follow-up: @nacin6 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?

comment:36 in reply to: ↑ 35 @Denis-de-Bernardy6 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. :-)

comment:37 @ryan6 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.

comment:38 @ryan6 years ago

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

comment:39 @ryan6 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.

comment:40 @ryan6 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.

comment:41 @caesarsgrunt6 years ago

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

comment:42 @dd326 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.