Page MenuHomePhabricator
Create Task
Maniphest T331602

Make $wgHooks a regular configuration variable
Closed, ResolvedPublic
Actions

Description

In the past, $wgHooks could be manipulated to register and unregister hook handlers dynamically. This greatly complicate the logic in HookContainer. Code that needs to register hooks dynamically should use HookContainer::register or HookContainer::scopedRegister instead.

Extensions in WMF production that set $wgHooks conditionally:

  • CentralAuth CentralAuth::onReegistration is a registration callback, so it can safely modify $wgHooks.
  • CentralNotice CentralNoticeHooks::onReegistration is a registration callback, so it can safely modify $wgHooks.

Other extensions that may still need fixing, but write accefss to $wgHooks has been emitting deprecation warnings since MW 1.40.

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Hooks: Remove FauxGlobalHooksArraymediawiki/coremaster+0 -281
Make $wgHooks trigger deprecation warnings.mediawiki/coreREL1_40+81 -101
Use HookContainer to register hooksmediawiki/extensions/CentralAuthREL1_40+12 -9
Use HookContainer to register hooksmediawiki/extensions/CentralAuthmaster+12 -9
Make $wgHooks trigger deprecation warnings.mediawiki/coremaster+81 -101
Treat $wgHooks as a regular settingmediawiki/coremaster+348 -185
Customize query in gerrit

Related Objects
Search...

Event Timeline

daniel created this task.Mar 9 2023, 9:03 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 9 2023, 9:03 AM
daniel added a parent task: T329075: Release MW 1.40.0.Mar 9 2023, 9:03 AM
daniel added a project: MW-1.40-release.
daniel removed a parent task: T329075: Release MW 1.40.0.
gerritbot added a comment.Mar 9 2023, 9:06 AM

Change 850669 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/core@master] Make $wgHooks trigger deprecation warnings.

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

gerritbot added a project: Patch-For-Review.Mar 9 2023, 9:06 AM
daniel mentioned this in T331603: Use HookContainer instead of $wgHooks.Mar 9 2023, 9:12 AM
gerritbot added a comment.Mar 9 2023, 9:18 AM

Change 849177 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/core@master] Treat $wgHooks as a regular setting

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

Jdforrester-WMF moved this task from Blocker to To deprecate or remove on the MW-1.40-release board.Mar 9 2023, 5:57 PM
gerritbot added a comment.Mar 10 2023, 2:14 PM

Change 849177 merged by jenkins-bot:

[mediawiki/core@master] Treat $wgHooks as a regular setting

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

ReleaseTaggerBot added a project: MW-1.40-notes (1.40.0-wmf.27; 2023-03-13).Mar 10 2023, 3:00 PM
gerritbot added a comment.Mar 29 2023, 10:20 PM

Change 850669 merged by jenkins-bot:

[mediawiki/core@master] Make $wgHooks trigger deprecation warnings.

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

Maintenance_bot removed a project: Patch-For-Review.Mar 29 2023, 10:29 PM
Krinkle added a project: ci-test-error.Mar 29 2023, 10:51 PM
Krinkle subscribed.

This has caused CI jobs to fail, in for example, https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MathSearch/+/902490/ with:

2023-03-29 22:36:06 a6088546b491 wikidb: [8cf9bb9c28c17f5756087930]
PHP Deprecated: Accessing $wgHooks directly is deprecated, use HookContainer::getHandlers() or HookContainer::register() instead. [Called from MediaWiki\Extension\CentralAuth\CentralAuthHooks::onRunExtensionFunctions]

ReleaseTaggerBot added a project: MW-1.41-notes (1.41.0-wmf.3; 2023-04-03).Mar 30 2023, 1:00 AM
gerritbot added a comment.Mar 30 2023, 8:01 AM

Change 904453 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/extensions/CentralAuth@master] Use HookContainer to register hooks

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

gerritbot added a project: Patch-For-Review.Mar 30 2023, 8:01 AM
daniel added a comment.EditedMar 30 2023, 8:02 AM
In T331602#8740260, @Krinkle wrote:

This has caused CI jobs to fail, in for example, https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MathSearch/+/902490/ with:

