Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 7.4 support #4797

Closed
summercms opened this issue Nov 30, 2019 · 27 comments
Closed

PHP 7.4 support #4797

summercms opened this issue Nov 30, 2019 · 27 comments

Comments

@summercms
Copy link
Contributor

summercms commented Nov 30, 2019

Today, been running a few tests with php 7.4 and October cms, it seems like many plugins are not working, but digging deeper it seems to be October CMS side and not the plugins.

Please can people investigate further.

Some examples:

Example 1

Clear file cache plugin (13,056 installations) see github issue: alserom/octobercms_clearcachewidget#18

The Stack Trace is saying the code line: https://github.com/alserom/octobercms_clearcachewidget/blob/7c7642de198c50d7bd61a285c59658c5cb4d2e49/reportwidgets/ClearCache.php#L66

That is calling an artisan command, so it looks like the issue is from October's side and not the plugin.

Example 2

Google Analytics plugin (11,934 installations) see github issue: rainlab/googleanalytics-plugin#92

The issue says:

Deprecated: implode(): Passing glue string after array is deprecated

Having searched that plugin I can't find implode() being used by the plugin, but October is using implode().

Example 3

Google Analytics Extension plugin (2,135 installations) see github issue: scottbedard/analyticsextension#9

The issue says:

Deprecated: implode(): Passing glue string after array is deprecated

Having searched that plugin I can't find implode() being used by the plugin, but October is using implode().

Example 4

This time, not a plugin example. Instead Twig is saying issues in the Stack Trace:

ErrorException: array_key_exists(): Using array_key_exists() on objects is deprecated. Use isset() or property_exists() instead in /home/example/public_html/vendor/twig/twig/src/Extension/CoreExtension.php:1396

See github issue: twigphp/Twig#3151

Example 5

PHP Deprecated Examples in October and plugins:

PHP Deprecated:  Unparenthesized `a ? b : c ?: d` is deprecated. Use either `(a ? b : c) ?: d` or `a ? b : (c ?: d)` in /home/example/public_html/modules/system/classes/CombineAssets.php on line 423

PHP Deprecated:  Unparenthesized `a ?: b ? c : d` is deprecated. Use either `(a ?: b) ? c : d` or `a ?: (b ? c : d)` in /home/example/public_html/plugins/rainlab/translate/behaviors/TranslatablePage.php on line 76

Conclusion

I'm seeing so many different errors from every direction when trying to use php 7.4 and October cms.

I would not recommend anyone try to use php 7.4 and October cms just yet.

Could the admins create an action plan or something to fully test everything with php 7.4

Thanks.

@summercms
Copy link
Contributor Author

  • Digging deeper it appears there are loads of changes (nothing like 7.2 to 7.3) see below:

Changes and deprecations

Besides new features, there are also lots of changes to the language. Most of these changes are non-breaking, though some might have an effect on your code bases.

Note that deprecation warnings aren't per definition "breaking", but merely a notice to the developer that functionality will be removed or changed in the future. It would be good not to ignore deprecation warnings, and to fix them right away; as it will make the upgrade path for PHP 8.0 more easy.

Left-associative ternary operator deprecation rfc

The ternary operator has some weird quirks in PHP. This RFC adds a deprecation warning for nested ternary statements. In PHP 8, this deprecation will be converted to a compile time error.

1 ? 2 : 3 ? 4 : 5;   // deprecated
(1 ? 2 : 3) ? 4 : 5; // ok

Exceptions allowed in __toString rfc

Previously, exceptions could not be thrown in __toString. They were prohibited because of a workaround for some old core error handling mechanisms, but Nikita pointed out that this "solution" didn't actually solve the problem it tried to address.

This behaviour is now changed, and exceptions can be thrown from __toString.

Concatenation precedence rfc

If you'd write something like this:

echo "sum: " . $a + $b;

PHP would previously interpret it like this:

echo ("sum: " . $a) + $b;

PHP 8 will make it so that it's interpreted like this:

echo "sum :" . ($a + $b);

PHP 7.4 adds a deprecation warning when encountering an unparenthesized expression containing a . before a + or - sign.

array_merge without arguments upgrading

Since the addition of the spread operator, there might be cases where you'd want to use array_merge like so:

$merged = array_merge(...$arrayOfArrays);

