The policy says "With the above criteria upheld, maintainers of MediaWiki core (as defined by the Privilege policy) are free to add or drop support for any additional versions of PHP without needing to involve TechCom or its RFC process. (...) If maintainers are unable to reach consensus about a proposed change in support, or if exemption from the criteria is desired, the decision may be escalated to TechCom by creating a task for TechCom on Phabricator, or by contacting a TechCom member directly."

I'll write to the mailing list to get more feedback, if that doesn't lead to a clear consensus, we can ask TechCom.

Spoke to @Paladox and there's no issue on Miraheze's end with dropping 7.2. To my knowledge, everything is 7.3 here.

Wikimedia is currently on php 7.2, and I expect the move from 7.2 to 7.3 or 7.4 will only happen after we've migrated to debian buster. That implies we have to do an ICU upgrade - a painful process we'll likely start around the end of September. I would expect the WMF to be able to move to php 7.3 no sooner than the end of the year.

Our original plans were to start the migration sooner, but this is the current situation.

I want to add that, as far as production code goes, we will need to keep support for 7.2 and 7.3 in parallel for some time. We won't be able to migrate overnight (nor it would be recommendable).

I'm certainly ignoring many of the very relevant arguments listed above. However, as far as I'm concerned there are very few reasons to not upgrade from PHP 7.2 to 7.3 – and they are all listed in https://www.php.net/manual/en/migration73.incompatible.php. Is your code affected by these incompatible changes? No? Then upgrade please.

On the other hand, PHP 7.3 has very, very few new features we are going to use the moment we drop PHP 7.2 support entirely. (Yes, I understand this will not happen on master right away, only in the 1.35 branch.) This was different before. Both PHP 7.1 and 7.2 came with very relevant changes we now use all over the place, and even applied to old code. But the basically only new PHP 7.3 feature I want to use are the two new array_key_first() and array_key_last() functions. Sure, there are a few more, some mentioned in the task description above. But I don't consider them relevant. More flexible heredoc syntax? Ok. But we are certainly not running around changing existing heredoc strings to make use of that. Trailing commas in function calls? Yea, nice. But again, we are certainly not running around to add commas to existing code. Personally, I even consider them bad style and will avoid them.

In other words: I believe it doesn't make much of a difference if the 1.35 branch requires backports to be compatible with PHP 7.2 or PHP 7.3. I believe it will be fairly easy to avoid using PHP 7.3 features in backports.

The changes in the mb_… functions and PCRE are, in my opinion, even an argument to stick to PHP 7.2 in the 1.35 branch. See, the 1.35 code was never developed with these changes in mind. I don't think it's a good idea to apply backports that modify small pieces of the 1.35 branch to rely on changed behavior, while the majority of the branch isn't aware of that. This might cause inconsistencies, possibly even unpredictable behavior. Remember, our coverage still isn't great.

As it is now, 1.35 supports both 7.2 and 7.3. So the goal is to get out of the situation where code using MB/PCRE methods need to account for two different behaviors, and might end up working differently on some production machine then on the developer's local environment. And since these methods are used for equality checks / pattern matching, those differences can easily turn out to be security-sensitive.

In T257879#6344561, @Tgr wrote:

As it is now, 1.35 supports both 7.2 and 7.3.

Can't it be announced that 1.35 supports 7.4?

In T257879#6344581, @RazeSoldier wrote:

Can't it be announced that 1.35 supports 7.4?

It actually does but that's irrelevant to this argument.

[…] code using MB/PCRE methods need to account for two different behaviors, and might end up working differently on some production machine […]

I don't think this argument works this way around. What "supporting two versions" means is that we are limited to a subset of the language, and this subset is smaller than what PHP 7.3 offers (obviously), but also smaller than what PHP 7.2 offers (can't use features that work different in 7.3).

Dropping support for a version means we are changing the boundaries of this subset. We will start using features that work different in PHP 7.3, only to patch them into a codebase that was written with PHP 7.2 in mind. In theory, this should not cause any trouble, if done perfectly. But we are not perfect. I would feel much better if we would continue testing the 1.35 branch with PHP 7.2.

Responding for CI / release engineering.

TLDR: support 7.2 and drop support for it later on as it becomes obsolete.

For MediaWiki extensions and skins deployed on the Wikimedia cluster, CI keeps whatever PHP versions are in use on the Wikimedia cluster. That is currently php 7.2 and we will keep that as long as Wikimedia is using it. When a new PHP version is introduced on the cluster we add it to CI in parallel. Once the transition is done we happily drop the old PHP version for the master / wmf branches. But that is out of scope for 1.35.

