Page MenuHomePhabricator

Bundle CheckUser extension with MediaWiki
Closed, ResolvedPublic

Description

  • Passed discussion or already Wikimedia deployed
  • Passed security review or already Wikimedia deployed
  • Voting CI structure tests
  • Runs MediaWiki-CodeSniffer
  • Runs phan
  • Supports MySQL, SQLite, and Postgres (if there are schema changes)
  • GPL v2 or later compatible license
  • Extension's default configuration provides optimal experience
  • Tested with web installer
  • Any relevant dependencies also bundled

Details

Related Changes in Gerrit:
Related Changes in GitLab:
TitleReferenceAuthorSource BranchDest Branch
settings.yaml: Promote CheckUser to being bundledrepos/releng/release!159reedybundle-checkusermain
Customize query in GitLab

Event Timeline

What do we actually need to do for "Voting CI structure tests"?

I note ext-openssl would be a new hard dependancy requirement; it's a suggest in MW core.

I'd presume most installs probably would have it...

Quoting from https://www.mediawiki.org/wiki/Suggestions_for_extensions_to_be_integrated/Checklist for "Voting CI structure tests":

The Git repository with the extension/skin's code must run the MediaWiki "structure" tests, which validates that extension features are registered and working correctly (E.g. API modules, content models, ResourceLoader modules).

If the repository has a CI job that runs core PHPUnit tests, then the structure tests are likely being run as well.

"Voting" means that the CI job is part of the required "V+2" check before commits can merge (e.g. not an advisory "non-voting" job).

Voting structure tests seems to be the case (although I could not seem to get them to trigger an error). https://integration.wikimedia.org/ci/job/quibble-vendor-mysql-php74-noselenium-docker/74217/console#console-section-7 ran and PHPUnit reported that:

You should really speed up these slow tests (>50ms)...
13:00:36  1. 1791ms to run SpecialPageFatalTest::testSpecialPageDoesNotFatal
...

Which suggests that the structure tests are being run.

Change 872965 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/core@master] Add firebase/php-jwt and ext-openssl to composer.json

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

Tested with the web installer on MediaWiki 1.39.1 tarball and CheckUser REL1_39. I added ext-openssl and firebase/php-jwt to the core composer.json (as would be done in 1.40 if the associated patch is merged). Was able to install CheckUser and run a check without making any configuration changes from the command line. Investigate was not loaded as by default it is not loaded. IMO that should change to be by default true, as Investigate also works by after using the web installer (after adding $wgCheckUserEnableSpecialInvestigate = true;).

IMO Investigate should be enabled by default so that "Extension's default configuration provides optimal experience", so added doing this as a subtask.

I added ext-openssl and firebase/php-jwt to the core composer.json (as would be done in 1.40 if the associated patch is merged).

That wouldn't be the case; it would be added to the composer.json in MediaWiki-Vendor, which includes stuff for WMF deployment, and then when release branches are created, trimmed for the bundled extensions in the tarball.

Okay, thanks for the info. I presume that means that my patch to core is not the right way to have this bundled?

Indeed not.

Something like rMREL74efc81a78f9: Add Linter to the MediaWiki bundle is all that's needed.

Probably need a bigger discussion around what we want to do for ext-openssl. If we just add it to composer.json in MediaWiki-Vendor it's basically a no-op...

I don't know if any other bundled extension has an ext- requirements that aren't already required in MediaWiki core. And/or whether we've just added them to MediaWiki core itself. Obviously ext-openssl is in suggest in MW core, but that's advisory at best.

Change 872965 abandoned by Dreamy Jazz:

[mediawiki/core@master] Add firebase/php-jwt and ext-openssl to composer.json

Reason:

Not the right method per the associated ticket.

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

Is there anything I can do to move bundling along?

I think new additions / removals in the bundle should get sign-off from the MW releasing process from a Product/etc. POV – that'd be @MSantos. Is this OK?

I think new additions / removals in the bundle should get sign-off from the MW releasing process from a Product/etc. POV – that'd be @MSantos. Is this OK?

LGTM.

OK, this will Just Work™ when the bundle is made using the script.

Remaining work:

  • Add an entry about this to the release notes for MW itself, and
  • Double-check when pruning vendor that nothing new is need (but looking at the composer.json and extension.json this is fine; open-ssl is already required by MW).
Reedy claimed this task.

There's still no mention in the release notes!