Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
#2917655: [9.4.x only] Drop official PHP 7.3 support in Drupal 9.4 discusses the PHP 7 version support policy we will adopt for Drupal 9. However, we have agreed that the minimum PHP version supported by Drupal 9 will be at least PHP 7.2.
Proposed resolution
- Prepare a patch to bump the minimum install and update PHP versions to 7.2 (both Drupal constants and the Composer requirement).
- Set the recommended version to PHP 7.3. (Note: Drupal 9.0 is scheduled to be released 2020 summer, at the time of this issue, 2019 October, however, you should still be very careful and through with testing before putting 7.3 in production.)
The patch can be created and tested against 8.8.x, but will be committed to 9.0.x as soon as it opens.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Drupal 9 now requires at least PHP 7.2.3 to be installed. The suggested PHP version is 7.3 though. This requirement may still be raised higher before the release of Drupal 9.0.0.
Comment | File | Size | Author |
---|---|---|---|
#47 | interdiff.3079791.38-47.txt | 2.01 KB | mikelutz |
#47 | 3079791-47.drupal.patch | 16.35 KB | mikelutz |
Comments
Comment #2
xjmComment #3
Ghost of Drupal Pasthttps://www.drupal.org/docs/9/how-is-drupal-9-made-and-what-is-included/... says
This issue, however, sounds like the decision is made. Perhaps that page should be updated, then?
Comment #4
NaheemSays CreditAttribution: NaheemSays commentedSecurity support for 7.2 ends on 30 November 2020.
If the supported version for Drupal 9.0 is only bumped to php-7.2, will the minimum version need to be bumped to php-7.3 for Drupal 9.1?
Comment #5
Ghost of Drupal PastAs I mentioned in another issue: PHP 7.3 has a behavior change in serialize https://bugs.php.net/bug.php?id=77302 and we have seen this change cause problems with core, contrib and Symfony. I think it's worth repeating this because there is a possibility of Drupal just erroring out when it tries to read previously serialized data. The unserialized data in 7.2 and prior might be bogus but it's possible the bogosity is only visible really on a Zend Engine level and not from userspace or it might be in an unused part of the unserialized data so this often wasn't a concern where the more correct but also more strict new behavior certainly is.
This is not to say I am against PHP 7.3 or that I dislike the change made there -- it has fixed half a dozen bugs outstanding on php.net for a long while so it's obviously a great change. I am merely raising visibility to a potential problem which I have deemed important enough to postpone Smartsheet upgrading to 7.3 for a while and I am commenting here because I am hoping the community will do the hard work of automated and manual testing of PHP 7.3 with large and diverse code- and databases.
Comment #6
catchThe decision here is that as soon as 9.x opens, we'll bump the requirement to PHP 7.2.
It does not prevent us bumping the requirement to PHP 7.3 (or even 7.4) prior to June 2020 in another issue. What it hopefully does do though is allow us to require PHP 7.2 as early as humanly possible without having to make a decision about PHP 7.3 or PHP 7.4 first, before, which is why both issues are open.
Comment #7
shaalUpdated the title of the issue per #6 + #d9readiness slack meeting
Comment #8
xjmI'm changing the title of this issue back. There may be a separate issue later in the Drupal 9 development process to further increase the version requirement, but this issue is for the must-have we will do prior to alpha1. Let's keep the broader topic on #2917655: [9.4.x only] Drop official PHP 7.3 support in Drupal 9.4. Thanks!
Comment #9
xjmComment #10
Ghost of Drupal PastBack to the original topic, I still believe
Should say
Comment #11
xjmWe can and should make a patch for this now as 9.0.x will be branched about one week from now! The patch can set the required version to 7.2 and the recommended version to 7.3.
Comment #12
xjmComment #13
xjmComment #14
xjmI updated https://www.drupal.org/docs/9/how-drupal-9-is-made-and-what-is-included/... along the lines of @Charlie ChX Negyesi's suggestion.
Comment #15
xjmComment #16
shaalI was following #1498574: [policy, no patch] Require PHP 5.4 to make the changes, and added composer.lock updates.
Hopefully some parts of this patch are in the right direction... :)
Comment #17
shaalfixing composer.lock requirements that prevented running composer with php 7.3
Comment #18
MixologicThis seems overly restrictive. Is this because there have been enough recent security releases to warrant this? I thought when we went with a version, we picked whatever distro's lowest normal shipping version was to pin it there.
In any case, the security argument would not be great, because 7.3.0 has several vulnerabilities that 7.2.23 does not, but would be acceptable with this constraint.
Perhaps >=7.2 would be adequate?
Comment #19
shaalI updated it to >=7.2 instead of 7.2.23
Comment #20
Ghost of Drupal PastComment #21
Gábor Hojtsy@Charlie ChX Negyesi: if we expect 7.3 to cause problems we need to solve, is it better to start solving them as soon as the dev branch opens for Drupal 9 or postpone solving those problems to later? To me sounds like it could be better sooner?
Comment #22
hussainwebAlthough I want to be able to use PHP 7.3 in code right now, I am also in favour of just running with PHP 7.2 for now. There are some cool features in 7.3 but not that valuable, IMHO, to worry about too much.
Also, to fix the test, please edit the file at
core/assets/scaffold/files/htaccess
as well.And yes, I think the patch is missing the change to require PHP 7.2 or more.
Comment #23
Wim LeersFixed test failure. But:
Can we really just delete this? Shouldn't we convert this to PHP 7? Then again, it seems we've been doing fine without all this on PHP 7 for years at this point…
#2455465: Add mod_php7 check to htaccess and remove php5 code was trying to add
mod_php7.c
support.This did not delete the comment just above it though:
And if we go look there, we can see that
is definitely fine to remove. The session autostarting disabling we can do without too AFAICT. Then there's 3 multi-byte (unicode) string settings, which are also fine to remove AFAICT. And finally there's the odd PHP 5.6 raw POST data setting, whose need was already disproven a long time ago.
That would explain how we've been doing okay without a
mod_php7.c
section: none of it seems to be necessary anymore :) If so,we can close #2455465: Add mod_php7 check to htaccess and remove php5 code too.
Comment #24
hussainwebI would say it is fine to remove. Apart from all the reasons in #23, there is also the fact that this won't apply for FCGI / FPM setups anyway.
Comment #25
Ghost of Drupal PastSure, we can recommend 7.3 , I like Gábor's proposal... I guess D9 won't be in production anywhere for quite some time so we don't need to issue much warnings about the dangers of that. Perhaps by next summer we can even require it but that's a (very) different bag of hurt.
Comment #26
hussainwebAnother issue I thought about. What about the packages that were held back because of the older version of PHP and now can be updated because of PHP 7.2. For that, I ran composer update in a PHP 7.2.22 container and got this diff for composer files only.
Apart from the updates, most notably, the package
ircmaxell/password-compat
was removed. I don't know why we even have this as this is for forward compatibility for PHP >= 5.3.7. The functions already exist in PHP 5.5.Comment #27
Wim Leers#26: I think we should not do that in this issue, that'll make this issue much harder to review. I agree it's an important follow-up though. I suspect an issue for this already exists?
Comment #28
catchYes this issue should just bump the minimum PHP, upgrading packages should also happen asap, just not here.
Not sure exactly which issue is the correct one, but should be a sub-issue of #3009213: [META] Update / reconsider PHP dependencies for Drupal 9.
Comment #29
hussainwebThat's fair. Then I think we still need to fix the composer.json in the patch (there isn't one) and an updated composer.lock. I could be wrong but I don't see how we got the modified composer.lock as in the patch. Was it manually edited?
Comment #30
hussainwebAfter travelling through the rabbit hole of that META issue, I didn't find an appropriate issue for this specific case. So I just created #3085456: Remove ircmaxell/password-compat. I think this got missed out because we tend to do changes piecemeal one package at a time. There are issues for updates for other packages.
Comment #31
xjmCorrect, this issue is only for the PHP version. Many dependency packaces need special handling and will be reviewed on a case-by-case basis. This just unblocks that whole chain of issues (which already has many filed for specific dependencies, i.e. Symfony 4.4, Twig 2, etc.)
Thanks everyone for working on this so quickly!
Comment #32
xjmComment #34
mikelutzI'm not sure why this patch is changing the vendor php dependencies in the lock file, I don't think we need to touch those. We do need to update the dependency in our composer.json though.
No interdiff due to too many conflicts.
Comment #35
mikelutzComment #36
heddnPlease base on 7.2.3 or above. See https://bugs.php.net/bug.php?id=75938 and #3053798: Checker: PHP version is lower than supported version. We've blacklisted 7.2.0-7.2.2 in automatic_updates for just this reason. I don't think using a slightly higher than 7.2.0 version is a hardship as there still are 20ish minor releases to pick from since 28 Feb 2018.
Comment #37
mikelutzSounds good to me.
Comment #38
mikelutzI forgot about all the component composer.json files..
Comment #39
mglamanWith the bump, shouldn't we remove
"paragonie/random_compat": "^1.0|^2.0|^9.99.99",
from core/composer.json? it's around for 5.x polyfill.Comment #40
Mixologic@mglaman: #3058116: Remove paragonie/random_compat as a top-level dependency from 8.8.x removed it already, but is showing up because its a dep of a couple other deps.
Comment #41
mikelutzI think (per #31) that this issue is specifically about changing the php version. There is a lot of dependency management upcoming around that and the SF4 switch, but as I understand it, this blocks everything else that needs to happen on the 9.0.x branch.
But correct me if I misunderstood.
Comment #42
mglamanI totally forgot Symfony is including it because we're not on v4, ignore me!
Comment #43
mikelutzbut but but... if someone could RTBC THIS issue, then we could be. There's a patch for that.. :-p
Comment #44
catchPatch is straightforward, let's unblock Symfony 4.
Comment #45
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedThere's another issue for removing this PHP 5 stuff: #3075999: Remove php5 from htaccess writer
We should either not do it in this patch since it's not really related to updating the requirement from 7.0.8 to 7.2.3, or close the other issue as a duplicate :)
Comment #46
catchLet's take it out of this patch, #3075999: Remove php5 from htaccess writer could still land in 8.8/8.9.
Comment #47
mikelutzOKay
Comment #48
catchThanks!
Comment #49
alexpottUpdating issue credit for comments that have helped this issue get to rtbc.
Comment #50
alexpottCommitted 68a659f and pushed to 9.0.x. Thanks!
Comment #52
Ghost of Drupal PastI am not going to kvetch over the 7.3 recommendation for 9.0 that makes sense but I do feel the necessity to put a warning in the IS for people who would get carried away by the shiny :)
Comment #53
xjmThis definitely needs a CR and a release note. And to go in the release notes. We'll want to collate a whole specific picture of how 9.0.x is different from 8.9.x in ways that aren't simply straightforward Drupal API deprecations.
Comment #54
Gábor HojtsyAdded this release notes snippet:
I don't think there is more in the patch or in the future plans to say yet :)
Change record still needed.
Comment #55
Gábor HojtsyAdded https://www.drupal.org/node/3089166 as a change record too.