Page MenuHomePhabricator

Allow use of phan 0.8.5+ in wikimedia CI
Closed, ResolvedPublic

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 28 2017, 12:31 PM
Addshore changed the task status from Open to Stalled.Aug 28 2017, 12:35 PM

A script to allow fetching a specified phan version was added in https://gerrit.wikimedia.org/r/#/c/371095/
This is used in https://gerrit.wikimedia.org/r/#/c/371097
Wikibase trials setting a phan version in https://gerrit.wikimedia.org/r/#/c/371046

Currently blocked on the need to have a newer version of ast for CI (see subticket)

Change 371097 had a related patch set uploaded (by Addshore; owner: Addshore):
[integration/config@master] Get phan version from composer.json

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

Change 371046 had a related patch set uploaded (by Addshore; owner: WMDE-leszek):
[mediawiki/extensions/Wikibase@master] Use Phan 0.8.5

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

Addshore renamed this task from Allow use of phan 0.8.5 in wikimedia CI to Allow use of phan 0.8.5+ in wikimedia CI.Jan 13 2018, 11:41 PM
Addshore changed the task status from Stalled to Open.
Addshore claimed this task.

Change 404142 had a related patch set uploaded (by Addshore; owner: Addshore):
[integration/config@master] docker: update phan image to phan 0.8.10

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

Addshore triaged this task as Normal priority.Jan 23 2018, 7:44 PM

In the repository's composer.json we should add something like:

"extra": {
    "phan-version": "0.8.0",
},

But even before that we need to update the docker image to not hardcode a specific version of phan.

But even before that we need to update the docker image to not hardcode a specific version of phan.

