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 compatibility #54

Closed
tuxayo opened this issue Jun 28, 2016 · 4 comments
Closed

php 7 compatibility #54

tuxayo opened this issue Jun 28, 2016 · 4 comments
Labels
bug This is a bug (not an enhancement)

Comments

@tuxayo
Copy link
Contributor

tuxayo commented Jun 28, 2016

When setting up the project with PHP 7
And running going to /management/install/
I see in the logs Call to undefined function mysql_connect() in /srv/http/coral/management/install/CORALInstaller.php:35
Which is a function of the mysql PHP extension
Which has been removed in PHP 7, as summed up here

There was already work done in the admin and install modules.

Grepping in the code shows that the Management and Report module are the last places using mysql_connect() and mysql_query()

I'll try to migrate them using the existing work.

@jeffnm
Copy link
Member

jeffnm commented Jun 28, 2016

CORAL doesn't officially support PHP 7, but the mysql vs. mysqli is an issue.

It appears that the management module never had its mysql functions converted to mysqli. This includes the installer, but also DBService.php. If you are going to attempt to update it, would you be willing to do the whole module?

@tuxayo
Copy link
Contributor Author

tuxayo commented Jun 28, 2016

If you are going to attempt to update it, would you be willing to do the whole module?

That's what I planned to do if I manage to understand well enough the conversion process.

tuxayo added a commit to tuxayo/Coral that referenced this issue Jun 30, 2016
the process was the same as:
ndlibersa/resources@26ae17c
(saved on https://archive.is if repo is deleted)
tuxayo added a commit to tuxayo/Coral that referenced this issue Jun 30, 2016
tuxayo added a commit to tuxayo/Coral that referenced this issue Jun 30, 2016
tuxayo added a commit to tuxayo/Coral that referenced this issue Jul 1, 2016
The process was the same as:
ndlibersa/resources@26ae17c
(saved on https://archive.is if repo is deleted)

And fix error in management conversion
tuxayo added a commit to tuxayo/Coral that referenced this issue Jul 1, 2016
The process was the same as:
ndlibersa/resources@26ae17c
(saved on https://archive.is if repo is deleted)

And fix error in management conversion
tuxayo added a commit to tuxayo/Coral that referenced this issue Jul 5, 2016
the process was the same as:
ndlibersa/resources@26ae17c
(saved on https://archive.is if repo is deleted)
tuxayo added a commit to tuxayo/Coral that referenced this issue Jul 5, 2016
The process was the same as:
ndlibersa/resources@26ae17c
(saved on https://archive.is if repo is deleted)

And fix error in management conversion
tuxayo added a commit to tuxayo/Coral that referenced this issue Jul 5, 2016
mysql was fine with: WHERE c.`licenseID`=
whereas mysqli throws a syntax error and needs: WHERE c.`licenseID`=''

Part of coral-erm#54
@PaulPoulain PaulPoulain added the bug This is a bug (not an enhancement) label Jul 6, 2016
tuxayo added a commit to tuxayo/Coral that referenced this issue Jul 6, 2016
To avoid regressions because mysql was fine with: WHERE c.`licenseID`=
whereas mysqli throws a syntax error and needs: WHERE c.`licenseID`=''

Part of coral-erm#54
tuxayo added a commit to tuxayo/Coral that referenced this issue Jul 6, 2016
To avoid regressions because mysql was fine with: WHERE c.`licenseID`=
whereas mysqli throws a syntax error and needs: WHERE c.`licenseID`=''
This is preventive and not in response to observed regressions.
Also it improves consistency as most of Coral's SQL building use this
form for WHERE parameters.

Part of coral-erm#54
tuxayo added a commit to tuxayo/Coral that referenced this issue Jul 12, 2016
the process was the same as:
ndlibersa/resources@26ae17c
(saved on https://archive.is if repo is deleted)
tuxayo added a commit to tuxayo/Coral that referenced this issue Jul 12, 2016
The process was the same as:
ndlibersa/resources@26ae17c
(saved on https://archive.is if repo is deleted)

And fix error in management conversion
tuxayo added a commit to tuxayo/Coral that referenced this issue Jul 12, 2016
mysql was fine with: WHERE c.`licenseID`=
whereas mysqli throws a syntax error and needs: WHERE c.`licenseID`=''

Part of coral-erm#54
tuxayo added a commit to tuxayo/Coral that referenced this issue Jul 12, 2016
To avoid regressions because mysql was fine with: WHERE c.`licenseID`=
whereas mysqli throws a syntax error and needs: WHERE c.`licenseID`=''
This is preventive and not in response to observed regressions.
Also it improves consistency as most of Coral's SQL building use this
form for WHERE parameters.

Part of coral-erm#54
@tuxayo
Copy link
Contributor Author

tuxayo commented Aug 8, 2016

Just for anyone wondering, as #75 has been merged in the master branch, (code candidate for the next release) there is initial PHP 7 support.
As of now there aren't any known issues. But very few QA/testing has been done so issues might have yet to be discovered so anyone is welcome to do QA/testing or development using PHP 7 instead of PHP 5.

@jeffnm
Copy link
Member

jeffnm commented Apr 19, 2017

I think CORAL supports PHP 7 now. If future issues are identified that cause issues with that particular version of PHP, we'll open new issues to deal with them. We officially support PHP 5.5 and higher with 2.0.0

@jeffnm jeffnm closed this as completed Apr 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This is a bug (not an enhancement)
Projects
None yet
Development

No branches or pull requests

3 participants