Use of globals should be reduced to make testing easier.
$wgConf is one of this globals, which cannot be replaced by use of ContextSource::getConfig().
Maybe make SiteConfiguration::class a service locator to use
Description
Description
Details
Details
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| DefaultSettings: Soft deprecate config var $wgConf | mediawiki/core | master | +3 -0 |
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Open | None | T11968 Reduce use of global and public variables | |||
| Open | None | T212739 Deprecate $wgConf |
Event Timeline
Comment Actions
Change 497302 had a related patch set uploaded (by D3r1ck01; owner: Derick Alangi):
[mediawiki/core@master] Soft deprecate config var $wgConf and move to service locator
Comment Actions
Change 497302 had a related patch set uploaded (by D3r1ck01; owner: Derick Alangi):
[mediawiki/core@master] DefaultSettings: Soft deprecate config var $wgConf
Comment Actions
Instead of turning this into an object retrieved via MediaWikiServices, I recommend we revisit its actual use. I think moving it into MediaWikiServices creates a new API and is something we should think about for a moment for this object in particular, given it's rare and uncommon usage.
I'm not entirely sure what to do with it. It seems mostly wmf-specific. The few places that call it basically assume 1) multiple wikis exist in a family, and 2) that the wiki farm uses the SiteConfiguration class to configure itself. This is not a safe assumption for MediaWiki in general.
I'm open to what others think we should do with it. Here's two ideas of my own:
- Adopt it as a core service and make it work on plain core (e.g. appearing as a single-wiki farm, with all settings for that wiki coming through the Config instance from getMainConfig).
In addition to the changes required in core, this also formalises quite strongly that other wiki farms have to use SiteConfiguration and/or re-define the SiteConfiguration service as something that reflects their own ways of configuring wikis.
- Deprecate it as a global variable. We keep the status quo. Just as today, wgConf is either useful or not, depending on how the wiki is set up.
The difference being that (after removal), it would no longer be an empty instance that code couldn't statically know when to use. Instead, it would be undefined by default, which is something much easier to detect for extensions opting in to this convention, e.g. through isset().
Instead of a global var, we may want to use MediaWikiServices::hasService() which would be a prettier way to detect this. The only problem with that might be that it's hard to make this work from within LocalSettings.php, which is very early on, and might be too early for MediaWikiServices (to be determined).
Note that the only thing MediaWikiServices would have to support during LocalSettings is defining the service. Checking its existence and retrieving it (afaik) only needs to work after/during extension initialisation.
Comment Actions
Change 497302 abandoned by D3r1ck01:
DefaultSettings: Soft deprecate config var $wgConf
Reason:
will revisit
Comment Actions
As far as I can see the SiteConfiguration class/$wgConf has two distinct purposes. First, it can configure MediaWiki based on the current wiki's database name and "wiki tags" which can be configured (WMF does this based on dblist files in mediawiki-config). The global isn't read, you need to tell it to export configuration as globals for it to work.
The second use is that WikiMap uses the global to create WikiReference objects (by reading $wgServer etc), which essentially tell core and some extensions certain information about remote wikis. For example, it is used to create clickable links when modifying interwiki userrights, power Special:SiteMatrix, db name to link translation on CentralAuth pages, etc.
WikiMap probably needs to stay around in some form, but that should probably be turned into a service instead of having static fields. I'm not exactly sure on what to do with $wgConf - SiteConfiguration functionality is needed, but that should not be tied to a global variable (and it isn't currently. that global just happens to be there and is used for other things).