Deprecate and remove extension database updating globals
Closed, ResolvedPublic

Description

This is about the globals $wgExtNewFields, $wgExtNewTables, $wgExtNewIndexes, $wgExtPGAlteredFields, $wgExtPGNewFields and $wgExtModifiedFields (no wiki page). Its replacement is the LoadExtensionSchemaUpdates hook. DatabaseUpdater::getOldGlobalUpdates mentions:

Before 1.17, we used to handle updates via stuff like $wgExtNewTables/Fields/Indexes. We refactored a lot of this in 1.17 but we want to remain back-compatible for a while.

Usages

Code search: https://codesearch.wmflabs.org/search/?q=ExtNewFields%7CExtNewTables%7CExtPGNewFields%7CExtNewIndexes%7CExtPGAlteredFields%7CExtModifiedFields&i=nope&files=&repos=

MediaWiki

  • DatabaseUpdater::initOldGlobals
  • DatabaseUpdater::getOldGlobalUpdates
  • PostgresUpdater::getOldGlobalUpdates

Extensions

All of these are already migrated and use the globals as back-compat only.

Event Timeline

Change 554602 had a related patch set uploaded (by Mainframe98; owner: Mainframe98):
[mediawiki/core@master] Remove old extension schema update globals

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

Why no deprecation? Doesnt seem like there is an urgent need to remove or that its blocking anything. Even if code search says no results this was still a very directly public api, there could be extensions elsewhere using it that would benefit from deprecation warnings.

Well, my reasoning was that with T223939: Type-hint all onLoadExtensionSchemaUpdate hook handler $updater parameter with DatabaseUpdater type (which would result in prohibiting passing null; therefore making the backwards compatibility fallback an un-executable branch of code), any of the known extensions use it as backwards compatibility only, its effective obsolescence in 1.17 and the fact that some globals lack documentation, we're effectively dealing with dead code here. I think we'd be hardpressed to find anything relying on these globals that is still compatible with MediaWiki 1.35.

Change 555665 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/IndexFunction@master] Remove use of $wgExtNewTables in updater

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

Change 555666 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/OnlineStatusBar@master] Remove use of $wgExtNewTables in updater

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

Change 555667 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/LiveTranslate@master] Remove use of $wgExtNewTables and friends in updater

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

Change 555668 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/Poll@master] Remove use of $wgExtNewTables and friends in updater

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

Change 555669 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/WikiLove@master] Remove use of $wgExtNewTables in updater

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

Change 555670 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/ApprovedRevs@master] Remove use of $wgExtNewTables in updater

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

Change 555671 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/DeviceMapLogCapture@master] Remove use of $wgExtNewTables in updater

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

Change 555669 merged by jenkins-bot:
[mediawiki/extensions/WikiLove@master] Remove use of $wgExtNewTables in updater

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

Change 555666 merged by jenkins-bot:
[mediawiki/extensions/OnlineStatusBar@master] Remove use of $wgExtNewTables in updater

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

Change 555670 merged by jenkins-bot:
[mediawiki/extensions/ApprovedRevs@master] Remove use of $wgExtNewTables in updater

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

Change 555668 merged by jenkins-bot:
[mediawiki/extensions/Poll@master] Remove use of $wgExtNewTables and friends in updater

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

Well, my reasoning was that with T223939: Type-hint all onLoadExtensionSchemaUpdate hook handler $updater parameter with DatabaseUpdater type (which would result in prohibiting passing null; therefore making the backwards compatibility fallback an un-executable branch of code), any of the known extensions use it as backwards compatibility only, its effective obsolescence in 1.17 and the fact that some globals lack documentation, we're effectively dealing with dead code here. I think we'd be hardpressed to find anything relying on these globals that is still compatible with MediaWiki 1.35.

I mean, that's basically saying that all extensions that have been updated, have been updated, which is certainly true, but there do exist extensions outside of gerrit.

Personally I am of the opinion that the deprecation stage should only be skipped if not doing so is blocking moving something else forward, which doesn't seem to be the case here. Its not like this is an issue I feel super strongly about or anything, so just my 2 cents, but personally I'd prefer there was at least a version of MediaWiki that used the DeprecatedGlobal class to give a warning for these globals, unless there was a reason why doing so would be overly annoying or problematic.

I've tried using DeprecatedGlobal, but that class is not suitable for globals using non-objects; for arrays, it'll result in a PHP warning: Cannot use object of type DeprecatedGlobal as array. I'm not sure what else could serve a clear deprecation warning. You could add a check to DatabaseUpdater::getOldGlobalUpdates that checks if any of the globals is something else than [], but that doesn't tell you what extension caused that warning.

I've tried using DeprecatedGlobal, but that class is not suitable for globals using non-objects; for arrays, it'll result in a PHP warning: Cannot use object of type DeprecatedGlobal as array. I'm not sure what else could serve a clear deprecation warning. You could add a check to DatabaseUpdater::getOldGlobalUpdates that checks if any of the globals is something else than [], but that doesn't tell you what extension caused that warning.

Ah good point, that sounds like a good reason to not use deprecation warnings. I withdraw my criticism :)

In theory its possible to make DeprecatedGlobal extend ArrayObject ( https://www.php.net/manual/en/class.arrayobject.php ) but that definitely does not seem worth it in this case.

Change 555667 merged by jenkins-bot:
[mediawiki/extensions/LiveTranslate@master] Remove use of $wgExtNewTables and friends in updater

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

Change 555671 merged by jenkins-bot:
[mediawiki/extensions/DeviceMapLogCapture@master] Remove use of $wgExtNewTables in updater

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

Change 555665 merged by Umherirrender:
[mediawiki/extensions/IndexFunction@master] Remove use of $wgExtNewTables in updater

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

I guess we can start the deprecation phase now? The quickest way seems to add a check to getOldGlobalUpdates and emit a wfDeprecated if any variable isn't empty.

While that does work, it doesn't tell us which extension is still using the globals; which makes finding the offending extension difficult.

While that does work, it doesn't tell us which extension is still using the globals; which makes finding the offending extension difficult.

Right, but we can add the table/columns/etc names to the message, which would be a clear hint.

While that does work, it doesn't tell us which extension is still using the globals; which makes finding the offending extension difficult.

Right, but we can add the table/columns/etc names to the message, which would be a clear hint.

The code search indicates no use of those globals anymore, and the Stable interface policy has changed since then too. Is any deprecation still necessary? At this point, any extension still using them has larger problems than using these globals.

Change 554602 merged by jenkins-bot:
[mediawiki/core@master] Remove old extension schema update globals

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

Mainframe98 moved this task from Unsorted to Needs removal on the Technical-Debt board.