search

h5p / h5p-php-library

GNU General Public License v3.0
124 stars 128 forks source link

Replace str_contains() call on moodle branch with PHP 7.4-compatible equivalent #194

Open Limekiller opened 2 weeks ago

Limekiller commented 2 weeks ago

Recently a change was made to the library included in the Moodle plugin mod_hvp that introduces a function call to str_contains(), which is only compatible with PHP 8.0 and higher. However, the plugin claims to support Moodle versions that do not support PHP 8.0, as well as Moodle 4.1, which is still actively supported by Moodle and is fully compatible with PHP 7.4.

Moodle sites on the actively-supported Moodle 4.1 that are running on PHP 7.4, or older sites that do not support PHP 8.0 but are claimed to be supported by mod_hvp, will encounter errors until the function call is removed. I have created a pull request to do so here.

cli-ish commented 5 days ago

I have the same issue while upgrading. Hope for a quick merge.

otacke commented 5 days ago

Please note that while older Moodle versions still support PHP 7, PHP 7 reached its end of life in November 2021 and has not received security updates for almost 2 years. So, while I fully understand why people want this to run (and the change is really marginal, of course), I wonder if indirectly supporting to further delay the migration to PHP 8 would be the right signal. (not a member of H5P Group and not expressing their opinions)

relecand commented 5 days ago

Ubuntu (and Debian) continues to provide security updates for PHP 7.4 through its long-term support system, even though official PHP support ended. This LTS ensures that security patches are backported. However, these updates do not include new features or non-essential bug fixes.

https://changelogs.ubuntu.com/changelogs/pool/main/p/php7.4/

otacke commented 5 days ago

That's good for Ubuntu and Debian.

Limekiller commented 5 days ago

I don't actually have an opinion on if H5P should support PHP 7 or not, but that if the decision is made that it no longer should, the plugin needs to be updated to indicate that it no longer supports older Moodle versions. Currently mod_hvp claims it supports every Moodle version from 2.5; requiring PHP 8.0+ would change that to, at most, 4.0 and up--and I would argue, should really only be 4.2, 4.3, and 4.4.

otacke commented 5 days ago

@Limekiller You're right, I know about that compatibility claim. It's up to H5P Group either way. It's just my personal view that I'd rather encourage people to migrate to PHP 8, and I merely wondered what opinion I might face here. And one hint: In my experience, H5P Group does hardly ever react to pull requests unless you create awareness by raising an issue at https://h5ptechnology.atlassian.net/jira/software/c/projects/HFP/issues So, if it's important to you ...