(Go: >> BACK << -|- >> HOME <<)
Page MenuHomePhabricator
Create Task
Maniphest T331602

Make $wgHooks a regular configuration variable
Open, Needs TriagePublic
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.

Details

SubjectRepoBranchLines +/-
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
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