2023-03-29 22:36:06 a6088546b491 wikidb: [8cf9bb9c28c17f5756087930]
PHP Deprecated: Accessing $wgHooks directly is deprecated, use HookContainer::getHandlers() or HookContainer::register() instead. [Called from MediaWiki\Extension\CentralAuth\CentralAuthHooks::onRunExtensionFunctions]

Looks like I missed a couple of places. I pushed a patch for CentralAuth. I'll make one for CentralNotice in a bit. Turns out CentralNotice doesn't need fixing.

Access to $wgHooks is still ok in settings files, in Maintenance::finalSetup, and in extension callbacks, because they all execute before MediaWikiServices is initialized. Extension functions are run after, this is why CentralAuth broke.

CodeSearch isn't showing anythign else that needs fixing: https://codesearch.wmcloud.org/deployed/?q=%5E%5Cs*%5C%24wgHooks%5Cb.*%5C%5B%5C%5D&files=&excludeFiles=%5Cbtests%2F%7C%5Cbconfig%5Cb%7CSettings%5C.&repos=

daniel added a comment.Mar 30 2023, 8:26 AM

...and once again, we are bitten by the fact that CI is not running all extensions we have in production...

gerritbot added a comment.Mar 30 2023, 8:48 AM

Change 904291 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/core@REL1_40] Make $wgHooks trigger deprecation warnings.

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

kostajh subscribed.Mar 30 2023, 10:07 AM
In T331602#8740884, @daniel wrote:

...and once again, we are bitten by the fact that CI is not running all extensions we have in production...

Indeed. See T333535: SpecialPageFatalTest::testSpecialPageDoesNotFatal.

kostajh added a comment.Mar 30 2023, 10:08 AM
In T331602#8740856, @daniel wrote:
In T331602#8740260, @Krinkle wrote:

This has caused CI jobs to fail, in for example, https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MathSearch/+/902490/ with:

2023-03-29 22:36:06 a6088546b491 wikidb: [8cf9bb9c28c17f5756087930]
PHP Deprecated: Accessing $wgHooks directly is deprecated, use HookContainer::getHandlers() or HookContainer::register() instead. [Called from MediaWiki\Extension\CentralAuth\CentralAuthHooks::onRunExtensionFunctions]

Looks like I missed a couple of places. I pushed a patch for CentralAuth. I'll make one for CentralNotice in a bit. Turns out CentralNotice doesn't need fixing.

Access to $wgHooks is still ok in settings files, in Maintenance::finalSetup, and in extension callbacks, because they all execute before MediaWikiServices is initialized. Extension functions are run after, this is why CentralAuth broke.

CodeSearch isn't showing anythign else that needs fixing: https://codesearch.wmcloud.org/deployed/?q=%5E%5Cs*%5C%24wgHooks%5Cb.*%5C%5B%5C%5D&files=&excludeFiles=%5Cbtests%2F%7C%5Cbconfig%5Cb%7CSettings%5C.&repos=

AFAICT, we do need the CentralAuth patch to be merged in order to unbreak PageTriage's CI.

gerritbot added a comment.Mar 30 2023, 10:17 AM

Change 904453 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] Use HookContainer to register hooks

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

kostajh mentioned this in T333541: Add CentralAuth to gated extensions.Mar 30 2023, 10:46 AM
hashar mentioned this in T333926: PHP Deprecated: Accessing $wgHooks directly is deprecated, use HookContainer::getHandlers() or HookContainer::register() instead. [Called from {closure}].Apr 4 2023, 9:53 AM
hashar added a subtask: T333926: PHP Deprecated: Accessing $wgHooks directly is deprecated, use HookContainer::getHandlers() or HookContainer::register() instead. [Called from {closure}].
hashar subscribed.Apr 4 2023, 9:56 AM

I had to revert the MediaWiki train (T330209) this morning because MobileFrontend relies on $wgHooks and I have filed that as T333926.

I don't know how CI did not manage to catch the issue when https://gerrit.wikimedia.org/r/c/mediawiki/core/+/850669/ has been proposed given there are at least two jobs testing it with MobileFrontend (PHPUnit extensions testsuite and the Selenium tests).