Or multiple docker images for different version of phan (as currently phan doesn't install at runtime)

I was thinking of having it install phan at runtime.

Addshore removed Addshore as the assignee of this task.Aug 17 2018, 3:08 PM
Addshore moved this task from Back Burner 🏛️ to Watching 👀 on the User-Addshore board.
hashar added a subscriber: hashar.

Sury now provides php-ast 0.1.6

Change 460199 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[integration/config@master] mediawiki-phan: Update to php-ast 0.1.6 from packages.sury.org

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

Change 460199 merged by jenkins-bot:
[integration/config@master] mediawiki-phan: Update to php-ast 0.1.6 from packages.sury.org

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

The new php-ast breaks the existing phan we have, so I haven't rolled it out yet:

https://integration.wikimedia.org/ci/job/mwext-php70-phan-docker/13214/console

06:13:45 + /mediawiki/tests/phan/bin/phan /mediawiki/extensions/3D -m checkstyle
06:13:47 RecursiveDirectoryIterator::__construct(includes/): failed to open dir: No such file or directory
06:13:47 RecursiveDirectoryIterator::__construct(maintenance/): failed to open dir: No such file or directory
06:13:47 RecursiveDirectoryIterator::__construct(tests/phan/stubs/): failed to open dir: No such file or directory
06:13:47 PHP Warning:  DOMDocument::loadXML(): Start tag expected, '<' not found in Entity, line: 1 in /mediawiki/tests/phan/bin/postprocess-phan.php on line 69
06:13:47 <?xml version="1.0"?>
06:13:47 [CHECKSTYLE] Collecting checkstyle analysis files...
06:13:47 [CHECKSTYLE] Searching for all files in /srv/jenkins-workspace/workspace/mwext-php70-phan-docker that match the pattern log/phan-issues
06:13:47 [CHECKSTYLE] No files found. Configuration error?

I didn't try debugging locally yet, will give that a shot tomorrow.

Addshore added a comment.EditedSep 13 2018, 7:07 AM

I mean, we don't have to use the new version of ast with our current version on phan though do we?
We can continue using the old version, that's one of the joys of versioned images right?

I mean, we don't have to use the new version of ast with our current version on phan though do we?
We can continue using the old version, that's one of the joys of versioned images right?

Kinda. The main problem is that right now is that the version of phan is hardcoded in CI. So if we do an upgrade, we have to manage updating 47 repositories at once (impossible), and then we broke all old branches too. So the best way to do this is move the phan version into each repository (similar to phan-taint-check-plugin), and then allow each repo to upgrade at their own pace. But of course to allow the upgrade we need the newer php-ast, so i was trying to see how easy it would be to get old phan to work with new php-ast...

We're running into https://github.com/phan/phan/issues/1134

Failed to parse line: /srv/phan/vendor/phan/phan/src/Phan/Analysis.php:44 [8192] ast\parse_file(): Version 30 is deprecated
Failed to parse line: #0  phan_error_handler()
Failed to parse line: #1  ast\parse_file() called at [/srv/phan/vendor/phan/phan/src/Phan/Analysis.php:44]
Failed to parse line: #2  Phan\Analysis::parseFile() called at [/srv/phan/vendor/phan/phan/src/Phan/Phan.php:107]
Failed to parse line: #3  Phan\Phan::analyzeFileList() called at [/srv/phan/vendor/phan/phan/src/phan.php:32]
Failed to parse line: #4  require_once(/srv/phan/vendor/phan/phan/src/phan.php) called at [/srv/phan/vendor/phan/phan/phan:2]

Change 371097 abandoned by Addshore:
Get phan version from composer.json

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

Change 404142 abandoned by Addshore:
docker: update phan image to phan 0.8.10

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

Legoktm claimed this task.Jan 18 2019, 7:45 PM

It's not pretty, but I figured out a full, non-breaking, upgrade path.

Change 485241 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[integration/config@master] mediawiki-phan: Support newer versions of php-ast and phan

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

Here's the summary on how to upgrade:

CI will look at whether .extra.phan is in composer.json on whether to run "new-style" phan (newer ast) or "old-style" phan (older ast), so the migration will need to be done in one commit. Also new style will skip the bash wrapper in MediaWiki core and directly execute /srv/phan/vendor/bin/phan -d . -p. My understanding is that the wrapper was only needed for per-line suppression, which is now built into phan itself.

Once a few extensions have been migrated to the new version, we can do the CI work and upgrade core.

cc @Addshore @Umherirrender @EBernhardson for feedback on the plan.

For mediawiki-codesniffer the upstream version is always bundled with a new version. That is needed to install it correctly with composer.

It is possible to bundle the phan version with phan-config? That would it make easier to write own plug-ins and be sure to have the correct phan version to run.
I have no idea if the extra key of composer.json is preserved on publish process and can be read by CI than or if there must be another json file to just store the version for CI

It is possible to bundle the phan version with phan-config? That would it make easier to write own plug-ins and be sure to have the correct phan version to run.

That's a good point, we should do that. I think we can put it into composer.json for mediawiki-phan-config...I'll look into that.

For mediawiki-codesniffer the upstream version is always bundled with a new version. That is needed to install it correctly with composer.
It is possible to bundle the phan version with phan-config? That would it make easier to write own plug-ins and be sure to have the correct phan version to run.
I have no idea if the extra key of composer.json is preserved on publish process and can be read by CI than or if there must be another json file to just store the version for CI

We have to start distributing the composer.json with the package now, see https://gerrit.wikimedia.org/r/c/mediawiki/tools/phan/+/492521/.

CI will look for extra.phan in the repository itself, then look for extra.phan in the mediawiki-phan-config package.

Change 485241 merged by jenkins-bot:
[integration/config@master] mediawiki-phan: Support newer versions of php-ast and phan

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

Change 492524 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[integration/config@master] Use mediawiki-phan:0.1.9 for mwext-phan-*

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

Change 492524 merged by jenkins-bot:
[integration/config@master] Use mediawiki-phan:0.1.9 for mwext-phan-*

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

Umherirrender closed this task as Resolved.May 29 2019, 5:14 PM

Now on 1.3.4