For release branches, CI runs tests for the minimal PHP version that is claimed to be supported as well as any other php versions supported after that. With time, those versions pill up and eventually it might end up being a bit difficult to forward port an old PHP version to a Debian distribution, it is not impossible but that definitely comes at a maintenance cost and does load the CI infrastructure since tests are run on an extra php version. I am happy we have dropped php 7.0 from CI although it was supported by 1.31.

MediaWiki 1.35 will be supported until August 2023, so whatever minimal PHP version we choose will need to be kept on CI. By the time 1.35 is EOL, php 7.4 will be past its end of life / security fixes only, 7.2 will be an artifact from the past having been released more that 5 years ago. CI uses using images we can keep in place in place, but eventually the Debian distribution they rely on might end up being unavailable on that time frame and make it difficult to rebuild the image if need be. Most probably we would need to forward port php 7.2 to a Debian version that is still around at that time and it is a bit painful.

Given 1.35 is cut from master (which is right now meant primary for php 7.2) I am in favor of stating the minimal supported version is 7.2. CI would keep supporting 7.2 on release branches until it is becomes a challenge. Once 7.2 is end of life and if it becomes too complicated to keep it running on CI, the next patch version of MediaWiki 1.35 can state it now only supports 7.3+ because we can no more ensure 7.2 is supported (use at your own risk). At this point most distributions will have upgraded to 7.3 or later and other would probably want to upgrade as well.

The alternative is to be bold and have our LTS to claim it only support the latest php LTS version available currently (php 7.4) and have CI tests to only run against php 7.4 or later. In all respect, I don't think it is required since it works just fine on 7.2 and bunch of our users are probably still on 7.2.

In T257879#6346169, @hashar wrote:

Once 7.2 is end of life and if it becomes too complicated to keep it running on CI, the next patch version of MediaWiki 1.35 can state it now only supports 7.3+ because we can no more ensure 7.2 is supported (use at your own risk).

Per the policy, Dropping support for a PHP version may only happen in major releases. So it's a now or never situation, as far as MediaWiki 1.35 is concerned.

In T257879#6346810, @Tgr wrote:

Per the policy, Dropping support for a PHP version may only happen in major releases. So it's a now or never situation, as far as MediaWiki 1.35 is concerned.

Ah sorry I got confused due to a task I have filed a a year ago (T216165 July 2019). I was asking to drop php 7.0 support on the claim it was EOL, even on the 1.31 release branch. The outcome of the task is that we have dropped 7.0 from the master branch but kept it for 1.31, the argument being we use a Docker image based on Stretch which is supported past our MediaWiki LTS. Thus we can still rebuild the image easily and still use php 7.0.

CI runs php 7.2 on Stretch which we will phase out in June 2022 a year before MediaWiki 1.35 is EOL. So we would need to port php 7.2 to Buster for CI purposes. At first glance that seems entirely doable, though I can't tell what is the impact of the different ICU version for the mediawiki integration/parser tests that might rely on it. I think we had asked for a rebuild of php7.2 for Buster already, but I can't find the task right now.

If we were to require 7.3, that would ease backports if Wikimedia switches to it, but iit might well end up switching to 7.4 and we would still have some extra care to ensure back compatibility. It is also probably too late to enhance REL1_35 with 7.3 features, which we can't even use yet in master since we need 7.2.

It seems thus the only concern about keeping 7.2 is CI having to support it for the next 3 years even after Debian Stretch is dropped. That is the same case we had for php 7.0 a year ago in T216165 and we have determined it was fine for us. The only culprit is we will have to port php 7.2 to Buster.

Hope that clarify?

In T257879#6348477, @hashar wrote:

Hope that clarify?

It does, thanks!

In T257879#6348477, @hashar wrote:

... I think we had asked for a rebuild of php7.2 for Buster already, but I can't find the task right now. ...

The task to port our component/php72 packages to Buster is T250515 which I guess will be done once SRE schedule the migration to Buster and our CI images will be adjusted from Stretch to Buster :]

Maybe a compromise is possible:

1. No official support for 7.2
2. Continue supporting 7.2 as long as it is needed by the WMF

Maybe that sounds naive, but is that a possible way to resolve this blocking issue?

@Akuckartz As I understand it, yes, that is and has been the proposal from the start of this task.

@Krinkle Thanks, yes, I agree. I was irritated by some of the comments :-)

Change 618336 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] Drop PHP 7.2 testing for REL1_35 branches

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