hashar added a subtask: T334005: PHP Deprecated: Use of Hooks::register was deprecated in MediaWiki 1.35. [Called from LdapPrimaryAuthenticationProvider::__construct].Apr 4 2023, 7:17 PM
hashar closed subtask T334005: PHP Deprecated: Use of Hooks::register was deprecated in MediaWiki 1.35. [Called from LdapPrimaryAuthenticationProvider::__construct] as Resolved.Apr 4 2023, 7:45 PM
hashar closed subtask T333926: PHP Deprecated: Accessing $wgHooks directly is deprecated, use HookContainer::getHandlers() or HookContainer::register() instead. [Called from {closure}] as Resolved.Apr 5 2023, 9:05 AM
gerritbot added a comment.Apr 5 2023, 3:24 PM

Change 905980 had a related patch set uploaded (by Jforrester; author: Daniel Kinzler):

[mediawiki/extensions/CentralAuth@REL1_40] Use HookContainer to register hooks

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

gerritbot added a comment.Apr 6 2023, 7:54 PM

Change 905980 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@REL1_40] Use HookContainer to register hooks

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

gerritbot added a comment.Apr 7 2023, 11:53 AM

Change 904291 merged by jenkins-bot:

[mediawiki/core@REL1_40] Make $wgHooks trigger deprecation warnings.

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

Maintenance_bot removed a project: Patch-For-Review.Apr 7 2023, 12:10 PM
ReleaseTaggerBot edited projects, added MW-1.40-notes; removed MW-1.40-notes (1.40.0-wmf.27; 2023-03-13).Apr 7 2023, 1:00 PM
Krinkle assigned this task to daniel.Aug 28 2023, 3:06 PM
Krinkle edited projects, added API Platform; removed MW-1.41-notes (1.41.0-wmf.3; 2023-04-03).
Krinkle moved this task from Untriaged to Registration on the MediaWiki-Core-Hooks board.

The hooks system doesn't have an owner, but tagging API Platform for visibility of @daniel's work on this particular task.

Umherirrender mentioned this in T347645: Sitenotice hide button is displayed even if there is no sitenotice.Sep 30 2023, 1:49 PM
hashar unsubscribed.Nov 24 2023, 11:51 AM
Jdforrester-WMF removed a project: MW-1.40-release.Jul 2 2024, 5:26 PM
gerritbot added a comment.Feb 18 2025, 10:10 AM

Change #1120497 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/core@master] Hooks: Remove FauxGlobalHooksArray

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

gerritbot added a project: Patch-For-Review.Feb 18 2025, 10:10 AM
Krinkle edited projects, added MW-Interfaces-Team; removed API Platform.Feb 18 2025, 7:04 PM
gerritbot added a comment.Feb 18 2025, 7:28 PM

Change #1120497 merged by jenkins-bot:

[mediawiki/core@master] Hooks: Remove FauxGlobalHooksArray

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

ReleaseTaggerBot added a project: MW-1.44-notes (1.44.0-wmf.18; 2025-02-25).Feb 18 2025, 8:00 PM
aaron subscribed.EditedFeb 20 2025, 4:38 PM

It would be nice to have a checkboxes listing the extensions that need to be changed (those that conditionally set $wgHooks or set it late, e.g. in extension functions, $wgExtensionFunctions).

daniel added a comment.EditedFeb 20 2025, 4:48 PM
In T331602#10568406, @aaron wrote:

It would be nice to have a checkboxes listing the extensions that need to be changed (those that conditionally set $wgHooks or set it late, e.g. in extension functions, $wgExtensionFunctions).

After several years of emitting deprecation warnings, I would have expected all of these extensions to have been fixed. I don't see anything in production that still manipulated $wgHooks outside of the registration callback.

Some third party extensions still do, but after two years of emitting warnings, we can now go ahead and break them.

daniel updated the task description. (Show Details)Feb 20 2025, 4:51 PM
daniel updated the task description. (Show Details)Feb 20 2025, 4:57 PM
Maintenance_bot removed a project: Patch-For-Review.Feb 20 2025, 5:30 PM
daniel closed this task as Resolved.Mar 17 2025, 6:52 AM
Krinkle awarded a token.Mar 21 2025, 10:34 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits