octobercms / october

Self-hosted CMS platform based on the Laravel PHP Framework.
https://octobercms.com/
Other
11.01k stars 2.21k forks source link

PHP 7.4 support #4797

Closed summercms closed 4 years ago

summercms commented 4 years ago

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: https://github.com/alserom/octobercms_clearcachewidget/issues/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: https://github.com/rainlab/googleanalytics-plugin/issues/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: https://github.com/scottbedard/analyticsextension/issues/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: https://github.com/twigphp/Twig/issues/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 commented 4 years ago

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:

Other changes upgrading

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

Here are some changes highlighted:

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

bennothommo commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

@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: https://github.com/laravel/framework/pull/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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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

Some plugins still may require attention

summercms commented 4 years ago

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 commented 4 years ago

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

p5ych0 commented 4 years ago

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 commented 4 years ago

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

p5ych0 commented 4 years ago

@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 commented 4 years ago

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

LukeTowers commented 4 years ago

@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 commented 4 years ago

image

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

LukeTowers commented 4 years ago

@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 commented 4 years ago

The fix for the combiner issue is here: https://github.com/octobercms/library/commit/b0566659e8d434a0e57f0cf8055c84996c9148a6

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 commented 4 years ago

@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 commented 4 years ago

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

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

Returns

a {
  color: red; }
caloggs commented 4 years ago

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 https://github.com/wikimedia/less.php/commit/9d1a0dc2934689b7880894409c00e3a2ae68748d

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

summercms commented 4 years ago

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:

LukeTowers commented 4 years ago

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

caloggs commented 4 years ago

@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 https://github.com/wikimedia/less.php/commit/9d1a0dc2934689b7880894409c00e3a2ae68748d 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 commented 4 years ago

@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 commented 4 years ago

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