Can this be decided now?

I have reread the comments. I only found this one which is somehow directed against the proposal:

In T257879#6306721, @Legoktm wrote:

It creates a double standard of "PHP 7.2 for me but not for you".

The MediaWiki-Stakeholders-Group was informed about this issue and I have not seen anyone there complaining about this alleged inequality. And nobody there asked to keep support for 7.2.

Change 619457 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/core@master] Drop official PHP 7.2 support in MediaWiki 1.35

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

T261044: Parsoid in MW 1.35 is not compatible with PHP 7.2.0 (uses array_key_first()) is somewhat amusing...

The code in parsoid isn't actually compatible with PHP 7.2.22... And there's seemingly not enough test coverage to have noticed/exercised the code in CI?

The array_key_first() method is provided in the Composer module 'symfony/polyfill-php73', which Parsoid requires in 'package-dev', so it's probably loaded when running the code in CI. And the module is also installed in mediawiki/vendor, although I am not sure what requires it there, so it also works in Wikimedia production.

In T257879#6410791, @matmarex wrote:

The array_key_first() method is provided in the Composer module 'symfony/polyfill-php73', which Parsoid requires in 'package-dev', so it's probably loaded when running the code in CI. And the module is also installed in mediawiki/vendor, although I am not sure what requires it there, so it also works in Wikimedia production.

Actually, where?

It's not in composer.json - https://github.com/wikimedia/parsoid/blob/v0.12.0-a23/composer.json#L41-L56 or https://github.com/wikimedia/parsoid/blob/v0.13.0-a7/composer.json#L41-L56

Anyone with any final thoughts on this?

It seems there is generally consensus that this is both a good thing, and should be done (for many reasons).

IMHO, the only thing I see I don't see much point immediately dropping PHP 7.2 testing from the REL1_35 branch; as while we're still maintaining and running PHP 7.2 in WMF prod, and therefore against MW master, it's not much extra effort to keep those running.

That being said, if we kept them around for now, I'm certainly not against dropping them at the same time we do for master etc, with the predefined assumption that testing on 7.2 on 1.35 would cease at that point.

Sure, it results in extra CI time expended running patches, but how much backporting is there likely to be?

So maybe just a case of shoring up the wording. Obviously it's "PHP 7.3.0+ is reccommended", but how do we want to phrase about 7.2 being not tested... Is it also going to be classed as unsupported? If we get bugs about MW 1.35 and PHP 7.2 (like T261044: Parsoid in MW 1.35 is not compatible with PHP 7.2.0 (uses array_key_first())), are they therefore viable and would/should be fixed post release? What counts as "official support"?

Should PHPVersionCheck be updated to include this wording? Something like "while 7.2 is supported, 7.3 is reccommended..." etc? I don't think we should necessarily just be relying on people reading docs and release notes; making sure it's in the installer (for new installs) seems prudent for visibility

In T257879#6412227, @Reedy wrote:

Anyone with any final thoughts on this?

It seems there is generally consensus that this is both a good thing, and should be done (for many reasons).

Agreed.

IMHO, the only thing I see I don't see much point immediately dropping PHP 7.2 testing from the REL1_35 branch; as while we're still maintaining and running PHP 7.2 in WMF prod, and therefore against MW master, it's not much extra effort to keep those running.

Eh. OK. I think it's neater if we explicitly only test in 7.3 and above, but if you insist.

So maybe just a case of shoring up the wording. Obviously it's "PHP 7.3.0+ is reccommended", but how do we want to phrase about 7.2 being not tested... Is it also going to be classed as unsupported? If we get bugs about MW 1.35 and PHP 7.2 (like T261044: Parsoid in MW 1.35 is not compatible with PHP 7.2.0 (uses array_key_first())), are they therefore viable and would/should be fixed post release? What counts as "official support"?

My proposed wording is: "1.35 supports PHP 7.3.0 or above (up from 7.2.9). PHP 7.2.22+ will work for the original release, but this is not supported and will be removed without prior notice before the end of life of MediaWiki 1.35 (T257879)."

Should PHPVersionCheck be updated to include this wording? Something like "while 7.2 is supported, 7.3 is reccommended..." etc? I don't think we should necessarily just be relying on people reading docs and release notes; making sure it's in the installer (for new installs) seems prudent for visibility

I don't know if that's needed. When we drop it for real, it'll be in 1.35.3 or whatever, and we can make more of a song and dance about it then.

In T257879#6412227, @Reedy wrote:

