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

Ensure PHP7 compatibility #1118

Closed
asmecher opened this Issue Feb 4, 2016 · 13 comments

Comments

Projects
None yet
3 participants
@asmecher
Copy link
Member

asmecher commented Feb 4, 2016

There's at least 1 compatibility problem: the String class is now a reserved name. Test and ensure compatibility.

@asmecher asmecher self-assigned this Feb 4, 2016

@asmecher asmecher added this to the OJS 2.4.8 milestone Feb 4, 2016

asmecher added a commit to asmecher/pkp-lib that referenced this issue Feb 4, 2016

asmecher added a commit to asmecher/pkp-lib that referenced this issue Feb 4, 2016

@asmecher

This comment has been minimized.

Copy link
Member Author

asmecher commented Feb 4, 2016

Things needing doing:

String class renaming:

  • find . -type f -name \*.inc.php -exec sed -i -e "s/String::/PKPString::/g" "{}" ";"
  • Edit imports in:
    • lib/pkp/classes/search/SearchHTMLParser.inc.php
    • lib/pkp/classes/core/PKPApplication.inc.php
  • Rename lib/pkp/classes/core/String.inc.php and adjust class header etc.

New-by-reference is now a syntax error, even if it's in a conditional "if". Remove.

MySQL (not MySQLi) functions removed. Need to use ADODB's MySQLi driver instead.

"split" removed. Suggest "explode" instead. (In ADODB.)

And maybe others... Will raise this on Friday for OJS 2.4.8.

asmecher added a commit to asmecher/ojs that referenced this issue Feb 4, 2016

asmecher added a commit to asmecher/ojs that referenced this issue Feb 4, 2016

@asmecher asmecher modified the milestones: OMP 1.2, OJS 2.4.8 Feb 6, 2016

@asmecher

This comment has been minimized.

Copy link
Member Author

asmecher commented Feb 6, 2016

It looks like market share for PHP 7 is currently around 1%, so I'm considering this a non-issue for OJS 2.4.8. Rescheduling for OMP 1.2, and of course for OJS 3.0.

asmecher added a commit that referenced this issue Feb 19, 2016

asmecher added a commit to pkp/omp that referenced this issue Feb 19, 2016

asmecher added a commit that referenced this issue Feb 19, 2016

asmecher added a commit to pkp/tinymce that referenced this issue Feb 19, 2016

asmecher added a commit to pkp/ojs that referenced this issue Feb 19, 2016

asmecher added a commit that referenced this issue Feb 19, 2016

asmecher added a commit to pkp/staticPages that referenced this issue Feb 19, 2016

asmecher added a commit to pkp/customBlockManager that referenced this issue Feb 19, 2016

asmecher added a commit to pkp/tinymce that referenced this issue Feb 19, 2016

asmecher added a commit to pkp/translator that referenced this issue Feb 19, 2016

asmecher added a commit to pkp/omp that referenced this issue Feb 19, 2016

asmecher added a commit to pkp/omp that referenced this issue Feb 19, 2016

@asmecher

This comment has been minimized.

Copy link
Member Author

asmecher commented Feb 19, 2016

Lots of warnings about function definitions not matching, but those are suppressable and will take some time to get through. Good enough while PHP7 is still nascent in the hosting world.

@asmecher asmecher closed this Feb 19, 2016

asmecher added a commit that referenced this issue Feb 20, 2016

asmecher added a commit to pkp/ojs that referenced this issue Feb 20, 2016

asmecher added a commit to pkp/omp that referenced this issue Feb 20, 2016

@pjohanneson

This comment has been minimized.

Copy link

pjohanneson commented Jun 13, 2016

PHP7 removes mysql_connect(), which means I'm getting this on an OJS installation I'm trying to migrate to Ubuntu 16.04 LTS (which installs PHP7 by default):

Fatal error: Uncaught Error: Call to undefined function mysql_connect() in /var/www/vhosts/SERVER/lib/pkp/lib/adodb/drivers/adodb-mysql.inc.php:456

@pjohanneson

This comment has been minimized.

Copy link

pjohanneson commented Jun 13, 2016

