style(PHP): Upgrade fossology project from PHP 7 to PHP 8 #1925

sritasngh · 2021-03-22T08:12:24Z

Description

The project currently uses PHP 7.x and is partially incompatible with PHP 8. It would be preferable to upgrade this project to PHP 8 for better compatibility with the latest technologies.

signed-off-by: Sarita Singh saritasingh.0425@gmail.com
fixes #1920

Changes

In ./lib/php/Util/StringOperation.php on line 33
In ./nomos/agent_tests/testdata/NomosTestfiles/AGPL/JsonMapper.php on line 109
replaced curly braces with [ ].

How to test

Run Syntax Checks for PHP8 using command: bash src/testing/syntax/syntaxtest.sh

Array and string offset access syntax with curly braces is no longer supported in PHP 8 so made replaced curly braces with []. signed-off-by: Sarita Singh<saritasingh.0425@gmail.com>

sritasngh · 2021-03-22T11:15:55Z

@GMishx I have change syntax incompatible with PHP 8 and now it's working fine for PHP 8 and all the CI. Is there any other change needed?

GMishx · 2021-03-22T11:52:26Z

The PR looks good 👍

But I see there was an error on src/nomos/agent_tests/testdata/NomosTestfiles/AGPL/JsonMapper.php which is a test file. Would it be okay to ignore the src/nomos/agent_tests/testdata folder from the syntax test?

sritasngh · 2021-03-22T14:07:43Z

If we ignore the src/nomos/agent_tests/testdata folder from the syntax test then also It will work fine but since we are migrating the project to PHP 8 so updating test would be a good idea as curly brace syntax has been deprecated for accessing array elements and string offsets from PHP 7.4. Besides syntax, there is no change in the working of code so It won't affect anyway.

GMishx · 2021-03-23T05:25:07Z

The test files in src/nomos/agent_tests/testdata are never executed. They are there for regression test on the nomos agent's license detection. It can safely be ignored.

You can keep the syntax changes done by you (as it does not fail any test case) but ignore the directory will be helpful for future when a new PHP version is out.

sritasngh · 2021-03-23T10:56:02Z

@GMishx Okay I'll ignore the test file but in that case the checks will still fail for PHP 8.0. So should I exclude src/nomos/agent_tests/testdata from syntaxtest.sh

GMishx · 2021-03-23T11:59:59Z

Oh, sorry for the confusion, the changes with the PHP can be ignored or can be kept, it does not matter at the moment. Just wanted to ignore the directory from the syntaxtest.sh itself for future incompatibility issues.

sritasngh · 2021-03-23T17:46:32Z

@GMishx I have excluded src/nomos/agent_tests/testdata from syntaxtest.sh. Please review!

src/testing/syntax/syntaxtest.sh

Signed-off-by: Sarita Singh<saritasingh.0425@gmail.com>

GMishx

Changes looks good. Need to set setup on PHP8

sritasngh · 2021-03-24T10:44:50Z

@GMishx Thanks! Please let me know if it requires more changes.

GMishx · 2021-03-24T10:46:58Z

@GMishx Thanks! Please let me know if it requires more changes.

If something came up during testing, will put it here. :-)

GMishx · 2021-04-08T11:59:58Z

Unfortunately, the code is producing a lot of warnings when running on PHP8. I have captured the following logs for reference.

Logs from Apache error log. php-8-error-log-unique.txt
I have selected only the unique entries, otherwise the file was too big.
Logs from scheduler. php-8-fossology-log.txt
Note that the PHP agents have failed because of the warnings. Scheduler expects the first output to be the agent's version.
Logs from fo-postinstall script. php-8-postinstall.txt

sritasngh · 2021-04-09T10:22:26Z

Okay I'll go through them and make the required changes.

sritasngh · 2021-04-16T17:37:45Z

Unfortunately, the code is producing a lot of warnings when running on PHP8. I have captured the following logs for reference.

Is there a way to reproduce these errors locally?

GMishx · 2021-04-20T05:28:39Z

Is there a way to reproduce these errors locally?