To support the edge case where $arrayOfArrays would be empty, both array_merge and array_merge_recursive now allow an empty parameter list. An empty array will be returned if no input was passed.

Curly brackets for array and string access rfc

It was possible to access arrays and string offsets using curly brackets:

$array{1};
$string{3};

This has been deprecated.

Invalid array access notices rfc

If you were to use the array access syntax on, say, an integer; PHP would previously return null. As of PHP 7.4, a notice will be emitted.

$i = 1;

$i[0]; // Notice

proc_open improvements upgrading

Changes were made to proc_open so that it can execute programs without going through a shell. This is done by passing an array instead of a string for the command.

strip_tags also accepts arrays upgrading

You used to only be able to strip multiple tags like so:

strip_tags($string, '<a><p>')

PHP 7.4 also allows the use of an array:

strip_tags($string, ['a', 'p'])

ext-hash always enabled rfc

This extension is now permanently available in all PHP installations.

Improvements to password_hash rfc

This is a small change and adds argon2i and argon2id hashing support when PHP was compiled without libargon.

PEAR not enabled by default externals

Because PEAR isn't actively maintained anymore, the core team decided to remove its default installation with PHP 7.4.

Several small deprecations rfc

This RFC bundles lots of small deprecations, each with their own vote. Be sure to read a more detailed explanation on the RFC page, though here's a list of deprecated things:

  • The real type
  • Magic quotes legacy
  • array_key_exists() with objects
  • FILTER_SANITIZE_MAGIC_QUOTES filter
  • Reflection export() methods
  • mb_strrpos() with encoding as 3rd argument
  • implode() parameter order mix
  • Unbinding $this from non-static closures
  • hebrevc() function
  • convert_cyr_string() function
  • money_format() function
  • ezmlm_hash() function
  • restore_include_path() function
  • allow_url_include ini directive

Other changes upgrading

You should always take a look at the full UPGRADING document when upgrading PHP versions.

Here are some changes highlighted:

  • Calling parent:: in a class without a parent is deprecated.
  • Calling var_dump on a DateTime or DateTimeImmutable instance will no longer leave behind accessible properties on the object.
  • openssl_random_pseudo_bytes will throw an exception in error situations.
  • Attempting to serialise a PDO or PDOStatement instance will generate an Exception instead of a PDOException.
  • Calling get_object_vars() on an ArrayObject instance will return the properties of the ArrayObject itself, and not the values of the wrapped array or object. Note that (array) casts are not affected.
  • ext/wwdx has been deprecated.

Link: https://github.com/php/php-src/blob/PHP-7.4/UPGRADING

@bennothommo
Copy link
Contributor

I've suggested here that people should hold off on updating their October sites to PHP 7.4 for the moment. That being said, the errors are pretty much entirely deprecations, so if you want to set PHP to ignore deprecations, it's quite possible that everything will still work as is.

This will also likely be resolved with the Laravel 6 upgrade too.

@LukeTowers
Copy link
Contributor

@ayumi-cloud also feel free to submit PRs to fix the errors as you find them, if the fix doesn't break support for 7.0 to 7.3 it'll be merged (until l6 is done, then it just needs to support 7.2)

@summercms
Copy link
Contributor Author

summercms commented Dec 1, 2019

@bennothommo @LukeTowers I think it's misleading calling it php7.4 it's more like php8.0-rc.0