Should PHPVersionCheck be updated to include this wording? Something like "while 7.2 is supported, 7.3 is reccommended..." etc? I don't think we should necessarily just be relying on people reading docs and release notes; making sure it's in the installer (for new installs) seems prudent for visibility

Yeah, I'd err on the side of most people not noticing anything that's not being shouted at them by the installer. If someone is stuck with PHP 7.2 and upgrades to 1.35, they won't get security updates, while 1.31 is good for another year. Personally, I would just break 1.35 on 7.2 straight from the start.

So the suggestion is to drop all support (rather than having a grey middle ground) of PHP 7.2 in REL1_35 (obviously master still supports 7.2 for WMF)? Bump versions in composer.json, PHPVersion check etc?

Certainly prevents miscommunication and confusion as to what/when/where 7.2 is supported in REL1_35

I don't have a strong opinion but that seems like the cleanest approach to me, without any real disadvantage.

Unless anyone has any major objections, I propose we do that then. It's clearer and simpler all around.

Let's do it. 7.3.0? Or are there known issues with low versions again?

In T257879#6416941, @Jdforrester-WMF wrote:

Let's do it. 7.3.0? Or are there known issues with low versions again?

I'm sure there'll be some... :D

7.3.0 was released 06 Dec 2018. And within just over 18 months they're upto 7.3.20 (09 Jul 2020)

7.2.0 -> 7.2.9 was T234766: Consider what PHP 7.2 point release to use and was decided based on symfony requirements (at the time, though it looks like they relaxed to 7.2.5 in https://github.com/symfony/symfony/commit/6194c2a96c6c8c35de8929a95a3f498941cd1760)...

7.2.9 -> 7.2.22 was T228346: PHP 7.2 garbage collector segfault

github indicates this patch is present in php-7.4.0alpha3 (and subsequent 7.4 tags) and php-7.3.0RC1 (and subsequent 7.3 tags). But it is not present in PHP 7.2.

and

segfault occurs on php 7.2.19, does not occur in 7.3.4-2.

In T228346#5385018, @cscott wrote:

@Smalyshev No, I did not verify that this patch fixes 7.2. I verified that the bug exists in the most recent release of PHP 7.2, and in releases of PHP 7.3 before php-7.3.0RC1, and releases of PHP 7.4 before php-7.4.0alpha3. That is, this patch fixed the segfault in the 7.3 and 7.4 branches.

I don't know if cscott remembers whether it was fixed in 7.3.0 in RC1 (ie "before"), or whether it wasn't fixed until 7.3.4 (Released: 04 Apr 2019); or do we care enough?

Debian Stable (as of writing) has 7.3.19 (Released: 11 Jun 2020) - https://packages.debian.org/buster/php7.3 and this is our usual rough benchmark

I suspect it's not probably worth digging into too deeply... But 7.3.4 is known good (and over a year old at this point), so we could go with that (newest is currently 7.3.20). We can obviously bump it higher past 7.3.4 at a later date if/when issues come up

Could be worth a quick scan of https://www.php.net/releases/index.php too for the 7.3 releases to see if anything stands out

A quick look at https://www.cvedetails.com/vulnerability-list/vendor_id-74/product_id-128/year-2019/PHP-PHP.html

If we go for 7.3.8, we've included fixes for all released/fixed CVE's for PHP 7.3

Let's do that, then.

I had another look (yesterday was a long day)... https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-11048 "below 7.3.18"

https://nvd.nist.gov/vuln/search/results?form_type=Advanced&cves=on&cpe_version=cpe%3A%2Fa%3Aphp%3Aphp%3A7.3.0%3A-

Maybe we just use Debian stables version as of writing?

7.3.19 then? OK, patch coming up.

Change 622783 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/core@REL1_35] Drop PHP 7.2 support in MediaWiki 1.35; require 7.3.19

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

Change 618336 merged by jenkins-bot:
[integration/config@master] Drop PHP 7.2 testing for REL1_35 branches

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

