WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

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

Download all attachments as: .zip

Change History (52)

comment:1 @nickbohle5 years ago

  • Component changed from General to Upgrade/Install

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

@westi5 years ago

A possible fix - as yet untested

comment:3 @Denis-de-Bernardy5 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-Bernardy5 years ago

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

comment:5 in reply to: ↑ 4 @westi5 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 @westi5 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 @scribu5 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 @nickbohle5 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-Bernardy5 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-Bernardy5 years ago

highlight the versions

comment:10 @Denis-de-Bernardy5 years ago

This would point the user into the right direction.

please try the new one.

@Denis-de-Bernardy5 years ago

woops

@Denis-de-Bernardy5 years ago

fix the typo as well

comment:11 follow-up: @nacin5 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 @nickbohle5 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 @nacin5 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 @azaozz5 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: @nacin5 years ago

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

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

@nacin5 years ago

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

@caesarsgrunt5 years ago

comment:18 @caesarsgrunt5 years ago

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

@nacin5 years ago

comment:19 @nacin5 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 @ryan5 years ago

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

comment:21 @ryan5 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 @ryan5 years ago

Updated the nightly builds so that have the new checks.

comment:23 @TobiasBg5 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 @nacin5 years ago

Oops, that was me. :)

comment:26 @ryan5 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 @ryan5 years ago

That looks better. Working here.

comment:28 @nickbohle5 years ago

Works like a charm!

comment:29 @ryan5 years ago

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

Thanks everyone. Resolving as fixed.

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

@ryan5 years ago

Future proof translations

@nacin5 years ago

@nacin5 years ago

comment:31 @ryan5 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 @ryan5 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 @nacin5 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-Bernardy5 years ago

could r12343 potentially update the wp_db_version option in weird circumstances?

comment:35 in reply to: ↑ 34 ; follow-up: @nacin5 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-Bernardy5 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 @ryan5 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 @ryan5 years ago

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

comment:39 @ryan5 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 @ryan5 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 @caesarsgrunt5 years ago

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

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