Page MenuHomePhabricator

Replace individual wfLoadExtension calls with a loop operating over an array
Open, Needs TriagePublic

Description

Conditionality of what we load, when, and how should be expressed through the static configuration set-up we're building, so theoretically for extension setup in CommonSettings.php we'll just need:

wfLoadExtensions( $extensionsToLoad );

… but no-doubt there will be issues with this naïve approach.

Related Objects

StatusSubtypeAssignedTask
OpenNone
OpenNone
OpenNone
ResolvedNone
ResolvedVictorbarbu
ResolvedLucas_Werkmeister_WMDE
ResolvedLadsgroup
ResolvedLucas_Werkmeister_WMDE
DeclinedTarrow
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLadsgroup
ResolvedNone
ResolvedLadsgroup
ResolvedLucas_Werkmeister_WMDE
ResolvedTarrow
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLadsgroup
ResolvedLadsgroup
ResolvedLadsgroup
ResolvedLadsgroup
ResolvedLadsgroup
ResolvedLadsgroup
ResolvedLadsgroup
ResolvedLucas_Werkmeister_WMDE
ResolvedLadsgroup
ResolvedItamarWMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLadsgroup
ResolvedItamarWMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLadsgroup
ResolvedLadsgroup
ResolvedLadsgroup
ResolvedLadsgroup
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLadsgroup
ResolvedNone
ResolvedLadsgroup
ResolvedLadsgroup
ResolvedLadsgroup
ResolvedTarrow
ResolvedItamarWMDE
ResolvedLadsgroup
ResolvedLadsgroup
ResolvedItamarWMDE
ResolvedItamarWMDE
ResolvedLadsgroup
ResolvedLadsgroup
ResolvedLadsgroup
ResolvedLucas_Werkmeister_WMDE
ResolvedLadsgroup
ResolvedLadsgroup

Event Timeline

Making this blocked by T75863 for now, but potentially instead we could have a hacky second parameter to wfLoadExtension instead.

Addshore added a subscriber: Addshore.EditedNov 26 2019, 9:38 AM

Also related to this ticket then I guess is T88258.
I'm not sure if either one of these tickets however is the right thing to do here to help solve this ticket.

hashar added a subscriber: hashar.EditedNov 26 2019, 9:43 AM

Note that we have another global functions to load several extensions from an array. So instead of:

foreach ($extensionsToLoad as $index => $extension) {
	wfLoadExtension( $extension );
}

One can use the plural variant of that function:

wfLoadExtensions( $extensionsToLoad );

MediaWiki installer via maintenance/install.php --with-extensions does inject wfLoadExtension() calls, I think in a sorted by name order. We caught several load ordering issues a while back, but most should have been caught by now.

I wonder if some concept of "subextensions" would be worth adding the extension registration.
I would see this working for wikibase as extension.json in the root of the repo then points to 2 more extension.json files in sub directories.
Not sure if there are any other use cases for this though?

I wonder if some concept of "subextensions" would be worth adding the extension registration.
I would see this working for wikibase as extension.json in the root of the repo then points to 2 more extension.json files in sub directories.
Not sure if there are any other use cases for this though?

It's a massive anti-pattern, so I seriously hope there aren't many use cases. ;-) ConfirmEdit has this – we have:

	wfLoadExtension( 'ConfirmEdit' );
	wfLoadExtension( 'ConfirmEdit/FancyCaptcha' );

… but I think that's incompatible with the directory structure you've picked for Wikibase (and won't work on Windows machines, oh well, how sad).

Not sure why that would be incompatible with the wikibase directory structure?
It sound like something like this could work.

	wfLoadExtension( 'Wikibase/repo' );
	wfLoadExtension( 'Wikibase/client' );

Also, not sure why this wouldn't work on windows?

I'm also not saying we don't necessarily want to do T75863, however, I don't think that really blocks this ticket.
However is wfLoadExtension is wanted, then T88258 definity blocks this ticket.

Not sure why that would be incompatible with the wikibase directory structure?

Maybe it would?

It sound like something like this could work.

	wfLoadExtension( 'Wikibase/repo' );
	wfLoadExtension( 'Wikibase/client' );

Also, not sure why this wouldn't work on windows?

The / would need to magically turn into a \, which they won't.

I'm also not saying we don't necessarily want to do T75863, however, I don't think that really blocks this ticket.
However i[f] wfLoadExtension is wanted, then T88258 definity blocks this ticket.

True; I'll add T88258 and not remove T75863 for now, until we get further towards resolution.