Page MenuHomePhabricator

Upgrade php-ast to 0.1.4
Open, Needs TriagePublic

Description

CI currently runs with version 0.1.2 of php-ast, as can be seen in https://integration.wikimedia.org/ci/job/composer-package-php70-docker/5497/console. However, we need version 0.1.4 (at least) to test seccheck, and 0.1.2 is very old anyway.

Event Timeline

Daimona created this task.Mar 19 2019, 6:58 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 19 2019, 6:58 PM

@Legoktm could you please take a look?

Will the current plugin continue to work properly with ast 0.1.4? Or do we also need to keep 0.1.2 for back/compat reasons?

@Legoktm I think the current version should work with 0.1.4. But actually, could you please add 0.1.5 as well? We'll need it for the next upgrade. Thanks!

@Legoktm I think the current version should work with 0.1.4.

Gotcha. I can do that then.

But actually, could you please add 0.1.5 as well? We'll need it for the next upgrade. Thanks!

Building newer versions of php-ast is trivial, the main problem is determining which version to use when running phan. If you look at https://gerrit.wikimedia.org/r/plugins/gitiles/integration/config/+/master/dockerfiles/mediawiki-phan/run.sh#36 you can see how I worked around this for standard phan.

I suppose we coud look at the phan version number being installed and do some conditional based on that.

@Legoktm Cool, thanks! I think for now you can use whatever hack comes more handy for seccheck, as at the end of the upgrade we should ideally have a single version installed.
A conditional checking the required phan version seems fine.

From a CI perspective, we need to keep supporting older branches of MediaWiki that we run tests against, so the "migration" of php-ast from my perspective only ends once the last branch using the older version is phased out.

@Legoktm Right. But anyway at the end of the migration we'll only have 0.1.4 for the current version, and ?[0] for the final version based on 1.2.x.

[0]: To be determined according to phan's requirement, I guess 1.0.x as in the phan job.

hashar renamed this task from Upgrade ext-ast to 0.1.4 to Upgrade php-ast to 0.1.4.Mar 20 2019, 8:27 AM
hashar updated the task description. (Show Details)
hashar added a subscriber: hashar.Mar 20 2019, 8:34 AM

Tentatively that has been solved already?

We have T174339 to upgrade to phan 0.8.5, which requires a newer version of php-ast than the one we had.

T174338: Provide php-ast 0.1.5 or later as a Debian package for CI was the task to get a newer one, I have forked the Debian repository back in January 2018 and rebuild it but the resulting Debian package never got published. I got distracted by an unrelated build failure.

Debian Stretch ships with 0.1.2, but testing has 0.1.6 so we can probably ask Operations to rebuild it for us and publish it to apt.wikimedia.org . As noted, the package is also available in sury.org, but that is not ideal (that might brings a lot more different packages as well).

Finally cscott found that Phan provides a parser to replace php-ast when it is not present phan --allow-polyfill-parser. And it seems to be deemed good enough.

From a CI perspective, we need to keep supporting older branches of MediaWiki that we run tests against, so the "migration" of php-ast from my perspective only ends once the last branch using the older version is phased out.

Potentially we could use per branches jobs that would be stalled to some arbirtary old version of php-ast/phan. However the current system is centrally managed which is a bit of a maintenance pain. For jobs that use php-ast we could only trigger them for the master/wmf branch, but most of the jobs should probably just loose the php-ast requirements via phan --allow-polyfill-parser.

We have T174339 to upgrade to phan 0.8.5, which requires a newer version of php-ast than the one we had.

IIUC that's for the phan job itself which yes, already has php-ast v.1.0.1 installed.

Debian Stretch ships with 0.1.2, but testing has 0.1.6 so we can probably ask Operations to rebuild it for us and publish it to apt.wikimedia.org . As noted, the package is also available in sury.org, but that is not ideal (that might brings a lot more different packages as well).

I think we should proceed with little steps to avoid breaking anything, and thus upgrade to 0.1.4 first, then 0.1.5 (and then ...?).

Finally cscott found that Phan provides a parser to replace php-ast when it is not present phan --allow-polyfill-parser. And it seems to be deemed good enough.

True, but it's been introduced in 0.8.13, which means that we still need ast until we get there.

@Legoktm I checked all the remaining versions up to 1.2.6 and I can now confirm that we only need 0.1.5 for php-ast. 0.1.2 should be kept for the current version only.