I had over 130 different errors in the event log (it's not a simple fix) for october.

I'm hoping that you guys will update October to Laravel 6 first as they have done some work to update to php7.4 see there pull request here: laravel/framework#29482

When that is done, I can start going through all the vendor files and checking for php7.4 issues.

Then after that continue checking plugins etc.

Either way, I think this is going to become a breaking change and also no support for php 7.0 - 7.3 with all these changes and more to come with php 7.4.x minor updates.

@filipac
Copy link
Contributor

filipac commented Dec 2, 2019

Laravel 6 upgrade is taking forever, do you have an estimate? It's always recommended to be on the latest php version and this breaks october sites.

@bennothommo
Copy link
Contributor

bennothommo commented Dec 2, 2019

@filipac No firm estimate yet - it was only started within the last couple of weeks. Your patience will be much appreciated. As stated above, you can likely use October just fine with PHP 7.4 if you set up your PHP instance to ignore deprecations.

@summercms
Copy link
Contributor Author

summercms commented Dec 2, 2019

Automated code analysis tools

Automated code compatibility testing helps to detect possible problems in your code.

(Using PHPCompatibility requires you to first install PHP CodeSniffer globally and then configure PHPCompatibility as the code standard to use).

(Would be great if someone created a plugin for October)

@LukeTowers
Copy link
Contributor

@filipac consider contributing to the bounty. I said I'd be able to prioritize working on it over client work when it gets to 2,400 USD.

@daftspunk
Copy link
Member

Update: 7.4 should be supported as of Build 461 (core October files)

Some plugins still may require attention

@LukeTowers LukeTowers changed the title October seems to have multiple errors with php 7.4 PHP 7.4 support Dec 15, 2019
@summercms
Copy link
Contributor Author

Cool, going to re-test October v461 with php7.4 any issues will post in this thread.

(I won't mention any plugin issues here - will only focus on core related issues here).

@BlackScorp
Copy link

BlackScorp commented Dec 30, 2019

Forget my last report, the installer throws the exception, i already created a pull request

@p5ych0
Copy link

p5ych0 commented Dec 31, 2019

combining assets doesn't work on build 462 and php 7.4.1
always returns Trying to access array offset on value of type null

@bennothommo
Copy link
Contributor

@p5ych0 Please create a new GitHub issue for that and provide a stacktrace.

@p5ych0
Copy link

p5ych0 commented Jan 3, 2020

@p5ych0 Please create a new GitHub issue for that and provide a stacktrace.

@bennothommo I'm unable to find the place where the issue is introduced. Somewhere in the kriswallsmith/assetic. Also there's no stack trace due to error handler which I wasn't able to find as well

@bennothommo
Copy link
Contributor

@p5ych0 Is there a screenshot of the issue, just to give us an idea of what is occurring?

@LukeTowers
Copy link
Contributor

@p5ych0 if it's an issue with assetic we can fix that since we're taking over that library with the Laravel 6 upgrade. @jaxwilko are you able to test the current 2.0 against PHP 7.4?

@jaxwilko
Copy link

jaxwilko commented Jan 3, 2020

image

@LukeTowers I'll put a patch in for this and push it on it's own branch

@LukeTowers
Copy link
Contributor

@jaxwilko you can push it to the main dev branch, I'm not going to have time to test it myself and I trust you'll get it right 😄

@daftspunk
Copy link
Member

daftspunk commented Jan 5, 2020

The fix for the combiner issue is here: octobercms/library@b056665

Seems unrelated to Assetic. Fortunately, wikimedia has picked up the package... but we can't quite use it yet. Notes are in the commit

Someone should test the SCSS combiner, if it fails, please raise a new issue

@LukeTowers
Copy link
Contributor

@jaxwilko could you check into using the package @daftspunk mentions in his commit in Assetic 2.0? We'll have to bump the PHP requirement but I'm fine with that

@daftspunk
Copy link
Member

Confirmed the SCSS compiler still seems to be working with a basic test

$blue: red;
a { color: $blue; }

Returns

a {
  color: red; }

@daftspunk daftspunk modified the milestones: v1.0.464, v1.0.463 Jan 6, 2020
@caloggs
Copy link
Contributor

caloggs commented Jan 19, 2020

Im still getting a error with PHP 7.4 when running the command php artisan october:util compile assets

Heres the stack trace:

[2020-01-19 16:06:14] local.ERROR: ErrorException: Trying to access array offset on value of type null in /Sites/october/test-site/vendor/october/rain/src/Parse/Assetic/Less/lib/Less/Tree/Import.php:217
Stack trace:
#0 /Sites/october/test-site/vendor/october/rain/src/Parse/Assetic/Less/lib/Less/Tree/Import.php(217): Illuminate\Foundation\Bootstrap\HandleExceptions->handleError(8, 'Trying to acces...', '/Users/calum/Si...', 217, Array)
#1 /Sites/october/test-site/vendor/october/rain/src/Parse/Assetic/Less/lib/Less/Tree/Import.php(156): Less_Tree_Import->PathAndUri()
#2 /Sites/october/test-site/vendor/october/rain/src/Parse/Assetic/Less/lib/Less/Tree/Ruleset.php(94): Less_Tree_Import->compile(Object(Less_Environment))
#3 /Sites/october/test-site/vendor/october/rain/src/Parse/Assetic/Less/lib/Less/Parser.php(199): Less_Tree_Ruleset->compile(Object(Less_Environment))
#4 /Sites/october/test-site/vendor/october/rain/src/Parse/Assetic/LessCompiler.php(42): Less_Parser->getCss()
#5 /Sites/october/test-site/vendor/kriswallsmith/assetic/src/Assetic/Filter/FilterCollection.php(62): October\Rain\Parse\Assetic\LessCompiler->filterLoad(Object(Assetic\Asset\FileAsset))
#6 /Sites/october/test-site/vendor/kriswallsmith/assetic/src/Assetic/Asset/BaseAsset.php(94): Assetic\Filter\FilterCollection->filterLoad(Object(Assetic\Asset\FileAsset))
#7 /Sites/october/test-site/vendor/kriswallsmith/assetic/src/Assetic/Asset/FileAsset.php(65): Assetic\Asset\BaseAsset->doLoad('// Vendor\n@impo...', NULL)
#8 /Sites/october/test-site/vendor/kriswallsmith/assetic/src/Assetic/Asset/BaseAsset.php(103): Assetic\Asset\FileAsset->load()
#9 /Sites/october/test-site/vendor/kriswallsmith/assetic/src/Assetic/Asset/AssetCollection.php(151): Assetic\Asset\BaseAsset->dump(NULL)
#10 /Sites/october/test-site/modules/system/Classes/CombineAssets.php(229): Assetic\Asset\AssetCollection->dump()
#11 /Sites/october/test-site/modules/system/Console/OctoberUtil.php(177): System\Classes\CombineAssets->combineToFile(Array, '/Users/calum/Si...')
#12 /Sites/october/test-site/modules/system/Console/OctoberUtil.php(81): System\Console\OctoberUtil->utilCompileAssets()
#13 [internal function]: System\Console\OctoberUtil->handle()
#14 /Sites/october/test-site/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(29): call_user_func_array(Array, Array)
#15 /Sites/october/test-site/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(87): Illuminate\Container\BoundMethod::Illuminate\Container\{closure}()
#16 /Sites/october/test-site/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(31): Illuminate\Container\BoundMethod::callBoundMethod(Object(October\Rain\Foundation\Application), Array, Object(Closure))
#17 /Sites/october/test-site/vendor/laravel/framework/src/Illuminate/Container/Container.php(549): Illuminate\Container\BoundMethod::call(Object(October\Rain\Foundation\Application), Array, Array, NULL)
#18 /Sites/october/test-site/vendor/laravel/framework/src/Illuminate/Console/Command.php(183): Illuminate\Container\Container->call(Array)
#19 /Sites/october/test-site/vendor/symfony/console/Command/Command.php(255): Illuminate\Console\Command->execute(Object(Symfony\Component\Console\Input\ArrayInput), Object(Illuminate\Console\OutputStyle))
#20 /Sites/october/test-site/vendor/laravel/framework/src/Illuminate/Console/Command.php(170): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArrayInput), Object(Illuminate\Console\OutputStyle))
#21 /Sites/october/test-site/vendor/symfony/console/Application.php(982): Illuminate\Console\Command->run(Object(Symfony\Component\Console\Input\ArrayInput), Object(Illuminate\Console\OutputStyle))
#22 /Sites/october/test-site/vendor/symfony/console/Application.php(255): Symfony\Component\Console\Application->doRunCommand(Object(System\Console\OctoberUtil), Object(Symfony\Component\Console\Input\ArrayInput), Object(Illuminate\Console\OutputStyle))
#23 /Sites/october/test-site/vendor/symfony/console/Application.php(148): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArrayInput), Object(Illuminate\Console\OutputStyle))
#24 /Sites/october/test-site/vendor/laravel/framework/src/Illuminate/Console/Application.php(88): Symfony\Component\Console\Application->run(Object(Symfony\Component\Console\Input\ArrayInput), Object(Illuminate\Console\OutputStyle))
#25 /Sites/october/test-site/vendor/laravel/framework/src/Illuminate/Console/Application.php(177): Illuminate\Console\Application->run(Object(Symfony\Component\Console\Input\ArrayInput), Object(Illuminate\Console\OutputStyle))
#26 /Sites/october/test-site/vendor/laravel/framework/src/Illuminate/Foundation/Console/Kernel.php(249): Illuminate\Console\Application->call('october:util', Object(October\Rain\Support\Collection), Object(Illuminate\Console\OutputStyle))
#27 /Sites/october/test-site/vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php(221): Illuminate\Foundation\Console\Kernel->call('october:util', Array, Object(Illuminate\Console\OutputStyle))
#28 /Sites/october/test-site/plugins/sqpx/core/console/Deploy.php(35): Illuminate\Support\Facades\Facade::__callStatic('call', Array)
#29 [internal function]: SQPX\Core\Console\Deploy->handle()
#30 /Sites/october/test-site/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(29): call_user_func_array(Array, Array)
#31 /Sites/october/test-site/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(87): Illuminate\Container\BoundMethod::Illuminate\Container\{closure}()
#32 /Sites/october/test-site/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(31): Illuminate\Container\BoundMethod::callBoundMethod(Object(October\Rain\Foundation\Application), Array, Object(Closure))
#33 /Sites/october/test-site/vendor/laravel/framework/src/Illuminate/Container/Container.php(549): Illuminate\Container\BoundMethod::call(Object(October\Rain\Foundation\Application), Array, Array, NULL)
#34 /Sites/october/test-site/vendor/laravel/framework/src/Illuminate/Console/Command.php(183): Illuminate\Container\Container->call(Array)
#35 /Sites/october/test-site/vendor/symfony/console/Command/Command.php(255): Illuminate\Console\Command->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Illuminate\Console\OutputStyle))
#36 /Sites/october/test-site/vendor/laravel/framework/src/Illuminate/Console/Command.php(170): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Illuminate\Console\OutputStyle))
#37 /Sites/october/test-site/vendor/symfony/console/Application.php(982): Illuminate\Console\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#38 /Sites/october/test-site/vendor/symfony/console/Application.php(255): Symfony\Component\Console\Application->doRunCommand(Object(SQPX\Core\Console\Deploy), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#39 /Sites/october/test-site/vendor/symfony/console/Application.php(148): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#40 /Sites/october/test-site/vendor/laravel/framework/src/Illuminate/Console/Application.php(88): Symfony\Component\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#41 /Sites/october/test-site/vendor/laravel/framework/src/Illuminate/Foundation/Console/Kernel.php(121): Illuminate\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#42 /Sites/october/test-site/artisan(35): Illuminate\Foundation\Console\Kernel->handle(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#43 {main}  

Looks like they fixed it in the Wikimedia copy of Less.php with this commit wikimedia/less.php@9d1a0dc

Do you want me to create a PR to add in this change?

@summercms
Copy link
Contributor Author

Going to write here some info:

PHP Version 7.4.1 is a security update and fixes the following security issues:

Bcmath:

Core:

EXIF:

October CMS uses the GD Plugin:

  • Fixed bug #78849 (GD build broken with -D SIGNED_COMPARE_SLOW).

  • Fixed bug #78923 (Artifacts when convoluting image with transparency).

@LukeTowers
Copy link
Contributor

@caloggs if you would like to submit a PR to Assetic to switch the LESS libraries used that would be great.

@caloggs
Copy link
Contributor

caloggs commented Jan 20, 2020

@LukeTowers The Assetic library you've linked seems to be using a different less package, its using this one https://github.com/leafo/lessphp and the issue is in this one https://github.com/oyejorge/less.php which has been taken over by https://github.com/wikimedia/less.php

I believe copying wikimedia/less.php@9d1a0dc to https://github.com/octobercms/library/blob/master/src/Parse/Assetic/Less/lib/Less/Tree/Import.php#L216-L218 will solve the issue, or am I missing something?

@LukeTowers
Copy link
Contributor

@caloggs sure, you can submit that PR for now. Assetic will be moving to use Wikipedia's one as a part of the L6 update so it's fine if we continue with the temporary monkey patch for now.

@caloggs
Copy link
Contributor

caloggs commented Jan 21, 2020

@LukeTowers Thanks for clarifying, will submit both PRs later today

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

9 participants