Page MenuHomePhabricator

Drop official PHP 7.2 support in MediaWiki 1.35
Open, Needs TriagePublic

Description

It is generally a good idea to drop support for old things in an LTS, otherwise we are stuck supporting them for 3 years, and are either stuck with them for the next three releases as well, or will have a hard time with backports. Wikimedia production is still using PHP 7.2 so I recommend dropping 7.2 support in the upcoming 1.35 release but keeping it in master until Wikimedia is ready to move. This is similar to what we have done in the past with PHP 5 (T172165).

Impact

  • PHP 7.3 has some nice new features (e.g. more reasonable heredoc/nowdoc syntax, trailing commas in function calls, JSON_THROW_ON_ERROR); we can only use those if we drop 7.2. While dropping or keeping 7.2 support does not make that much difference for new feature development, it does make backports (and maintenance of extensions using the master or LTS compatibility policy) easier.
  • Requiring 7.3 means we can take support for Argon2id (the current best practice in password hashing) for granted.
  • mb_* functions work significantly differently in 7.2 and 7.3, and PCRE expressions have also changed somewhat so supporting them in parallel would probably be annoying and make introducing security issues easier.
  • Supporting 7.2 in the LTS version would mean committing to keep it on our CI boxes for 3 more years. (Per T257879#6348477 that is acceptable.)
  • Parsoid (intended to be an out-of-the-box experience with MW 1.35 and the VisualEditor extension) doesn't perform well on 7.2 without manual tweaking ( T230861)

Policy

Per Support policy for PHP, the requirements for which PHP versions to support in a given release are:

  • A PHP version that will be supported by the upstream PHP Group for the full duration of that MediaWiki major release cycle (from our planned release date until our planned end-of-life date).
    • PHP 7.4 qualifies. (1)
  • A PHP version that is provided by a Debian Linux LTS release channel that will be supported for the duration of that MediaWiki release.
    • Buster qualifies (2), it supports PHP 7.3 (3). (Assuming the odd release date moves back to May-ish, Stretch would qualify but it only supports 7.0 which we already dropped.)
  • A PHP version that is provided by an Ubuntu Linux LTS release channel that will be supported for the duration of that MediaWiki release.
    • Focal / 20.04 and Bionic / 18.04 qualify. (4) 20.04 comes with PHP 7.4, 18.04 with 7.2. (5) (16.04 would qualify too if we consider EOL instead of end of standard support, but it only comes with 7.0 anyway.)
  • For every Debian Linux LTS and Ubuntu Linux LTS release there must be at least one compatible MediaWiki version that is supported at the time the Linux distribution's LTS period starts.
    • LTS releases that start during the lifetime of MediaWiki 1.35: Stretch (2020 July - 2022 June, PHP 7.0), Buster (2022 July - 2024 June, PHP 7.3), Focal (2020 April - 2025 April, PHP 7.4). 7.0 is supported by MediaWiki 1.31 for another year; 7.3 and 7.4 are supported by MediaWiki 1.35. Check.
  • At any given point in time, there must be at least one combination of Debian Linux LTS and MediaWiki that both parties support for an overlapping period of two years. Thus allowing a site operator to remain on a given combination for 2 years (with support), before upgrading to the next supported combination. The same applies to Ubuntu Linux LTS as well.

Thus, all of the policy requirements are met.

Event Timeline

Tgr created this task.Mon, Jul 13, 10:42 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMon, Jul 13, 10:42 PM
Tgr updated the task description. (Show Details)Tue, Jul 14, 12:23 AM
Krinkle added subscribers: tstarling, Krinkle.EditedTue, Jul 14, 1:03 AM

Note that WMF is currently on PHP 7.2.

See also:

That's another thing, the task description lacks any mention of WMF production, which has often been the sole reason for continuing to support a given PHP version.

This point was discussed during […] the Last Call period. Based on the input we got from SRE and CPT, we believe it's desirable and expected for WMF to follow (at worst) the same Debian LTS timeline that we prescribe to third parties. Upgrading once per 2-3 years should be doable.

If and when a different situation emerges this can still be re-evaluated the old-fashioned way with a TechCom request on a case-by-case basis. […]

Looking at the awesome spreadsheet Gergo has just made...

I think we were right to expect WMF to upgrade PHP at least once every 2 years. However, what we failed to account for is what WMF would be running. WMF is obviously not going to run MediaWiki LTS, we're running latest master in production every week.

On the one hand, this gives us 3-6 months extra on either side because we can choose to drop support very early or very late in the internal release cycle (like we did with dropping HHVM).

On the other hand, it means our 2 year window might not start until after we remain several months on the "current" major release while the next major one is being developed. But while it is entirely normal for a third-party to ignore the development branch, we obviously can't.

Tgr added a comment.Tue, Jul 14, 11:21 AM

The minimum version requirement is just a number in a few configuration / text files. IMO it's fine to say we require 7.3 at minimum when MediaWiki would actually work fine on 7.2, if that gets us out of committing to support 7.2 for way longer than we'd like. We did the same with PHP 5 (T172165).

Keeping PHP 7.2 support in MW 1.35 would mean committing to keep 7.2 around on our CI boxes for three more years, and to either continue supporting it for three more years in master, or deal with any complications during backporting. An LTS release is the ideal time to bump requirements as high as we can, IMO. We cannot go higher because Buster is on 7.3 and Bullseye is not out for another year, but we should go up to 7.3. Significantly delaying that switch on master is fine, it won't hurt anyone.

The minimum version requirement is just a number in a few configuration / text files.

CI is also running phan and some other tests on php72 only, so it's a little more complicated than that, but doable, yes.

IMO it's fine to say we require 7.3 at minimum when MediaWiki would actually work fine on 7.2, if that gets us out of committing to support 7.2 for way longer than we'd like. We did the same with PHP 5 (T172165).

The timeline during PHP 5 -> PHP 7/HHVM was a month or two where MediaWiki REL1_31 was "ahead" of master/Wikimedia prod... are we on track for the same with 7.3?

Keeping PHP 7.2 support in MW 1.35 would mean committing to keep 7.2 around on our CI boxes for three more years, and to either continue supporting it for three more years in master, or deal with any complications during backporting. An LTS release is the ideal time to bump requirements as high as we can, IMO. We cannot go higher because Buster is on 7.3 and Bullseye is not out for another year, but we should go up to 7.3.

I agree with you, but I feel this should've been done earlier in the release cycle rather than after branching, and with the Wikimedia upgrade cycle timeline in mind.

Significantly delaying that switch on master is fine, it won't hurt anyone.

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

The minimum version requirement is just a number in a few configuration / text files. IMO it's fine to say we require 7.3 at minimum when MediaWiki would actually work fine on 7.2, if that gets us out of committing to support 7.2 for way longer than we'd like. We did the same with PHP 5 (T172165).

Keeping PHP 7.2 support in MW 1.35 would mean committing to keep 7.2 around […] for three more years […]

I agree that our policy allows discontinueing PHP 7.2 support, and I agree that unofifically supporting it only until WMF upgrades seems fine.

But.. I'm not sure any of the code maintenance benefits apply in that case. By the time WMF upgrades, MW 1.35 will have long release and shipped.

The PHP 5/HHVM situation (T172165) was different because we did actually fully drop support for PHP 5 in that RFC. That we kept was HHVM support, and what we gained was the ability to use PHP 7 syntax ahead of dropping HHVM support, because there was a subset of PHP 7 features available in HHVM by default which we were able to transition to ahead of the release.

Tgr added a comment.Wed, Jul 15, 11:55 AM

But.. I'm not sure any of the code maintenance benefits apply in that case.

The benefit would be being able to backport code that uses 7.3 features (such as trailing commas in function argument lists, which makes code more revision-control-friendly in some circumstances) without having to fix it for compatibility. Also mb_ methods giving different results in 7.2 and 7.3 is something that can easily lead to security issues in third-party code so avoiding that risk is a nice plus.

Also if I understand T230861: PHP 7.2 is very slow on an allocation-intensive benchmark correctly, Parsoid/PHP does not work super well on PHP 7.2 and MW 1.35 will be the first version released with Parsoid/PHP as a sort-of-default experience.

I would appreciate if we (who?) could make a decision on this sooner rather than later just so people can prepare for upgrades properly. I was asking another project I support to upgrade the xenial VM we use for MediaWiki, but I didn't know whether bionic (7.2) would be sufficient or we would need focal (7.4) (in the end they were OK with focal).

Also if I understand T230861: PHP 7.2 is very slow on an allocation-intensive benchmark correctly, Parsoid/PHP does not work super well on PHP 7.2 and MW 1.35 will be the first version released with Parsoid/PHP as a sort-of-default experience.

It's worked around for now with Parsoid disabling the gc on PHP 7.2, but yeah.

Akuckartz added a subscriber: Akuckartz.
Tgr added a comment.Sat, Jul 25, 10:57 AM

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.

Tgr updated the task description. (Show Details)Sat, Jul 25, 11:37 AM
Tgr updated the task description. (Show Details)Sat, Jul 25, 12:05 PM
Izno added a subscriber: Izno.Sat, Jul 25, 12:20 PM

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

Base added a subscriber: Base.Sun, Jul 26, 10:34 AM
Joe added a subscriber: Joe.Tue, Jul 28, 4:42 AM

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.

Joe added a comment.Tue, Jul 28, 4:44 AM

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.

Tgr added a comment.Wed, Jul 29, 1:08 PM

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.

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

Can't it be announced that 1.35 supports 7.4?

Tgr added a comment.Wed, Jul 29, 1:27 PM

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.

hashar added a subscriber: hashar.Wed, Jul 29, 6:12 PM

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.

Tgr added a comment.Wed, Jul 29, 9:16 PM

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.

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?

Tgr updated the task description. (Show Details)Thu, Jul 30, 2:21 PM

Hope that clarify?

It does, thanks!

... 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.

TK-999 added a subscriber: TK-999.Fri, Jul 31, 4:03 PM
Akuckartz added a comment.EditedFri, Jul 31, 8:05 PM

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

Jdforrester-WMF renamed this task from Drop PHP 7.2 support in MediaWiki 1.35 to Drop official PHP 7.2 support in MediaWiki 1.35.Mon, Aug 3, 8:20 AM

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