Page MenuHomePhabricator

Decide on future of running Phan tests on release branches
Open, Needs TriagePublic

Description

Following on from T226766: Avoid explicit commit references in composer.json in code meant for public consumption, T226156: Phan job fails on CI for mediawiki/core REL1_3[12], https://gerrit.wikimedia.org/r/#/c/integration/config/+/519793/ and the mediawiki-core-php72-phan-docker jobs

Phan in 1.31 and 1.32 won't run on mediawiki-core-php72-phan-docker because it has PHP 7.2, not 7.0.

As such, the phan jobs have been completely disabled in the branch, pending a longer term solution.

I don't believe we want to continue running jobs on PHP 7.0... So the options are to leave things disabled, or backporting whatever is necessary to run phan compatible with PHP 7.2 to 1.31 and 1.32

Event Timeline

Reedy created this task.Jun 30 2019, 9:35 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 30 2019, 9:35 PM

Yeah, I'd propose just running them on master.

Reedy added a comment.Jul 3 2019, 12:23 PM

Yeah, I'd propose just running them on master.

Doesn't seem unreasonable. Are we going to blanket disable on branches? Or just disable when it stops working due to external factors?

I think we don't really need it on WMF branches (though, in most cases if master works, they should too) as they're fairly short lived too, and we're generally picking from master anyway

Do we need to document it anywhere?

hashar added a subscriber: hashar.Jul 24 2019, 5:14 PM

A real life example https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/UserMerge/+/525286/ for REL1_33

Package mediawiki/phan-taint-check-plugin at version 1.5.1 has a PHP requirement incompatible with your PHP version (7.2.16)

So I guess for now we should apply a branch filter to the job mwext-php72-phan-seccheck-docker so that it only runs against master. For the old branches, we would need to keep providing some php7.0 based container.

I'd like to make sure they keep running on wmf branches at least. Especially our practice of cherry picking things quite frequently, it becomes quite likely that an implicit dependency gets forgotten in a way that isn't detected by Git (e.g. not in the same diff context).

Phan is pretty much our only line of defence there to detect things like missing variables and (renamed) classes etc.

Change 525878 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] layout: Restrict Phan jobs to only run on master, wmf/, & fundraising/ branches

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

We absolutely need phan for release branches. Many of the regressions in previous releases could've been caught if phan were running.

We absolutely need phan for release branches. Many of the regressions in previous releases could've been caught if phan were running.

People pushing commits directly into legacy release branches, rather than back-ports of merged and tested code in master, are already risking quite a bit, but this is a fair concern.

Given that we're able to backport fixes to make other CI infra upgrades work, I think we should be able to do the same for phan. The easy way is to keep the php70 containers around until we drop support for those branches.

Alternatively we could consider looking into using phan's pure-PHP fallback/polyfill parser so we don't need to mess with php-ast versions as much. But that comes with its own problems (though I'm skeptical about what the performance will be like).

Change 534426 had a related patch set uploaded (by Hashar; owner: Hashar):
[integration/config@master] zuul: disable phan on extensions release branches

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

https://gerrit.wikimedia.org/r/#/c/integration/config/+/534426 disables the phan job for extensions/skins on release branches due to T231966

Change 534426 merged by jenkins-bot:
[integration/config@master] zuul: disable phan on extensions release branches

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

Mentioned in SAL (#wikimedia-releng) [2019-09-04T12:20:37Z] <hashar> Dropped Phan jobs for extensions from release branches T226945

Daimona added a subscriber: Daimona.Wed, Sep 4, 2:55 PM
Krinkle moved this task from Inbox to Checkers on the MediaWiki-Core-Testing board.Wed, Sep 4, 5:48 PM