search

phpcr / phpcr-utils

A set of helper classes and command line commands you want to use with phpcr, but that are not part of the API itself
phpcr.github.io
Other
72 stars 30 forks source link

build with php 7.2 #174

Closed dbu closed 7 years ago

dbu commented 7 years ago

this is quite puzzling. whats more, it looks like the test assumption was expecting a wrong behaviour and php 7.2 magically fixes this. the code in question is https://3v4l.org/IOvBR - and it makes no sense to me why the result is different on 7.2.

fbourigault commented 7 years ago

The 7.2 test use PHPUnit 5.7 which is probably not compatible with PHP 7.2.

fbourigault commented 7 years ago

After searching what could be wrong, I can conclude the that unset has a different behavior. I replaced the unset call with an array_filter and I get the same output in any PHP version. https://3v4l.org/gG7BG

fbourigault commented 7 years ago

I've sent a bug report to PHP: https://bugs.php.net/bug.php?id=75433.

dbu commented 7 years ago

thanks! and we already got an answer on this. so its an optimization about array_values - we seem to have (ab)used the method to reset the counter. a stack overflow search proposes other things like array_merge($array) to get the same effect, but if they can fix array_values, that would be best. lets see. if we are not compatible with some RC versions, i don't care, as long as 7.2.0 is fine.

fbourigault commented 7 years ago

I ran the tests against PHP 7.2RC6 and it's now working. You could trigger a build if 7.2RC6 is available on travis-ci.

dbu commented 7 years ago

https://twitter.com/dbu/status/928645328949202944

dbu commented 7 years ago

thanks a lot @fbourigault for your help in tracking this one down! looks like you even found a relevant regression in PHP 7.2 during the process :-)