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
Drupal 8.4.x is not compatible with PHP 7.2. In order to make Drupal 8 compatible with PHP7.2 we'd need to resolve #2923015: [PHP 7.2] Incompatible method declarations on 8.4.x but this is a breaking change.
Proposed resolution
Update core/composer.json to not allow install on PHP7.2
Remaining tasks
Given this is the first time we've done something like this I think we need release manager feedback.
User interface changes
None
API changes
None
Data model changes
None
Comment | File | Size | Author |
---|---|---|---|
#11 | interdiff.txt | 1.22 KB | pfrenssen |
#11 | 2932574-11.patch | 2.64 KB | pfrenssen |
Comments
Comment #2
alexpottThe one downside of doing this is if someone is patching core to be compatible they'll have to have a patch to reverse this change too. The advantage of course is that someone doesn't waste time learning that 8.4.x is not PHP7.2 compatible.
Comment #3
alexpottHmm... this would only work for composer-only workflows where you try to get the entire drupal/core package. If you have the codebase already and just do a composer install it won't error. This is because of https://github.com/wikimedia/composer-merge-plugin/issues/96 - if you run
composer update --lock
it breaks as expected but it is unlikely people are doing that before running composer.install.Comment #4
alexpottAh of course we need to update the lock file. Then it works... after doing this running
composer install
on PHP7.2 does:I'm still ambivalent about the patch... but at least it works. tbh honest if you look at a Drupal 8.4.x site on PHP 7.2 you can see it is pretty broken.
Comment #5
mondrakeComment #6
catchI'm ambivalent as well. If PHP 7.2 had landed a week after 8.4.0 it'd be very annoying and we'd definitely want something like this. With 8.5.0-alpha1 out mid-January it's tempting to let it slide at this point.
Comment #7
alexpottYep it is tempting to leave this can of worms alone. Just tagging so all release managers can be aware.
Comment #8
pfrenssenI think that the advantage of informing site maintainers about the incompatibility with PHP 7.2 far outweighs having another patch to apply for people that _do_ want to update to PHP 7.2. Depending on the technical skills of the people performing the updates it might not be obvious that PHP 7.2 is causing the fatal errors. At least with this patch they will get a clear message that indicates the source of the problem.
At least if they run the composer update on a machine that also uses PHP 7.2 :)
Comment #9
pfrenssenSince this is tagged with release manager review I don't know if I'm allowed to RTBC this, but this looks ready to go to me.
Comment #10
alexpottIt is actually possible to do a clean install of standard with 8.4.x on PHP 7.2 if you have warnings disabled. The first hard break I encountered was on node/add/article - therefore, if we choose to do this, I think we need to add a system requirement too to prevent install and to put a big red error on admin/reports/status.
Comment #11
pfrenssenGood point, added an error message during install and in the status report.
Comment #12
alexpottLet's not introduce a constant otherwise we have to maintain it for all time. At the moment we do not know the maximum PHP for 8.5.x for example.
Comment #13
mondrakeOTH, having an upper limit for PHP would also have its benefits:
Comment #14
alexpott@mondrake to me all of this feels like a bad idea. If we do what you propose then even to make Drupal 8 work PHP 7.3 we have to patch it. Even PHPUnit with it's aggressive changing of support for PHP versions doesn't do this.
Comment #15
catchAgreed that we shouldn't introduce a constant for this.
Comment #16
xjm8.4.4 was the last regular patch release of Drupal 8.4, so I'm not sure this issue is worth doing at this point. The February window is critical fixes only. I guess we could put this in there but people would only see it for a month until they could update to 8.5.0 anyway.
Comment #17
xjm@catch and I agreed to mark this wontfix for the above reason. Thanks everyone (and sorry for adding to the confusion by advertising our 7.2 support prematurely).