Page MenuHomePhabricator

Move taint-check to require-dev in composer.json
Closed, DeclinedPublic


The dependency on PHP == 7.0.0 has gone since T207344, and since we don't support HHVM anymore, taint-check can be moved to require-dev, like we're doing for phan (T220589).

Event Timeline

Note that we don't need to change anything in seccheck itself, because it already requires phan. We only need to update our CI, and then all repos

Huh, the dependency on php-ast is however a problem. Both for CI and local installs. However, given that phan doesn't require it and it provides a fallback, we can probably remove the requirement from seccheck's composer.json - as long as we use phan wrappers around php-ast stuff.

Change 541889 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/tools/phan/SecurityCheckPlugin@master] Remove explicit dependency on ext-ast

Uhhhh, I just realized that there's a huge problem in doing this: we'd be requiring two versions of phan at the same time. Right now, it'd be 2.2.13 for mediawiki-phan-config, and 1.3.2 for taint-check.

I think the only viable solution is to make both use the same version, and add a constraint on it. The most obvious solution would be to have mw-phan-config require seccheck, and then merge the two jobs. It will still be possible for every repo to disable seccheck (by changing the plugins option in the config).

sbassett triaged this task as Medium priority.Oct 15 2019, 7:06 PM

Change 541889 merged by jenkins-bot:
[mediawiki/tools/phan/SecurityCheckPlugin@master] Remove explicit dependency on ext-ast