Page MenuHomePhabricator

Using extension.json and callback: when `callback` is initialized the queue hasn't finished processing
Closed, ResolvedPublic

Description

I'm experimenting with extension.json [0], the callback is the earliest possible function to set our global SMW_VERSION to an actual version expected to be maintained in extension.json but by the time callback is initialized we are unable to have access to the version because the queue hasn't finished processing.

},
"callback": "SemanticMediaWiki::initExtension",
"ExtensionFunctions": [
	"SemanticMediaWiki::onExtensionFunction"
],
$extensionData = ExtensionRegistry::getInstance()->getAllThings();

if ( isset( $extensionData['SemanticMediaWiki']['version'] ) ) {
	$version = $extensionData['SemanticMediaWiki']['version'];
}

I'd like to avoid any reference to ExtensionRegistry in our repository and instead have access to the configuration in the initExtension callback such as:

public static function initExtension( $configuration ) {

	define( 'SMW_VERSION', $configuration['version'] );

}

[0] https://github.com/SemanticMediaWiki/SemanticMediaWiki/pull/1732

Event Timeline

mwjames created this task.Nov 20 2016, 1:07 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 20 2016, 1:07 AM
mwjames updated the task description. (Show Details)Nov 20 2016, 1:08 AM
Aklapper renamed this task from Using extension.json and callback to Using extension.json and callback: when `callback` is initialized the queue hasn't finished processing.Nov 20 2016, 11:56 AM

Currently, $configuration would contain the information of any extension loaded in this queue, not only the one, where the callback was defined. However, I've another question: Why do you want to define the SMW_VERSION const? What is the goal of it? You could, e.g., set the SMW_VERSION as an attribute in your extension.json (simply in the "config" section). In your code you would use ExtensionRegistry::getInstance()->getAttribute( 'SMW_VERSION' ) to get the value. However, this would mean you'd have a reference to ExtensionRegistry in your code, however, I'm not sure, why you don't want to have it? :)

Currently, $configuration would contain the information of any extension loaded in this queue,

I don't think so because you could do:

 		foreach ( $info['callbacks'] as $cb ) {
-			call_user_func( $cb );
+			call_user_func_array( $cb, array( $info['credits'] ) );
 		}

Why do you want to define the SMW_VERSION const? What is the goal of it?

That shouldn't really matter but for the fact it is a particular constant that have been set since the beginning of the extension with others relying on it to be present when the extension is enabled.

SMW_VERSION as an attribute in your extension.json (simply in the "config" section).

That defeats the purpose of the version field. This or any other configuration may lead to derived settings just making them available by manual maintenance is not the expected objective. (DRY - Don't repeat yourself)

Currently, $configuration would contain the information of any extension loaded in this queue,

I don't think so because you could do:

 		foreach ( $info['callbacks'] as $cb ) {
-			call_user_func( $cb );
+			call_user_func_array( $cb, array( $info['credits'] ) );
 		}

And exactly that would be, what I said: The $info array contains the data of any loaded extension, so credits will contain not only one extension, but the credits of all loaded extensions and skins. However, just passing the credits feels wrong, too, as at some time another one would possibly want other informations, too, and we would need to create breaking changes :)

Why do you want to define the SMW_VERSION const? What is the goal of it?

That shouldn't really matter but for the fact it is a particular constant that have been set since the beginning of the extension with others relying on it to be present when the extension is enabled.

Doing things just because it was done in this way for years mostly isn't a good argument :] What the other extensions try to achieve with this constant? Maybe they can do the same thing with ExtensionRegistration already? Such as checking, if an extension is loaded, and even if a specific version is installed is possible, too. And once https://gerrit.wikimedia.org/r/#/c/250060/ is merged (at some time), it would even be possible to express dependencies directly in extension.json. So, it really depends on what you want to achieve with something, so we (I mean you and anyone who want to be involved, just so we don't get into troubles if I or you miunderstand each other :P) can think about good implementations to achieve what you want :]

SMW_VERSION as an attribute in your extension.json (simply in the "config" section).

That defeats the purpose of the version field. This or any other configuration may lead to derived settings just making them available by manual maintenance is not the expected objective. (DRY - Don't repeat yourself)

That's correct. You can also get the version by using the ExtensionRegistry (getInstance()->getAllThings()['extensionname']).

To be clear: I don't want to say, that your request isn't valid, I just want to understand what you really want to achieve with it and evaluate better ways to do that, if there're better ones :]

Change 324666 had a related patch set uploaded (by Legoktm):
registration: Provide credits information to callbacks

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

Change 325080 had a related patch set uploaded (by Paladox):
registration: Provide credits information to callbacks

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

Change 325081 had a related patch set uploaded (by Paladox):
registration: Provide credits information to callbacks

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

Change 325080 abandoned by Legoktm:
registration: Provide credits information to callbacks

Reason:
Please don't create backports until the patch is merged against master.

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

Change 325081 abandoned by Legoktm:
registration: Provide credits information to callbacks

Reason:
Please don't create backports until the patch is merged against master.

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

Change 324666 merged by jenkins-bot:
registration: Provide credits information to callbacks

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

Change 325081 restored by Legoktm:
registration: Provide credits information to callbacks

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

Change 325081 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/core@REL1_27] registration: Provide credits information to callbacks

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

Change 325080 restored by Legoktm:
registration: Provide credits information to callbacks

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

Change 325080 merged by jenkins-bot:
[mediawiki/core@REL1_28] registration: Provide credits information to callbacks

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

Change 325081 merged by jenkins-bot:
[mediawiki/core@REL1_27] registration: Provide credits information to callbacks

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