kaliop-uk / ezmigrationbundle

This bundle makes it easy to handle eZPlatform / eZPublish5 content upgrades/migrations
GNU General Public License v2.0
53 stars 81 forks source link

PHP 7.4 compatibility #237

Closed coloso closed 3 years ago

coloso commented 4 years ago

Dear Gaetano,

since PHP 7.4 changes how some array-functions work, see e.g.: https://stackoverflow.com/questions/60339183/arrayobject-does-not-work-with-end-in-php-7-4

We needed to change these three lines as follows in order to make our migrations work again:

1) https://github.com/kaliop-uk/ezmigrationbundle/blob/5.11.0/Core/Matcher/AbstractMatcher.php#L127 -> $resultsArray = $results->getArrayCopy(); return reset($resultsArray);

2) https://github.com/kaliop-uk/ezmigrationbundle/blob/5.11.0/Core/Matcher/LocationMatcher.php#L66 -> $resultsArray = $results->getArrayCopy(); return reset($resultsArray);

3) https://github.com/kaliop-uk/ezmigrationbundle/blob/5.11.0/Core/Matcher/ContentMatcher.php#L65 -> $resultsArray = $results->getArrayCopy(); return reset($resultsArray);

If you wish we can make a patch for that. Downward compatibility has not been tested with older PHP versions below 7.4

gggeek commented 4 years ago

Dang, what a s**y BC break from the php runtime - and apparently it's not even documented!

Feel free to send a PR with a patch - however keep in mind that the signature of that interface says that match can return array|\ArrayObject, so there has to be at least an if added before the getArrayCopy() call

gggeek commented 4 years ago

PS: I will also have to add to the Travis config testing with php 7.4...

gggeek commented 4 years ago

btw, which eZPlatform version are you running on php 7.4 ?

coloso commented 4 years ago

we are using ez ee 2.5.11 and 2.5.12

gggeek commented 4 years ago

FYI: I have pushed to Master a couple of other fixes for php 7.4, and now the tests are green on Travis (see https://travis-ci.org/github/kaliop-uk/ezmigrationbundle/builds/711424648 ). However, I think there are other cases of the same problem still lurking in the code, and I am investigating why/if they are not checked by the test suite. As soon as I am confident that those are fixed, I will release version 5.12

coloso commented 4 years ago

Thank you very much!

coloso commented 4 years ago

Good morning, when is the next release (5.12) planned? Thank you!

gggeek commented 4 years ago

Hello. I'll be back from holodays next week, and should have a fair amount of time free for working on this, so expect a new release by end of week...

JorgenSolli commented 3 years ago

Hi, @gggeek I see there's a fix merged into master. awesome!

Any status on this, though? Currently unable to deploy a production application that recently upgraded to PHP 7.4 Should we consider forking, or is the next minor release a few days away?

gggeek commented 3 years ago

Sorry, It turned out to be busier than expected. If you can afford to do that, I'd suggest that you add an alias to your composer.json file with 'dev-master as 5.12'. (of course this is still unpaid work I'm doing in my free time, so please accept the fact that there might be delays occasionally...)

JorgenSolli commented 3 years ago

Thanks for getting back to me so quickly.

No worries! We'll go ahead and do that. I did not realize this was pro bono work. Thank you for this amazing package :)

gggeek commented 3 years ago

I finally tagged version 5.12 this morning. Will now close this ticket - feel free to open a new one in case you find any other issue with php 7.4...