Page MenuHomePhabricator

Merge taint-check-plugin into mediawiki-phan-config
Closed, ResolvedPublic

Description

Right now we run them in separate jobs because taint-check is still sort of experimental, and because it was built on top of an old version of phan. However, things have changed in the meanwhile, and IMHO we're ready for that step. Notably, this would have two benefits:

  • We would avoid the extra key in composer.json for seccheck
  • We'd use less CI resources (running one phan job instead of two)

This is necessary because otherwise we'd have to require two (possibly different) versions of phan in composer.json (see T235053#5571224 for details).

In order to resolve this task, we should first release 3.0.0 (T235383), which uses the same version as mw-phan does right now. Then require it from mw-phan's composer.json and release a new version of mw-phan. Then cleanup CI config/dockerfiles.

Plan

  • Wait for taint-check 3.0.1, which will use the same phan version as mw-phan.
  • Add taint-check to the require section of mw-phan's composer.json, and load it from the 'plugins' option in the cfg file, importing some required settings.
  • Add some kind of testing for taint-check within mw-phan. E.g. find a way to run taint-check tests, or T226117
  • Tag a new version of phan
  • Update docs on mw.org.
  • T248630: Upgrade phan to 0.10.2 and remove phan-taint-check-plugin

Notably, this will force us to keep mw-phan and taint-check on the same phan version. That shouldn't be too hard, though, as long as both are constantly kept up-to-date.

Event Timeline

Daimona created this task.Oct 13 2019, 5:05 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 13 2019, 5:05 PM
sbassett triaged this task as Medium priority.Oct 15 2019, 7:06 PM
Daimona updated the task description. (Show Details)Oct 24 2019, 11:18 AM
Daimona updated the task description. (Show Details)Nov 3 2019, 2:52 PM

Change 563669 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/tools/phan/SecurityCheckPlugin@master] [WIP] Add a config file with required options only

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

Daimona updated the task description. (Show Details)Jan 11 2020, 1:08 PM

Change 574190 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/tools/phan@master] [WIP][POC] Require taint-check

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

Change 563669 abandoned by Daimona Eaytoy:
[WIP] Add a config file with required options only

Reason:
Per comment above

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

Daimona updated the task description. (Show Details)Mar 26 2020, 5:10 PM
Daimona updated the task description. (Show Details)

Change 583696 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[labs/libraryupgrader@master] Remove phan-taint-check-plugin from extras if new mediawiki-phan-config present

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

Change 574190 merged by jenkins-bot:
[mediawiki/tools/phan@master] Require taint-check

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

Change 583761 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/tools/phan@master] Release 0.10.0

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

Change 583761 merged by jenkins-bot:
[mediawiki/tools/phan@master] Release 0.10.0

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

Jdforrester-WMF updated the task description. (Show Details)
Daimona closed this task as Resolved.Mar 26 2020, 11:01 PM
Daimona claimed this task.

Closing in favour of T248630 for the remaining part.