CORE-POS / IS4C

Cooperative Operational Retail Environment
http://www.core-pos.com
GNU General Public License v2.0
63 stars 44 forks source link

PHP 8.x support #1204

Open lgedgar opened 11 months ago

lgedgar commented 11 months ago

Was curious about support for newer PHP versions. Their release / EOL schedule (https://endoflife.date/php) seems a tad aggressive given the slower-moving nature of our CORE upgrades. Which in practice may encourage everyone to stick with 7.4 but surely that's not great in the long run.

I don't know what 8.x brings that might be a "problem" for CORE per se. I see a reference in a recent issue here which seems benign but ultimately 7.4 was used IIUC. I did a very basic test locally where I loaded the main page in CORE Office (item editor) and looked at the logs, using PHP 7.4, 8.0, 8.1, 8.2 - only the 8.2 gave (deprecation) warnings, but plenty of them re: the adodb driver.

So deprecation warnings aren't terrible except they do point to the need for some code changes. And that's just the first page, have no idea how extensive changes might need to be. Nor do I know if the adodb driver should be "edited" or if a newer version can be obtained (IIUC this is a "vendor-ed" lib so to speak?).

Anyway wanted to get some notion of your thoughts before deciding on how to tackle installs we maintain. (What version do you use?)

gohanman commented 11 months ago

The deprecations are really the easy part. The piece that's tricky is that in PHP < 8

I haven't found any good static analysis tools for identifying where this might be an issue, but the fact that the configuration wrappers and GET/POST value wrappers return empty string when a value isn't defined means there's probably quite a few of them to find and fix.

adodb is editable. Ages ago I tried getting to compatibility with the upstream project but ran into a brick wall trying to get PRs approved and we've been shipping a custom patched version ever since.

I'm running 7.4 as well at the moment for Office. I could move to something in the 8.x line, but again I think that's the easier part. Office pages are pretty close to stateless. The Lane side has a lot more in the way of application state that's checked across various places and can at times include empty strings.

lgedgar commented 11 months ago

Okay, dang. Well at least I'll try to get our demo install(s) on 8.x at some point, see what happens. Thanks for the info.

lgedgar commented 2 months ago

I just tried to use php 8.1 on a demo server and while I did see some warnings logged from the ADODB driver, the bigger problem was that some composer package dependencies still require php < 8. So if we were to seriously tackle this, IUC we would need to update those dependencies as well. In which case when we merge the effort into upstream master, it's possible CORE would then need to require php 8+

In the long run requiring "recent" php seems pretty normal/sane but wondering what you think about this. Is it worth the effort etc.? Would it require simultaneous effort on both Lane and Office, or could we somehow stick to just one (probably Office) for the first phase?