Yii2: Fix PHP 7.4

Created on 30 Nov 2019  路  21Comments  路  Source: yiisoft/yii2

Most helpful comment

Another option is to release Yii 2.1.0 for PHP >= 7.4 and leaving Yii 2.0.x for PHP < 7.4.

All 21 comments

So we have two problem classes:

  1. There's no PHPUnit that is compatible with both PHP 5.4 (which is set as platform target) and PHP 7.4.
  2. PHPUnit 8 requires adjusting tests in the way they don't work on lower versions.
  3. Real errors.

There are two ways fixing it.

  1. Patch old PHPUnit like I've tried in #17706. Messy and complicated.
  2. Remove "platform": {"php": "5.4"} from config, set two possible PHP versions in dependencies, have separate test sets for PHP 7.4 and PHP < 7.4. Leading to maintenance hell.

Another option is to release Yii 2.1.0 for PHP >= 7.4 and leaving Yii 2.0.x for PHP < 7.4.

Bumping required PHP version is not a BC break. If bumping requirements is the only reason for next major release, then it is definitely not worth it - we could require PHP 7 in Yii 2.0.31 and leave 2.0.30 as LTS with security fixes only.

I would vote to have a minimum version of php 7.4, such as Yii 2.1 and take advantage of and add the functionalities that bc would do in lower versions of php, and an LTS version in 2.0 that would not accept corrections but only security maintenance. Yii2 has had many corrections and should continue advancing and debugging the code with the new php functionalities as much as possible, those are my two cents.

Another option is to release Yii 2.1.0 for PHP >= 7.4 and leaving Yii 2.0.x for PHP < 7.4.

Quite agree with this option. Yii 2.0.x has done enough compatibility work, it is time to release 2.1.x for PHP 7.4 and later.

There is another option: drop everything except 7.2, 7.3 and 7.4 from tests but don't drop support for 5.4 overall.

There is another option: drop everything except 7.2, 7.3 and 7.4 from tests but don't drop support for 5.4 overall.

I don't think thsi is a good idea. If so, how do we ensure that test cases always pass?

You mean dropped ones? We can't.

馃槙 So Yii 2 being compliant with 7.4 is tricky.

  • The code of the framework works without many issues. There are three of them to fix. I'm going to merge it separately since these issues are kinda general ones that were not too prominent before.
  • Tests are the problem because https://phpunit.de/supported-versions.html.
  • Previously I've thought that bumping dev-dependencies only and changing platform target would solve the issue but, in fact, that makes it impossible to develop using older versions of PHP because on developer machine framework is installed in development mode. Thus, it could be considered a thing for 2.2.x version (2.1.x could be confusing because it was planned to do current Yii 3 as 2.1 previously).
  • We may use something like Symfony https://symfony.com/doc/current/components/phpunit_bridge.html but that particular one, while works, contains lots of Symfony-specific things so I'd avoid it.
  • PHPUnit 7 works fine with 7.4 well despite what is stated at https://phpunit.de/supported-versions.html. See https://github.com/yiisoft/yii2/pull/17715/files.
  • Supporting a PHP version without running tests for it isn't a good idea. We won't be able to guarantee that it won't break w/o adding too much hassle.

Overall I see it as releasing Yii 2.2 with minimum PHP version bumped to 7.1 and PHPUnit bumped to 7.5. Alternative variant is bumping PHPUnit to 8 and PHP to 7.2.

Wouldn't having our own phpunit-bridge package will be simpler to implement and less confusing for Yii 2 users? There were already plans for such package: https://github.com/yiisoft/yii2-phpunit.

Are there any serious compatibility problems? It looks the most of them could be solved by simple wrappers, which will provide unified interface for specific methods/features.

Wouldn't having our own phpunit-bridge package will be simpler to implement and less confusing for Yii 2 users?

Simpler than what? It is an option but such package has to be developed. I wasn't able to solve issues in about 4 hours. Maybe haven't tried hard enough... https://github.com/yiisoft/yii2/pull/17723 should fix real issues so compatibility problems will be visible in latest build there.

Simpler than what?

Simpler than releasing Yii 2.2 which will be something more than Yii 2.0 with bumped PHP requirements.

More convenient for end users - yes, definitely. Simpler - no. At least based on what I've tried about creating a compatibility layer. But again, maybe I haven't tried hard enough...

According to current php _composer install_ statistics: php5.6 has 6.9% and lower versions have 2.1% in total.
Guess folk survived Yii 2.0.14+ adoption are already on php 7 at least (maybe someone have a statistics?), so bumping to 7.2 or 7.1, sticking with PHPUnit 7 and adding Yii3 compatibility features should be optimal upgrade path.

According to current php _composer install_ statistics: php5.6 has 6.9% and lower versions have 2.1% in total.
Guess folk survived Yii 2.0.14+ adoption are already on php 7 at least (maybe someone have a statistics?), so bumping to 7.2 or 7.1, sticking with PHPUnit 7 and adding Yii3 compatibility features should be optimal upgrade path.

I believe people using <= php 5.6 do not necessarily use composer, so those stats are not really reflecting php5.6 usage.

So here's what we have after cleaning up real errors: https://travis-ci.com/yiisoft/yii2/jobs/265505646

Function ReflectionType::__toString() is deprecated

is caused by PHPUnit_Framework_MockObject_Generator this line:

$typeDeclaration = (string) $parameter->getType() . ' ';

It's not that simple to replace this class before autoloading fires because it's heavily relying on paths relative to the file to load templates.

Maybe we could use https://github.com/cweagans/composer-patches to patch this file.

Thanks for idea, @rob006. It worked. See https://github.com/yiisoft/yii2/pull/17725. There's a weird test fails with PHP 5.4 and Security that I have no idea about yet but PHPUnit part was solved.

Solved security issue. Will be reported to Symfony team later. For now I'd make sure we're not using polyfills.

Was this page helpful?
0 / 5 - 0 ratings