Page MenuHomePhabricator

Run phan secheck on PHP 7.2, not PHP 7.0
Closed, ResolvedPublic

Description

Split from main.

Event Timeline

Change 517749 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] layout: Drop php70-phan-seccheck entirely

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

Change 517750 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] jjb: Drop php70-phan-seccheck jobs, unused

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

Jdforrester-WMF changed the task status from Open to Stalled.Jun 25 2019, 11:08 PM

Stalled on blockers.

Daimona changed the task status from Stalled to Open.Jul 3 2019, 8:10 AM
Daimona subscribed.

Copying what I said in T227172: the current version (1.5.1) requires PHP70 jobs, while the new version (2.0.0) can run on PHP70+. I believe the easier way to proceed is to first upgrade seccheck in every repo, keeping the PHP70 job alone. When all repos are upgraded (or also gradually, whatever you prefer), we'll be able to switch to PHP72 jobs and get rid of the PHP72.

Note that something that can already be done is upgrade php-ast for both seccheck jobs, in the following way:

  • PHP71+ jobs: These should only have php-ast 1.0.1, and T218719 should provide anything needed for the upgrade
  • PHP70 job: This one is more complicated. It should behave the same as the phan job: if there's seccheck <=1.5.1 in composer.json, it should use php-ast 0.1.2. If there's seccheck 2.0.0 it should use php-ast 1.0.0.

Given the above, and the fact that upgrading ast may be a little harder than expected (as it happened in T218719), I'm un-stalling this task in case someone is interested in giving it a try.

This is directly blocked by T227172 which is itself Stalled, so surely this is also Stalled?

@Jdforrester-WMF Well, T227172 is definitely a blocker and for that part yes, this one is stalled. However, as I said above, the seccheck containers have to be updated with the new php-ast, and that's something that can be done regardless of T227172. And actually, that task needs the PHP70 part described above, so they're blocking each other more or less.

Anyway, I marked this as Stalled on T218719 in the first place. :-)

Yeah that makes sense :) TBH I thought that upgrading ast would have been easier, but if there's no clear solution then yes, I guess this can go back stalled.

Jdforrester-WMF changed the task status from Open to Stalled.Jul 3 2019, 9:29 PM

Yeah, I thought it'd be easy too. :-(

Change 521863 had a related patch set uploaded (by Jforrester; owner: Daimona Eaytoy):
[integration/config@master] dockerfiles: [mediawiki-phan-seccheck] Upgrade to PHP 7.2

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

Change 522209 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] jjb: Point seccheck jobs at mediawiki-phan-seccheck:0.4.0

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

Change 521863 merged by jenkins-bot:
[integration/config@master] dockerfiles: [mediawiki-phan-seccheck] Upgrade to PHP 7.2

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

Change 522209 merged by jenkins-bot:
[integration/config@master] jjb: Point seccheck jobs at mediawiki-phan-seccheck:0.4.0

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

Mentioned in SAL (#wikimedia-releng) [2019-07-12T00:32:50Z] <James_F> Zuul: Migrate from php70-phan-seccheck to php72-phan-seccheck T226420

Change 517750 merged by jenkins-bot:
[integration/config@master] jjb: Drop php70-phan-seccheck jobs, unused

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