Page MenuHomePhabricator

Deprecate $wgVersion and move to a constant
Closed, ResolvedPublic

Description

When going forward and replace globals with use of ContextSource::getConfig(), than the global $wgVersion must be deprecated, because it is not a changable config you should set in your LocalSettings.php

Krinkle on https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/481654/:

I don't think this [$wgVersion] is considered a configuration variable. I'm not sure if we have a policy for this sort of thing, but the way I usually try to differentiate between config and non-config globals (such as wgRequest), is to think whether it would be editable from a database-based Config subclass.

With that, I think wgVersion would not be in that category. Rather, I would expect it to become a constant.

https://codesearch.wmflabs.org/search/?q=%5C%24wgVersion%5Cb&i=nope&files=&repos=

Event Timeline

Sensible enough. :) I'd recommend MW_VERSION as the constant name; but should it be set in DefaultSettings.php where $wgVersion is set now, or up in defines.php or elsewhere? May also need to update documentation about cutting release versions (updated location of the changed version).

Would obviously need to keep the variable for compatibility. Since the value is constant and it can be initialized to the named constant value, there should be no danger of them getting out of sync.

MediaWikiVersionFetcher would need to be altered as well, which currently looks for $wgVersion being set in DefaultSettings.php

Using a define sounds good to me, but it seems modern code is not using this style. So having a class constant is also a way to do it. I did not prefer any way here.

At the moment the wgVersion is also used in php entry points to do some version checks. So backward compatibility is needed, even the php entry point is deprecated itself. But it could be used in callback from extension.json as well.

Change 481950 had a related patch set uploaded (by Brion VIBBER; owner: Brion VIBBER):
[mediawiki/core@master] Proposed switch from $wgVersion to MW_VERSION

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

Using a define sounds good to me, but it seems modern code is not using this style. So having a class constant is also a way to do it. I did not prefer any way here.

Hmm, yes class constant will fit our current style better and has the advantage of not worrying about order of operations as long as loaders are set up. I can retool the provisional patch in a bit. :)

+1 to the general concept.

The value of a class constant is that it can be autoloaded. But in the case of MW_VERSION I don't think that's as much of a factor since it will be expected to always be available.

(Also there's a nice similarity with PHP_VERSION and MW_VERSION.)

Change 481950 had a related patch set uploaded (by Jforrester; owner: Brion VIBBER):
[mediawiki/core@master] Provide MW_VERSION and deprecate fake global $wgVersion

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

I'm not sure this is a good idea. It makes it impossible to test any backwards-compatibility code that is based on checking the version number. I'd prefer to see something like WikiVersion "service" that offers methods like getVersionNumber() and checkVersion( $versionSpec ) and such. These can be mocked, which allows

I'm not sure this is a good idea. It makes it impossible to test any backwards-compatibility code that is based on checking the version number.

Right now altering $wgVersion is almost certain to blow up the world. I'm not sure adding this method of code testing is a good plan.

Right now altering $wgVersion is almost certain to blow up the world.

With everything depending on global state, that's for sure. But in a shine future with isolated units of code, that will hopefully no longer be the case.

I'm not sure adding this method of code testing is a good plan.

What other plan do you suggest for testing backwards compatibility code?

I mean - in core, we don't need this. This would be for extensions. The only alternative I can think of is to actually (automatically?) running extension tests on all the relevant releases of MediaWiki. That would be cool, but we don't currently have the CI infrastructure for it.

Extension code that's dependent on specific interfaces should probably feature-detect for those specific interfaces with function_exists, class_exists etc rather than trusting version numbers as well -- and that can only be tested by running with the actual version. And you'd only detect failures (like using a new feature on the old version path) from running with the other version anyway, so I think they'd have to go together if that's something we want tested.

However I'm undecided on the general benefit/pain ratio of changing this var to a constant. I could go either way. :)

Right now altering $wgVersion is almost certain to blow up the world.

With everything depending on global state, that's for sure. But in a shine future with isolated units of code, that will hopefully no longer be the case.

I'm not sure adding this method of code testing is a good plan.

What other plan do you suggest for testing backwards compatibility code?

I mean - in core, we don't need this. This would be for extensions. The only alternative I can think of is to actually (automatically?) running extension tests on all the relevant releases of MediaWiki. That would be cool, but we don't currently have the CI infrastructure for it.

We certainly could do that; right now we strongly (if mostly implicitly) discourage people trying to support multiple versions of MediaWiki from the same branch (instead, we auto-branch every extension with MediaWiki for this reason). I think adding technical debt and the overhead of a service for this theoretical value to extension authors doing something odd is not appropriate. As Brion says, feature-sniffing is what we encourage.

Ok, you convinced me. Testing is not an issue (or rather, having a variable or service doesn't help with the B/C testing issue).

Like Brion, I'm unsure whether transitioning to a constant is worth the pain, though I agree that a constant is The Right Thing here.

Change 481950 abandoned by Brion VIBBER:
Provide MW_VERSION and deprecate fake global $wgVersion

Reason:
I'm abandoning this as I was never quite happy with the patch. Feel free to reopen/recreate it if folks are interested.

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

Change 481950 restored by Brion VIBBER:
Provide MW_VERSION and deprecate fake global $wgVersion

Reason:
Restoring per IRC convo with Timo. Needs rebasing but is cleaner than I remember it being, so if folks are happy with the idea of the change let's update it.

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

Change 573646 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/tools/release@master] Make changes to MW_VERSION, not $wgVersion

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

Change 574621 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] Update all use of $wgVersion to MW_VERSION

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

Change 574628 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@REL1_34] Provide MW_VERSION and soft-deprecate global $wgVersion

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

Change 574629 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@REL1_33] Provide MW_VERSION and soft-deprecate global $wgVersion

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

Change 574632 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@REL1_31] Provide MW_VERSION and soft-deprecate global $wgVersion

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

Change 574632 merged by jenkins-bot:
[mediawiki/core@REL1_31] Provide MW_VERSION and soft-deprecate global $wgVersion

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

Change 574628 merged by jenkins-bot:
[mediawiki/core@REL1_34] Provide MW_VERSION and soft-deprecate global $wgVersion

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

Change 574629 merged by jenkins-bot:
[mediawiki/core@REL1_33] Provide MW_VERSION and soft-deprecate global $wgVersion

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

Change 481950 merged by jenkins-bot:
[mediawiki/core@master] Provide MW_VERSION and deprecate fake global $wgVersion

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

Change 574621 merged by jenkins-bot:
[mediawiki/core@master] Update all use of $wgVersion to MW_VERSION

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

Jdforrester-WMF assigned this task to brion.

Declaring this done. Thanks, all.

Change 573646 merged by jenkins-bot:
[mediawiki/tools/release@master] Make changes to MW_VERSION, not $wgVersion

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

Change 597616 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Pingback: update SCHEMA_REV following deprecation of $wgVersion

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

Change 597616 abandoned by DannyS712:
[mediawiki/core@master] Pingback: update SCHEMA_REV following deprecation of $wgVersion

Reason:

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

Change 713613 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/core@master] MW_VERSION exists since 1.34.1

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

Change 713613 merged by jenkins-bot:

[mediawiki/core@master] MW_VERSION also exists in 1.33.3 and 1.34.1

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