WordPress.org

Make WordPress Core

Opened 9 years ago

Closed 6 years ago

#11639 closed enhancement (fixed)

In core upgrader, replace the hard-coded required PHP/MySQL versions

Reported by: nacin Owned by: dd32
Milestone: 3.5 Priority: normal
Severity: normal Version: 2.9
Component: Upgrade/Install Keywords: has-patch commit early
Focuses: Cc:

Description

Related: #11464... This originated there.

wp-admin/includes/update-core.php, which gets copied over after download during an upgrade, has a new check in 2.9 to ensure that the MySQL and PHP versions are what is required by the downloaded version.

Originally, a patch was committed that pulled in the downloaded version.php file to check what the new version required. (That patch also added the $required_mysql_version and $required_php_version globals.)

This part got reverted because the include path wasn't right (and only coincidentally correct on the systems it was tested on before commit), and so for 2.9 the values were hard-coded.

Attached is a patch that gets it right. It has been tested on an environment that would have failed the first time around.

I have also attached a separate diff that includes the core hacks required to test this, as otherwise the downloaded update-core.php will replace your patched update-core.php.

This patch fixes #11638 as well, but that fix belongs in 2.9.1.

Attachments (4)

11639-update-core.diff (1.6 KB) - added by nacin 9 years ago.
11639-testing-only.diff (1.3 KB) - added by nacin 9 years ago.
For testing only: Prevents the downloaded update-core.php from being copied over the patched update-core.php. Also in here is code to force wpdb::db_version to report < 4.1.2
11639-update-core.2.diff (1.5 KB) - added by nacin 9 years ago.
$wp_filesystem->copy returns false, not WP_Error.
11639-update-core.3.diff (2.9 KB) - added by nacin 9 years ago.
Includes the better strings introduced in #11666.

Download all attachments as: .zip

Change History (21)

@nacin
9 years ago

For testing only: Prevents the downloaded update-core.php from being copied over the patched update-core.php. Also in here is code to force wpdb::db_version to report < 4.1.2

#1 @nacin
9 years ago

  • Milestone changed from Unassigned to 3.0
  • Version set to 2.9

@nacin
9 years ago

$wp_filesystem->copy returns false, not WP_Error.

#2 @nacin
9 years ago

dd32 pointed out that $wp_filesystem->copy() returns false on failure, unlike $wp_filesystem->copy_dir(), which returns a WP_Error.

Patch 2 corrects that.

#3 follow-up: @miqrogroove
9 years ago

#11666 Added a bunch of duplicated code. I was surprised this wasn't already centralized. So, can this ticket be expanded to combine upgrade-core.php and upgrade.php where appropriate?

#4 in reply to: ↑ 3 @nacin
9 years ago

Replying to miqrogroove:

#11666 Added a bunch of duplicated code. I was surprised this wasn't already centralized. So, can this ticket be expanded to combine update-core.php and upgrade.php where appropriate?

Unfortunately, no. We could standardize between upgrade.php and install.php, but update-core.php is copied by an old installation from a new installation. So a blog running 2.7 copies over the 2.9 update-core.php file, which then installs 2.9 over the 2.7 files. This patch would cause us to also copy over version.php from the incoming package so we can ascertain new requirements.

Looking at upgrade.php and install.php, there really isn't much to dry up. The strings are different and it looks like the logic is handled a bit differently. At most, we could add a function to update-core.php, and upgrade.php and install.php could include it, but I don't see the point.

I do like the new strings from upgrade.php for use in update-core.php, though, and I may suggest we copy them over. Of course, since the file is being copied over, these strings are only future proof when used in update-core.php, so a blog running 2.9 (or 2.9.1 if we change it) will have them, but a blog running less than 2.9 and upgrading won't have them.

At least by 3.0 we won't be hard-coding the requirements anywhere.

@nacin
9 years ago

Includes the better strings introduced in #11666.

#5 @nacin
9 years ago

New patch also includes the better upgrade strings from #11666.

#6 @voyagerfan5761
9 years ago

  • Cc WordPress@… added

#7 @dd32
9 years ago

Copying the file to WP_CONTENT_DIR and then including it seems to be a reliable way to get rid of those hard coded constants. I've not tested the patch, but logically, it looks like the right thing to do.

#8 @nacin
9 years ago

For the record, something I apparently didn't mention, this patch was based off the code that copies over the downloaded update-core.php and includes it to run the installation.

#10 @nacin
8 years ago

  • Keywords commit added; needs-testing removed

Anyone upgrading from 2.7.0 to 2.9.x won't be affected by #11562. Thus, we'll need to continue to bump update-core.php with each new WP version. I'm rather ambivalent about this but it can't really hurt and makes use of our globals.

#11 @nacin
8 years ago

  • Keywords early added
  • Milestone changed from 3.0 to 3.1

We'll continue to hard-code the version bumps in admin/includes/update-core.php, and handle this in 3.1 early so we don't need to hard-code them forever.

We discussed doing a version_compare, as if we're upgrading from 3.0, the new core update API won't allow us to even download the package.

#12 @nacin
8 years ago

  • Milestone changed from Awaiting Triage to 3.1

#13 @jane
8 years ago

  • Milestone changed from 3.1 to Future Release

Punting due to beta.

#14 @nacin
6 years ago

  • Milestone Future Release deleted
  • Resolution set to maybelater
  • Status changed from new to closed

#15 @dd32
6 years ago

I've run into this again, there's a up-to-date patch sitting on #21789 (21789.diff) right now which does it slightly different (but mostly the same).

#16 @dd32
6 years ago

  • Milestone set to 3.5
  • Resolution maybelater deleted
  • Status changed from closed to reopened

Re-opening so as to fix #21789 and #22015

#17 @dd32
6 years ago

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

In [22227]:

Replace the hard coded version and PHP/MySQL requirements in update-core.php by pulling in the new version.php file during installation. Fixes #22015 #11639. See #21789

Note: See TracTickets for help on using tickets.