Page MenuHomePhabricator

Vector's php-composer-test job is not running anything
Closed, ResolvedPublic


00:23:26 + /srv/deployment/integration/composer/vendor/bin/composer test
00:23:27 This version of the Vector skin requires MediaWiki 1.25+...
00:23:27 Time: 484ms; Memory: 15Mb
00:23:27 Finished: SUCCESS

This is because the composer autoloader is including Vector.php, which will die if the wfLoadExtension function does not exist (which it will not outside the context of MediaWiki...).

Event Timeline

Legoktm created this task.Jun 25 2015, 7:34 AM
Legoktm raised the priority of this task from to Needs Triage.
Legoktm updated the task description. (Show Details)
Legoktm added subscribers: Legoktm, hashar, polybuildr, bd808.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 25 2015, 7:34 AM

Right, this was embarrassing.

I spent some time on #composer looking into this and talking to ethanol/alcohol, one of composer's developers.

The issue essentially is that since we put Vector.php into composer.json's autoload field, anything that requires vendor/autoload.php is going to also include Vector.php. Unfortunately, jakub-onderka/php-parallel-lint includes ../../autoload.php and so composer test just die()s. (Not sure if CodeSniffer requires it too.)

I think the easiest and most direct solution is the one ethanol suggested on IRC: check in Vector.php whether it is being executed in a CLI environment, and if it, then don't die. This seems okay because there is no common situation where Vector.php will be included in a CLI environment, except maybe in maintenance/eval.php.

If there's a different approach entirely that might be more suitable, we can discuss that here. In the meantime, I'll submit a patch.

Change 220725 had a related patch set uploaded (by Polybuildr):
Vector.php: Don't die on missing wfLoadSkin if in CLI environment

Change 220725 merged by jenkins-bot:
Don't autoload Vector.php using composer.json

Legoktm closed this task as Resolved.Jun 25 2015, 9:31 PM
polybuildr set Security to None.

Change 221183 had a related patch set uploaded (by Paladox):
Don't autoload Vector.php using composer.json

Change 221183 abandoned by Paladox:
Don't autoload Vector.php using composer.json