Never mind -- I fixed my "problem" by setting the DB driver in config.inc.php to mysqli instead of mysql.

@thebiomat

This comment has been minimized.

Copy link

thebiomat commented Sep 7, 2017

PHP 7 is eminent. Our server providers are switching completely to PHP 7 in less than an month. We have to either move to a VPS or upgrade OJS to PHP 7. Would prefer the latter. Will there be any help available with updating the OJS 248 code to PHP7?

@asmecher

This comment has been minimized.

Copy link
Member Author

asmecher commented Sep 7, 2017

Hmm, it looks like PHP7 has taken a sudden leap in server share: https://seld.be/notes/php-versions-stats-2017-1-edition

There shouldn't be a massive amount of work required to make OJS 2.4.x PHP7-compatible, but I'll have to take a look.

@thebiomat

This comment has been minimized.

Copy link

thebiomat commented Sep 7, 2017

asmecher added a commit to pkp/ojs that referenced this issue Sep 7, 2017

@asmecher

This comment has been minimized.

Copy link
Member Author

asmecher commented Sep 7, 2017

@thebiomat, try the following patches:

These were enough to run OJS through its test suite, which includes quite a bit of workflow exercise etc. However, there will be a pile of warnings due to PHP7's increased pickiness (we're still running through these in the OJS 3.x codebase and aren't likely to backport any of that unpleasant work); I also suspect that there will be corners of OJS that'll cause PHP7 to throw errors, but those should be clearly indicated by an error message and we can follow up on them here. I don't expect any of those to require more than tweaks.

I've tested with PHP7.1.0.

If you get the chance to give this a crack, please let me know what your results are. Make sure to clear your template cache (rm cache/t_compile/*.php) to get OJS to recompile all its templates after applying the patches.

@thebiomat

This comment has been minimized.

Copy link

thebiomat commented Sep 8, 2017

That's a really fast response. Thank you very much. We'll give it a try. Since this is a bit over my head, I'll need first ask one of the other folks involved in the server installation.

@thebiomat

This comment has been minimized.

Copy link

thebiomat commented Sep 8, 2017

@asmecher

This comment has been minimized.

Copy link
Member Author

asmecher commented Sep 8, 2017

@thebiomat, you can download a patch file from a git commit URL (like the two above) by adding .diff to the end of the URL. Then, you can apply each patch by following instructions like these:

https://www.lifewire.com/patch-linux-command-unix-command-4097158

See the "how to apply a patch" section of the instructions; use the -p1 flag to the patch tool. The --dry-run option is also useful to test a patch before actually trying to apply it.

@thebiomat

This comment has been minimized.

Copy link

thebiomat commented Sep 8, 2017

Thank you. We'll try it and let you know.

ctgraham added a commit to ulsdevteam/pkp-lib that referenced this issue Nov 14, 2017

ctgraham added a commit to ulsdevteam/ojs that referenced this issue Nov 14, 2017

ctgraham added a commit to ulsdevteam/ojs that referenced this issue Nov 14, 2017

pkp/pkp-lib#1118, pkp/pkp-lib#3054: Catchup update on some String:: t…
…o PKPString:: test outliers ##ulsdevteam/string-to-pkpstring-outliers##

asmecher added a commit that referenced this issue Nov 14, 2017

Merge pull request #3055 from ulsdevteam/string-to-pkpstring-outliers
#1118, #3054: Catchup update on some String:: to PKPString:: test outliers

asmecher added a commit to pkp/ojs that referenced this issue Nov 14, 2017

Merge pull request #1690 from ulsdevteam/string-to-pkpstring-outliers
pkp/pkp-lib#1118, pkp/pkp-lib#3054: Catchup update on some String:: to PKPString:: test outliers

mtub added a commit to mtub/ojs that referenced this issue Dec 23, 2017

mtub added a commit to mtub/ojs that referenced this issue Dec 23, 2017

pkp/pkp-lib#1118, pkp/pkp-lib#3054: Catchup update on some String:: t…
…o PKPString:: test outliers ##ulsdevteam/string-to-pkpstring-outliers##
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment