? Pending

User tests: Successful: Unsuccessful:

avatar SharkyKZ
SharkyKZ
5 Aug 2019

Summary of Changes

Replaces curly brackets used for array and string access with square brackets to solve deprecation warnings in PHP 7.4:

Array and string offset access syntax with curly braces is deprecated

Testing Instructions

Code review?

Documentation Changes Required

No.

avatar SharkyKZ SharkyKZ - open - 5 Aug 2019
avatar SharkyKZ SharkyKZ - change - 5 Aug 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 5 Aug 2019
Category Administration com_joomlaupdate Repository Installation Libraries Front End Plugins
avatar SharkyKZ SharkyKZ - change - 5 Aug 2019
Title
Replace curly braces for PHP 7.4 compat
Replace curly braces for PHP 7.4 compatibility
avatar SharkyKZ SharkyKZ - edited - 5 Aug 2019
avatar wilsonge
wilsonge - comment - 5 Aug 2019

Given this as a PR to @nikosdion akeeba/angie#76 so we can try and get an official version of the restore.php code with this in. Rather than starting to monkey patch this ourselves. If we do need to patch this way we'll need to modify the copyright at the top of restore.php similar to how we did in fof with 4889f00

avatar SharkyKZ
SharkyKZ - comment - 5 Aug 2019

Should we make changes to FoF too? And what about other libraries like idna_convert and lessphp?

avatar brianteeman
brianteeman - comment - 5 Aug 2019

Should we make changes to FoF too? And what about other libraries like idna_convert and lessphp?

Never change an upstream library. If needed submit a pull request to that libraries vendor

avatar SharkyKZ
SharkyKZ - comment - 5 Aug 2019

Those libraries aren't maintained. We did make changes to FoF in the past https://github.com/joomla/joomla-cms/commits/staging/libraries/fof.

avatar wilsonge
wilsonge - comment - 5 Aug 2019

FOF is fine to change. idna convert is also fine as it's something internal to string and doesn't exist on packagist. i already did that one in the framework joomla-framework/string@33944ac

Let's try submitting lessphp back to them - leafo/lessphp@v0.5.0...master but there hasn't been a release in nearly 5 years - so unsure whether they'll actually accept it or not.

avatar SharkyKZ
SharkyKZ - comment - 5 Aug 2019

Submitted leafo/lessphp#647.

avatar SharkyKZ SharkyKZ - change - 5 Aug 2019
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 5 Aug 2019
Category Administration com_joomlaupdate Repository Installation Libraries Front End Plugins Administration com_joomlaupdate Repository Installation External Library Libraries Front End Plugins
c37a84e 5 Aug 2019 avatar SharkyKZ FOF
avatar nikosdion
nikosdion - comment - 6 Aug 2019

Hey, guys. Thank you for the PR! George made it to the wrong repo but don't worry, I know where else to apply it :)

@wilsonge Do you want me to make a PR with the new version of restore.php? If so, only J3 or both J3 and J4?

avatar wilsonge
wilsonge - comment - 6 Aug 2019

Thankyou very much! Sorry for the wrong repo thing. Just to j3 please and it will get merged upto j4

avatar wilsonge
wilsonge - comment - 6 Aug 2019

@SharkyKZ can you remove the restore.php changes from this PR please and they'll be covered in @nikosdion 's separate PR

avatar joomla-cms-bot joomla-cms-bot - change - 6 Aug 2019
Category Administration com_joomlaupdate Repository Installation Libraries Front End Plugins External Library Repository Installation External Library Libraries Front End Plugins
avatar SharkyKZ
SharkyKZ - comment - 6 Aug 2019

Done.

avatar nikosdion
nikosdion - comment - 6 Aug 2019

I PR'ed the new restore.php, see #25793

BTW, has anyone managed to reliably run PHP 7.4 betas on either macOS or Ubuntu 19.04? I don't want to have to pull in half of the known universe to compile it myself on Ubuntu, I really don't have the time anymore :/

avatar wilsonge
wilsonge - comment - 7 Aug 2019

I haven't yet - but we're doing it in drone with the php 7.4 nightly docker container - so i think that's where i'm going to start at https://github.com/joomla/joomla-cms/blob/4.0-dev/.drone.yml#L73

avatar nikosdion
nikosdion - comment - 7 Aug 2019

@wilsonge OK, that's pretty much what I thought I would have to do. I am not a huge fan of dockerizing everything because of various issues I've had with XDebug, hostnames, testing CLI applications and ownership / permissions of files in mounted volumes but I digress. I am now using https://github.com/devilbox/docker-php-fpm-7.4 because it solves the ownership / permissions issue and creates a tidy PHP-FPM server I can use with my localhost setup, e.g.:

docker run \
	--rm -d --name devilbox-php74 \
	-e NEW_UID=$(id -u) \
	-e NEW_GID=$(id -g) \
	-e TIMEZONE="Europe/Athens" \
	-e DOCKER_LOGS=0 \
	-e FORWARD_PORTS_TO_LOCALHOST="3306:host.docker.internal:3306, 1025:host.docker.internal:1025, 6379:host.docker.internal:6379" \
	-p 127.0.0.1:9074:9000 \
	-v $(pwd)/logs:/var/log/php \
	-v $(pwd)/php_config:/etc/php-custom.d \
	-v /path/to/my/sites:/path/to/my/sites \
	-v /path/to/my/git/repos:/path/to/my/git/repos \
	-t devilbox/php-fpm:7.4-work

to start the PHP-FPM server and then a couple of lines in my .htaccess to use it:

<FilesMatch "\.php$">
SetHandler "proxy:fcgi://127.0.0.1:9074/"
</FilesMatch>
avatar C-Lodder
C-Lodder - comment - 9 Aug 2019

R.E the lessphp library, it looks like one of the forks has a lot of PR ported from the original repo and has more interaction: https://github.com/MarcusSchwarz/lesserphp

May be an idea to submit your PR there

avatar SharkyKZ
SharkyKZ - comment - 9 Aug 2019

@C-Lodder I don't think that library is compatible with ours.

avatar Quy Quy - test_item - 27 Sep 2019 - Tested successfully
avatar Quy
Quy - comment - 27 Sep 2019

I have tested this item successfully on 2861347


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25782.

avatar mbabker
mbabker - comment - 27 Sep 2019

Files in libraries/idna_convert need a notification they have been altered.

avatar SharkyKZ
SharkyKZ - comment - 27 Sep 2019

@mbabker Added note. Thanks.

avatar HLeithner HLeithner - change - 28 Sep 2019
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2019-09-28 09:17:49
Closed_By HLeithner
avatar HLeithner HLeithner - close - 28 Sep 2019
avatar HLeithner HLeithner - merge - 28 Sep 2019
avatar HLeithner
HLeithner - comment - 28 Sep 2019

Thanks for fixing deprecated the syntax.

Add a Comment

Login with GitHub to post a comment