Page MenuHomePhabricator

wfEntryPointCheck() should clearly indicate what the problem is (PHP version, or missing vendor)
Closed, ResolvedPublic

Description

I'm on IRC with a developer who's seeing wfPHPVersionError():

MediaWiki 1.25 internal error

MediaWiki 1.25 requires at least PHP version 5.3.3, you are using PHP 5.5.21. Installing some external dependencies (e.g. via composer) is also required.
Supported PHP versions
Please consider upgrading your copy of PHP. PHP versions less than 5.3.0 are no longer supported by the PHP Group and will not receive security or bugfix updates.
If for some reason you are unable to upgrade your PHP version, you will need to download an older version of MediaWiki from our website. See our compatibility page for details of which versions are compatible with prior versions of PHP.
External dependencies
MediaWiki now also has some external dependencies that need to be installed via composer or from a separate git repo. Please see mediawiki.org for help on installing the required components.

To reproduce, move vendor/autoload.php out of the way, then browse a wiki page or run php maintenance/eval.php.

But

  • leading with a PHP incompatibility that isn't a problem for most developers is hella confusing.
  • if you're running HHVM the PHP download link is irrelevant
  • if you downloaded the tarball for 1.25.1, AIUI you *don't* have to run composer, it already has the modules you need and autoloader.php
    • the "see mediawiki.org" link is only for people who installed from git; if you downloaded the tarball and don't notice that it puts you halfway down Download_from_Git you'll do the wrong thing.

Expected results:

  • Tell the user what specifically went wrong and leave out the other errors.
  • Don't point HHVM users to php downloads.
  • Detect "You appear to be running from git" before linking to Download_from_git instructions.

The wfEntryPointCheck() in includes/PHPVersionCheck.php should pass an errorcode of the actual problem to wfPHPVersionError() which indexes ASCII and HTML error messages.

Event Timeline

Spage raised the priority of this task from to Needs Triage.
Spage updated the task description. (Show Details)
Spage added a subscriber: Spage.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 11 2015, 1:48 AM

Change 222917 had a related patch set uploaded (by Paladox):
Fix php check

https://gerrit.wikimedia.org/r/222917

Paladox added a subscriber: Paladox.Jul 5 2015, 4:04 PM

This is a bug in the core. The check for checking if you have php 5.3.3 or lower was removed so the check thought that all php were incompatible causing that error message.

TheDJ added a comment.Jul 5 2015, 9:04 PM

Eh yeah it was removed intentionally in https://gerrit.wikimedia.org/r/#/c/205001/

Suggest we fix the include/PHPVersionCheck.php, instead of reintroducing the unmaintainable version checks everywhere.

This comment was removed by Paladox.
Paladox set Security to None.Jul 5 2015, 11:44 PM
Paladox added a subscriber: aude.
Krinkle added a subscriber: Krinkle.Jul 6 2015, 7:52 AM

The problem is that wfEntryPointCheck() does a version check *and* a vendor-autoload check. And in both cases it calls wfPHPVersionError with the same arguments.

I suggest we split this function in two if-branches and pass an a parameter to wfPHPVersionError(), which then then display only the relevant part (e.g. only about version or only about external libraries).

Krinkle renamed this task from wfPHPVersionError() doesn't tell you what actually went wrong, and can be misleading about PHP versions composer. to wfEntryPointCheck() should clearly indicate what the problem is (PHP version, or missing vendor).Jul 6 2015, 7:53 AM

But tarball releases do not include vendor/autoload.php.

There needs to be a way to detect if you using a release that dosent include the autoload.php and one that does.

Harej removed a subscriber: Harej.Jul 6 2015, 2:35 PM
Krinkle added a comment.EditedJul 6 2015, 8:27 PM

But tarball releases do not include vendor/autoload.php.

Incorrect. Tarball releases do include vendor/autoload.php.

Paladox closed this task as Resolved.Jul 24 2015, 7:15 PM
Paladox assigned this task to TheDJ.
Paladox removed TheDJ as the assignee of this task.
Paladox removed a project: Patch-For-Review.
Paladox removed a subscriber: gerritbot.

I am not show who to add to assigned to.

Change 222917 merged by jenkins-bot:
Rework PHP and vendor check

https://gerrit.wikimedia.org/r/222917