Mentioned in SAL (#wikimedia-releng) [2020-09-02T12:47:23Z] <James_F> Zuul: Drop PHP 7.2 testing for REL1_35 branches T257879

Change 622783 merged by jenkins-bot:
[mediawiki/core@REL1_35] Drop PHP 7.2 support in MediaWiki 1.35; require 7.3.19

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

Might be too late, but can I highly urge the project to step back the PHP requirement to at least 7.3.5? This is the highest version currently supported officially by RHEL 8, for both RHEL/CentOS. Please don't bastardize our PHP by using unsupported repositories for PHP. Please? 😀

RHEL 8 has PHP 7.3.20 available. To use it:

yum module install php:7.3/*

In T257879#6454753, @Golbez81 wrote:

Might be too late, but can I highly urge the project to step back the PHP requirement to at least 7.3.5? This is the highest version currently supported officially by RHEL 8, for both RHEL/CentOS. Please don't bastardize our PHP by using unsupported repositories for PHP. Please? 😀

{{cn}}

Have you got a reference for this? Trying to browse rpm repos always confuses me quite a bit more than deb based distros....

For example, Debian "stable" (buster) is 7.3.19 (as of writing) - https://packages.debian.org/buster/php7.3

FWIW, I'm not adversed to changing it, but having it properly documented would be helpful!

Reedy, it is especially hard to find the official RH versions without a subscription. I got a 30 day free trial today and found this list of supported versions:

In T257879#6455213, @Reedy wrote:

In T257879#6454753, @Golbez81 wrote:

Might be too late, but can I highly urge the project to step back the PHP requirement to at least 7.3.5? This is the highest version currently supported officially by RHEL 8, for both RHEL/CentOS. Please don't bastardize our PHP by using unsupported repositories for PHP. Please? 😀

{{cn}}

Have you got a reference for this? Trying to browse rpm repos always confuses me quite a bit more than deb based distros....

For example, Debian "stable" (buster) is 7.3.19 (as of writing) - https://packages.debian.org/buster/php7.3

FWIW, I'm not adversed to changing it, but having it properly documented would be helpful!

Debian follows the 7.3.x releases, security updates for Debian's 7.3 update to the then latest 7.3.x release in Buster. I don't know what the policy for RHEL is, but they probably rather backport isolated security fixes instead.

Yay, paywalls! :)

So if 7.3.20 is officially supported by RH... The question lies with @Golbez81 and whether they can upgrade, and if not, why not? :)

I have module php:7.3 installed on CentOS Linux release 8.2.2004 and it's currently on php-common-7.3.5-5.module_el8.2.0+314+53b99e08.x86_64

titan ~ # dnf module list php
Last metadata expiration check: 0:32:57 ago on Fri 11 Sep 2020 03:47:19 PM CDT.
CentOS-8 - AppStream
Name Stream Profiles Summary
php 7.2 [d] common [d], devel, minimal PHP scripting language
php 7.3 [e] common [d] [i], devel, minimal PHP scripting language

Hint: [d]efault, [e]nabled, [x]disabled, [i]nstalled

Ok I guess CentOS 8 is just lagging a little behind as usual. My RHEL8 Azure machine has 7.3.20, so I guess I'll have to wait for CentOS to catch up.

neptune ~ # dnf module install php:7.3
Last metadata expiration check: 0:44:59 ago on Fri 11 Sep 2020 03:48:34 PM CDT.

Dependencies resolved.

Package Arch Version Repository Size

Installing group/module packages:
php-cli x86_64 7.3.20-1.module+el8.2.0+7373+b272fdef rhui-rhel-8-for-x86_64-appstream-rhui-rpms 3.0 M
php-common x86_64 7.3.20-1.module+el8.2.0+7373+b272fdef rhui-rhel-8-for-x86_64-appstream-rhui-rpms 669 k
php-fpm x86_64 7.3.20-1.module+el8.2.0+7373+b272fdef rhui-rhel-8-for-x86_64-appstream-rhui-rpms 1.6 M
php-json x86_64 7.3.20-1.module+el8.2.0+7373+b272fdef rhui-rhel-8-for-x86_64-appstream-rhui-rpms 73 k
php-mbstring x86_64 7.3.20-1.module+el8.2.0+7373+b272fdef rhui-rhel-8-for-x86_64-appstream-rhui-rpms 618 k
php-xml x86_64 7.3.20-1.module+el8.2.0+7373+b272fdef rhui-rhel-8-for-x86_64-appstream-rhui-rpms 187 k

FWIW, the maintainer of the RH package is the same person who maintains the Remi repository -- his name is on the changelog. I think it is reasonable to use the Remi repo for your CentOS instance.

In T257879#6455500, @Golbez81 wrote:

Ok I guess CentOS 8 is just lagging a little behind as usual. My RHEL8 Azure machine has 7.3.20, so I guess I'll have to wait for CentOS to catch up.

So for clarification, you're ok with the version choice?

Yeah, it looks like 7.3.20 is official on RH distro so that will populate down to CentOS in a few weeks. That works! Thanks for hearing me out on it, sorry for the fire drill lol.