Not noting anything popping out on testing yet. Qualitatively, it appears performance is about the same(if google this topic, it does appear mysqli is a tiny bit slower than mysql). Will likely commit this to the codebase over next couple days(note this will not be in the planned 4.3.0 release to allow more maturity time in the development codebase).
After this commit goes in, then the next step will be running OpenEMR on php7 and seeing what breaks.
Hi,
I just committed more work by John's group towards PHP7 compatibility issues. It was upgrading the adodb library to most recent adodb release (which is PHP7 compatibile).
-brady OpenEMR
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I just committed more work by John's group that removed the adodb libraries from phpgacl and postnuke calendar(so everything now uses one adodb library in library/adodb directory).
There is a possibility that we may need to upgrade smarty for php7, which is not very straightforward since there are 3 smarty libraries in OpenEMR (also would require lots of testing considering high risk of bugs). To get an idea of the mess involved, made a page here with overview of the smarty libraries in OpenEMR: http://www.open-emr.org/wiki/index.php/Smarty_Module
(if we did do this, then it seems like it would make sense to centralize all 3 into 1 upgraded smarty library)
Smarty 3 API has a new syntax. Much of the Smarty 2 syntax is supported
by a wrapper but deprecated. See the README that comes with Smarty 3 for more
information.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
John's group just updated the embedded html2pdf within OpenEMR to support php7 for that item. This is used to download patient report to a PDF file. Details are here: http://www.open-emr.org/wiki/index.php/Html2pdf
Note it should also work better for international users since this new html2pdf package now uses TCPDF.
I'm so pleased that John's group took the time to understand and update this. Thanks!
One suggestion, I think most of the fonts there will be unused and bloat the distribution too much. I expect just the core fonts will do: courier, helvetica, symbol, times, zapfdingbats.
Hi Brady, I'm sure Western European and similar alphabets will be fine, but I have no clue about fonts for Chinese, Arabic etc. Can anyone else answer that? In any case I think most of those fonts are just different styles for the same character sets.
Practice Provider's group has just posted the Smarty solution to support php7. Basically, the 3 different smarty packages in OpenEMR were centralized to one place, which was upgraded to the most recent smarty 2.6.x package(going to Smarty version 3 was not even close to possible). Since postnuke was using a much older version of Smarty, several changes were necessary in the calendar code to support the upgrade. On my extensive testing, everything looks very good. Here's the code, which plan to bring into the codebase after several days if no issues are brought up: https://github.com/bradymiller/openemr/commit/99f4b3b0684f1a106e7c1518e9b056c381eb3e67
That would be a lot of work that I do not have the resources for. It's been a large project, which has been difficult enough just keeping it in just 1 commit.
For commits of this size(where github just doesn't cut it), I generally use several strategies to review them:
git log -p
And then a bunch of clicking on 'Page Down'/'Page Up' button.
Create a patch file from the commit. Then it's easier to scroll around the code and view it in a text editor.
Actually, the patch file is easier to review in this situation(took me about 5 minutes to scroll through it and identify the items that were modified). This is because in github, it is showing a bunch of smarty scripts that were moved and changed(which was not the case). The patch file just shows the smarty files either being all removed or all added (thus makes it easy to identify which scripts were modified by scrolling through it).
Last edit: Brady Miller 2016-03-12
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I just committed the above commit. At this point the development codebase is now PHP7 compliant. There is still 1 related bug that is still in process of being fixed (the address/barcode/patient labels).
This php7 project has been a rather huge undertaking by Practice Provider that has touched on many central parts of the codebase. Please report any noted bugs.
Just committed fix for the pdf label issue. For now, have brought back the fpdf library to "fix" the labels. At some point, it will make sense to send all pdf output through tcpdf to support internationalization. There has already been some work in sending the labels through tcpdf, however, still lots of testing and configuration to get it right. See the following wiki page for details: http://www.open-emr.org/wiki/index.php/PDF#Overview
The php7 project is officially complete. Thank you Practice Provider!
Hi Rod,
Not noting anything popping out on testing yet. Qualitatively, it appears performance is about the same(if google this topic, it does appear mysqli is a tiny bit slower than mysql). Will likely commit this to the codebase over next couple days(note this will not be in the planned 4.3.0 release to allow more maturity time in the development codebase).
After this commit goes in, then the next step will be running OpenEMR on php7 and seeing what breaks.
-brady
OpenEMR
Hi,
I just committed the above commit to the codebase. Next step is to test OpenEMR on PHP7 and see what breaks,
-brady
OpenEMR
Hi,
I just committed more work by John's group towards PHP7 compatibility issues. It was upgrading the adodb library to most recent adodb release (which is PHP7 compatibile).
-brady
OpenEMR
Hi,
I just committed more work by John's group that removed the adodb libraries from phpgacl and postnuke calendar(so everything now uses one adodb library in library/adodb directory).
There is a possibility that we may need to upgrade smarty for php7, which is not very straightforward since there are 3 smarty libraries in OpenEMR (also would require lots of testing considering high risk of bugs). To get an idea of the mess involved, made a page here with overview of the smarty libraries in OpenEMR:
http://www.open-emr.org/wiki/index.php/Smarty_Module
(if we did do this, then it seems like it would make sense to centralize all 3 into 1 upgraded smarty library)
-brady
OpenEMR
Smarty 3.1.29 is released, many bug fixes.
Also this is now intended to be installed using Composer and NOT included in the code base of a given project....
Also worth noting: http://smarty-php.googlecode.com/svn/trunk/distribution/SMARTY_2_BC_NOTES.txt
Smarty 3 API has a new syntax. Much of the Smarty 2 syntax is supported
by a wrapper but deprecated. See the README that comes with Smarty 3 for more
information.
It is looking like may be able to get away without upgrading smarty. Still working on it.
-brady
Just updated the embedded phpmyadmin within OpenEMR to ensure support for php7 for that item.
-brady
OpenEMR
John's group just updated the embedded html2pdf within OpenEMR to support php7 for that item. This is used to download patient report to a PDF file. Details are here:
http://www.open-emr.org/wiki/index.php/Html2pdf
Note it should also work better for international users since this new html2pdf package now uses TCPDF.
-brady
OpenEMR
I'm so pleased that John's group took the time to understand and update this. Thanks!
One suggestion, I think most of the fonts there will be unused and bloat the distribution too much. I expect just the core fonts will do: courier, helvetica, symbol, times, zapfdingbats.
Rod
http://www.sunsetsystems.com/
Hi Rod,
Will removing fonts limit its international use?
-brady
OpenEMR
Hi Brady, I'm sure Western European and similar alphabets will be fine, but I have no clue about fonts for Chinese, Arabic etc. Can anyone else answer that? In any case I think most of those fonts are just different styles for the same character sets.
Rod
http://www.sunsetsystems.com/
Hi,
Practice Provider's group has just posted the Smarty solution to support php7. Basically, the 3 different smarty packages in OpenEMR were centralized to one place, which was upgraded to the most recent smarty 2.6.x package(going to Smarty version 3 was not even close to possible). Since postnuke was using a much older version of Smarty, several changes were necessary in the calendar code to support the upgrade. On my extensive testing, everything looks very good. Here's the code, which plan to bring into the codebase after several days if no issues are brought up:
https://github.com/bradymiller/openemr/commit/99f4b3b0684f1a106e7c1518e9b056c381eb3e67
Here's a wiki page decribing the process of centralizing/upgrading the smarty package:
http://www.open-emr.org/wiki/index.php/Smarty_Module#OpenEMR_4.3.1_and_above
Although this was necessary for php7, it's also a very nice improvement/simplification of the codebase.
-brady
OpenEMR
Is it possible to see this commit in two phases? (One the changes to OpenEMR files, and two the changes two the smarty libraries)
It's difficult to review 207 different files.
Hi Kevin,
That would be a lot of work that I do not have the resources for. It's been a large project, which has been difficult enough just keeping it in just 1 commit.
For commits of this size(where github just doesn't cut it), I generally use several strategies to review them:
git log -p
And then a bunch of clicking on 'Page Down'/'Page Up' button.
Create a patch file from the commit. Then it's easier to scroll around the code and view it in a text editor.
git rebase it to see what files are added/removed
-brady
OpenEMR
I attached the commit in a patch file.
Actually, the patch file is easier to review in this situation(took me about 5 minutes to scroll through it and identify the items that were modified). This is because in github, it is showing a bunch of smarty scripts that were moved and changed(which was not the case). The patch file just shows the smarty files either being all removed or all added (thus makes it easy to identify which scripts were modified by scrolling through it).
Last edit: Brady Miller 2016-03-12
I forgot to add, another thing I do when reviewing a large commit, is listing the files that are involved with the following command:
Last edit: Brady Miller 2016-03-12
Hi,
I just committed the above commit. At this point the development codebase is now PHP7 compliant. There is still 1 related bug that is still in process of being fixed (the address/barcode/patient labels).
This php7 project has been a rather huge undertaking by Practice Provider that has touched on many central parts of the codebase. Please report any noted bugs.
-brady
OpenEMR
Hi,
Just committed fix for the pdf label issue. For now, have brought back the fpdf library to "fix" the labels. At some point, it will make sense to send all pdf output through tcpdf to support internationalization. There has already been some work in sending the labels through tcpdf, however, still lots of testing and configuration to get it right. See the following wiki page for details:
http://www.open-emr.org/wiki/index.php/PDF#Overview
The php7 project is officially complete. Thank you Practice Provider!
-brady
OpenEMR
Great news!
Absolutely Amazing contribution!