Yes, you can install PHP 8 and simply install FOSSology on top of it.

If you do not want to install PHP-8, you can try chroot or create a Docker container on Debian and install PHP-8 from Ondřej Surý's repository

apt-get install -y apt-transport-https lsb-release ca-certificates curl
curl --output /etc/apt/trusted.gpg.d/php.gpg https://packages.sury.org/php/apt.gpg
echo "deb https://packages.sury.org/php/ $(lsb_release -sc) main" > /etc/apt/sources.list.d/php.list
apt-get update

sritasngh · 2021-05-04T08:44:57Z

I am unable to reproduce the logs on my local machines with php8. I am getting some of the given fo-postinstall warning but not all. Could you share me actual steps you have used for testing so that I can reproduce these errors and warnings locally?

GMishx · 2021-05-04T09:36:31Z

I am getting some of the given fo-postinstall warning but not all.

Maybe because you are upgrading an existing installation? But the fix should be same for all.

Could you share me actual steps you have used for testing so that I can reproduce these errors and warnings locally?

Sure, so I took the existing Dockerfile and modified it to install PHP8. Then I build a container out of it and captured all logs from it.
Here is my Dockefile: https://gist.github.com/GMishx/5ccf1fbb231c4a2ecf8fcc84c8b69f43

shaheemazmalmmd · 2022-06-06T07:55:39Z

@itssingh Please reopen, once you do some further work on this PR.

Commit 1 from fossology#1925 style(php):upgrade fossology from PHP7 to PHP8 Array and string offset access syntax with curly braces is no longer supported in PHP 8 so made replaced curly braces with []. Commit 2 from fossology#1925 refactor(syntaxtest.sh): Exclude testdata folder from syntax test Signed-off-by: Sarita Singh <saritasingh.0425@gmail.com> Signed-off-by: Gaurav Mishra <mishra.gaurav@siemens.com>

style(php):upgrade fossology from PHP7 to PHP8

e943858

Array and string offset access syntax with curly braces is no longer supported in PHP 8 so made replaced curly braces with []. signed-off-by: Sarita Singh<saritasingh.0425@gmail.com>

sritasngh changed the title ~~Upgrade fossology project from PHP 7 to PHP 8~~ style(PHP): Upgrade fossology project from PHP 7 to PHP 8 Mar 22, 2021

sritasngh force-pushed the php7_to_php8 branch from 99b5b1d to e943858 Compare March 22, 2021 10:18

sritasngh marked this pull request as draft March 22, 2021 10:28

sritasngh marked this pull request as ready for review March 22, 2021 10:59

GMishx added enhancement GSOC-21 Label to tag issues and pull request for GSOC 2021 activities needs code review needs test labels Mar 22, 2021

GMishx mentioned this pull request Mar 23, 2021

Upgrade this project from PHP 7 to PHP 8 #1920

Closed

GMishx requested changes Mar 24, 2021

View reviewed changes

src/testing/syntax/syntaxtest.sh Outdated Show resolved Hide resolved

refactor(syntaxtest.sh): Exclude testdata folder from syntax test

1a0f58f

Signed-off-by: Sarita Singh<saritasingh.0425@gmail.com>

sritasngh force-pushed the php7_to_php8 branch from 5e1abbc to 1a0f58f Compare March 24, 2021 10:27

GMishx approved these changes Mar 24, 2021

View reviewed changes

GMishx removed the needs code review label Mar 24, 2021

GMishx added the WIP label Apr 8, 2021

shaheemazmalmmd closed this Jun 6, 2022

GMishx mentioned this pull request Aug 1, 2022

feat(php): make PHP 8 compatible #2284

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

style(PHP): Upgrade fossology project from PHP 7 to PHP 8 #1925

style(PHP): Upgrade fossology project from PHP 7 to PHP 8 #1925

style(PHP): Upgrade fossology project from PHP 7 to PHP 8 #1925

style(PHP): Upgrade fossology project from PHP 7 to PHP 8 #1925

Conversation

Description

Changes

How to test

Choose a reason for hiding this comment