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

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

Closed
wants to merge 2 commits into from

Conversation

sritasngh
Copy link
Contributor

@sritasngh sritasngh commented Mar 22, 2021

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 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 sritasngh marked this pull request as draft March 22, 2021 10:28
@sritasngh sritasngh marked this pull request as ready for review March 22, 2021 10:59
@sritasngh
Copy link
Contributor Author

sritasngh commented Mar 22, 2021

@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 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
Copy link
Member

GMishx commented Mar 22, 2021

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
Copy link
Contributor Author

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
Copy link
Member

GMishx commented Mar 23, 2021

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
Copy link
Contributor Author

@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
Copy link
Member

GMishx commented Mar 23, 2021

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
Copy link
Contributor Author

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

src/testing/syntax/syntaxtest.sh Outdated Show resolved Hide resolved
Signed-off-by: Sarita Singh<saritasingh.0425@gmail.com>
Copy link
Member

@GMishx GMishx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes looks good. Need to set setup on PHP8

@sritasngh
Copy link
Contributor Author

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

@GMishx
Copy link
Member

GMishx commented Mar 24, 2021

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

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

@GMishx
Copy link
Member

GMishx commented Apr 8, 2021

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

  1. Logs from Apache error log. php-8-error-log-unique.txt
    I have selected only the unique entries, otherwise the file was too big.
  2. 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.
  3. Logs from fo-postinstall script. php-8-postinstall.txt

@GMishx GMishx added the WIP label Apr 8, 2021
@sritasngh
Copy link
Contributor Author

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

@sritasngh
Copy link
Contributor Author

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
Copy link
Member

GMishx commented Apr 20, 2021

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
Copy link
Contributor Author

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
Copy link
Member

GMishx commented May 4, 2021

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
Copy link
Contributor

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

GMishx added a commit to siemens/fossology that referenced this pull request Aug 1, 2022
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>
GMishx added a commit to siemens/fossology that referenced this pull request Aug 1, 2022
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>
GMishx added a commit to siemens/fossology that referenced this pull request Aug 1, 2022
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>
GMishx added a commit to siemens/fossology that referenced this pull request Aug 2, 2022
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>
GMishx added a commit to siemens/fossology that referenced this pull request Aug 2, 2022
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>
GMishx added a commit to siemens/fossology that referenced this pull request Aug 2, 2022
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>
GMishx added a commit to siemens/fossology that referenced this pull request Aug 4, 2022
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>
GMishx added a commit to siemens/fossology that referenced this pull request Aug 4, 2022
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>
GMishx added a commit to siemens/fossology that referenced this pull request Aug 4, 2022
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>
GMishx added a commit to siemens/fossology that referenced this pull request Aug 4, 2022
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>
GMishx added a commit to siemens/fossology that referenced this pull request Aug 4, 2022
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>
GMishx added a commit to siemens/fossology that referenced this pull request Aug 4, 2022
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>
GMishx added a commit to siemens/fossology that referenced this pull request Aug 5, 2022
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>
GMishx added a commit to siemens/fossology that referenced this pull request Aug 8, 2022
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>
GMishx added a commit to siemens/fossology that referenced this pull request Aug 8, 2022
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>
GMishx added a commit to siemens/fossology that referenced this pull request Aug 12, 2022
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>
fwhdzh pushed a commit to fwhdzh/fossology that referenced this pull request Sep 12, 2022
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>
fwhdzh pushed a commit to fwhdzh/fossology that referenced this pull request Sep 12, 2022
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement GSOC-21 Label to tag issues and pull request for GSOC 2021 activities needs test WIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade this project from PHP 7 to PHP 8
3 participants