Allow setting custom prefixes for config settings in extension.json ("wg" is hardcoded right now)
Closed, ResolvedPublic

Description

Allow customizing the prefix used when configuration settings are expanded into $GLOBALS. Right now "wg" is hardcoded which sucks :(

Paladox created this task.Apr 24 2015, 9:45 PM
Paladox updated the task description. (Show Details)
Paladox raised the priority of this task from to Needs Triage.
Paladox added subscribers: Paladox, Legoktm.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 24 2015, 9:45 PM

Please explain which problem you would like to solve - see https://www.mediawiki.org/wiki/How_to_report_a_bug

Hi this is not a problem this is a suggestion to stop using wg by default and allow prefix or non prefix support.

Yeah. And my request was: Please explain which problem you would like to solve.

The problem is when converting skins and extensions to use the new extension registration and the skins and extensions have for example a mix of $wgname and $name it wont convert the $name but will for $wgname.

Legoktm renamed this task from Support non prefix in extension registration to Allow setting custom prefixes for config settings in extension.json ("wg" is hardcoded right now).Apr 27 2015, 6:44 PM
Legoktm updated the task description. (Show Details)
Legoktm set Security to None.

@Paladox: do you know if this affects any extensions that are shipped in the tarball (T94758)? If so it should be high priority...

@Legoktm nope doint know any extensions that are in the tarball that doint use non prefix settings.

Legoktm triaged this task as Low priority.May 3 2015, 6:07 PM

This should be rated high so that it can be backported to Mediawiki 1.25 before released.

Paladox added a comment.EditedMay 3 2015, 6:13 PM

I found the lines that is hard coded for wg

the code is

https://git.wikimedia.org/blob/mediawiki%2Fcore.git/01799a7cb33c707a675d8222ff9543934791aa13/includes%2Fregistration%2FExtensionProcessor.php#L131

foreach ( $info as $key => $val ) {
     if ( in_array( $key, self::$globalSettings ) ) {
         $this->storeToArray( "wg$key", $val, $this->globals );
     // Ignore anything that starts with a @
     } elseif ( $key[0] !== '@' && !in_array( $key, $this->processed ) ) {
         $this->storeToArray( $key, $val, $this->attributes );
     }
 }

and

https://git.wikimedia.org/blob/mediawiki%2Fcore.git/01799a7cb33c707a675d8222ff9543934791aa13/includes%2Fregistration%2FExtensionProcessor.php#L261

protected function extractConfig( array $info ) {
     if ( isset( $info['config'] ) ) {
         foreach ( $info['config'] as $key => $val ) {
             if ( $key[0] !== '@' ) {
                 $this->globals["wg$key"] = $val;
             }
         }
         $this->processed[] = 'config';
     }
 }

Change 209251 had a related patch set uploaded (by Paladox):
Remove hardcoded wg from config in extension registration

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

Change 209252 had a related patch set uploaded (by Paladox):
Remove hardcoded wg from config in extension registration

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

Change 209255 had a related patch set uploaded (by Paladox):
Add wg prefix in config

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

Change 209252 abandoned by Paladox:
Remove hardcoded wg from config in extension registration

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

Change 209255 abandoned by Legoktm:
Add wg prefix in config

Reason:
As discussed, we're not doing this.

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

How can this be achieved. Why not just upload this change then upload changes for extensions and skins then merge at same time. This way is better. Since this way can only be achieved.

How can this be achieved. Why not just upload this change then upload changes for extensions and skins then merge at same time. This way is better. Since this way can only be achieved.

Because that will result in a breaking change, which we are trying very hard to not cause.

Could if and elseif be used

Paladox added a comment.EditedMay 19 2015, 3:59 PM

But isn't it a breaking change anyways since you can't use a non prefixed setting.

Paladox raised the priority of this task from Low to Normal.Jul 26 2015, 1:07 PM
Paladox added subscribers: MarkAHershberger, Bawolff.

This should be high or at least normal because this blocks other extension/skin that doing use wg prefix in there config.

Change 231244 had a related patch set uploaded (by Paladox):
Support custom prefix

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

I copy the part of my gerrit command here, that explains why I think, that this task should be closed as declined:

I'm totally against this change. The wg prefix is a standard for configuration options in MediaWiki. Not only ExtenRegistration requires it, the Config object does it, too, for a good reason. If any extension or skin has config options without the wg prefix, this should be changed before converting it to ExtensionRegistration.

Ok. Currently The incubation extension doesent follow mediawiki standards it dosent use the wg prefix so is blocked by this task.

Ok. Currently The incubation extension doesent follow mediawiki standards it dosent use the wg prefix so is blocked by this task.

I think it would be much easier to convert the Incubator extension to use the wg prefix for it's config options (in a compatible way, without breaking existing setup), instead of implementing exceptions for any kind of problems we may encounter in converting extensions to ExtensionRegistration (or, in a bigger view, to the Config object, too). That's my opinion, I don't know, how other think about that :)

Ok I currently have a patch waiting that does this I also updated the Wikimedia file. Once someone agrees that it can be merged the documentation on wiki can be updated.

Kghbln added a subscriber: Kghbln.Aug 14 2015, 3:57 PM

Please also remember that this is a BREAKING change for a minimum of 10 to 15 % of all third party wikis. I understand that this issue was or will be publicised and documented prominently if this has not been done already.

@Kghbln: Do you mean Paladaox change for WikimediaIncubator or fo you mean change https://gerrit.wikimedia.org/r/#/c/231244/ ? :)

Change 231244 abandoned by Paladox:
Support custom prefix

Reason:
I am not quite sure how to do this so abandoning.

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

Change 209251 abandoned by Paladox:
Remove hardcoded wg from config in extension registration

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

Kghbln added a comment.EditedAug 14 2015, 4:29 PM

I mean the hard requirement of $wg

Please also remember that this is a BREAKING change for a minimum of 10 to 15 % of all third party wikis. I understand that this issue was or will be publicised and documented prominently if this has not been done already.

I might be missing something, but I don't see how this is a breaking change for wikis. Extensions that don't use a "wg" prefix should not migrate to extension registration until this bug is fixed.

Extensions that don't use a "wg" prefix should not migrate to extension registration until this bug is fixed.

At the time I wrote this I was under the clear impression that it was decided not to fix this. In this case it is indeed breaking.

Change 233331 had a related patch set uploaded (by Legoktm):
registration: Allow custom prefixes for configuration settings

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

Legoktm claimed this task.Aug 23 2015, 10:58 PM

Any thoughts on @Legoktm's patch? Is there a reason not to accept it?

Change 233331 merged by jenkins-bot:
registration: Allow custom prefixes for configuration settings

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

Legoktm closed this task as Resolved.Sep 3 2015, 9:29 PM

@Legoktm how would I use the new custom prefix for wikimediaincubator is there any example for wikimediaincubator so that it can start converting to extension.json.

Use something like:

{
    "config": {
        "_prefix": "wminc",
        "Pref": "incubatortestwiki"
    }
}

Change 237975 had a related patch set uploaded (by Legoktm):
registration: Allow custom prefixes for configuration settings

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

Change 237975 merged by jenkins-bot:
registration: Allow custom prefixes for configuration settings

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

There should be array support and maybe something like

"_prefix-1": "wminc", This should be either { or [
"ConfigGoes here": This can be either { or [
And then code goes here

Either } or ] here and same again.

And then "_prefix-2": "example", for second config if it is different Since some configs have a wg prefix which would affect all configs. Making it hard to use non prefixed and prefixed ones.