-
-
Notifications
You must be signed in to change notification settings - Fork 150
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
Fix removed "php_strtolower" for PHP 8.4 #690
base: develop
Are you sure you want to change the base?
Conversation
@Danack can we get this merged? |
I checked the patch and it is working. One thing, though: I still use the same code to update PHP 5.3 (yeah, I know), so I had a #ifdef #else #endif construction anyway. |
Based on https://github.com/Imagick/imagick/blob/3.7.0/package.xml#L8 PHP 5.3 (and lower) is no longer supported. |
Fixes phpv8#540 Relates Imagick/imagick#690
Could we have this merged pretty please? It hit a popular repo in Ubuntu/Debian and now we can't use imagick. Than you for your work folks.
|
Same here :-) php8.4 -v |
WordPress uses imagick and certain functions are now broken in 8.4 |
https://github.com/mvorisek/image-php Docker images can be used which contain most of the extensions, incl. this one and the fix, prebuilt. |
We don't use WordPress in a docker image nor do we currently plan to expend the effort to migrate. We've rolled back to 8.3 for now, but I thought it would be worth mentioning that WordPress is also currently broken with regards to imagemagick on 8.4 |
Do we have to go back to 8.3, or will this get merged soon? |
First, just add the modules, copying from 8.3 versions. $ for p in php-8.3-*.yaml; do sed -e 's,epoch: .*,epoch: 0,' \ -e 's,name: php-8.3-,name: php-8.4-,' \ "$p" > php-8.4${p#php-8.3}; done Then fix some to build with 8.4, cherry picking from upstream. * php-8.4-pecl-mcrypt - cherry-pick a PR to build for 8.4 Grab php/pecl-encryption-mcrypt#19 which just uses php_mt_rand_range rather than now-removed php_rand. * php-8.4-imagick - cherry pick pr to build for 8.4 Grab Imagick/imagick#690 to replace php_strlower with zend_str_tolower * php-8.4-xdebug - cherry pick several upstream changes NOTE: I have very low confidence in this, other than it builds. xdebug is active and likely to release a 3.4.0 release soon that has php 8.4 support properly. By adding php-8.4-xdebug with updates enabled, we will get a PR automatically made and hopefully landed just by dropping the cherry-picks here. * Drop php-8.4-swoole - not supported upstream yet. Several changes would be needed to make the 5.1.5 version of swoole build with php 8.4. Upstream explicitly removed support from the 5.1 branch swoole/swoole-src#5525 There is likely 6.0.0 release coming soon (beta released 2024-10-17). We will have to manually re-add at that point in time. * php-8.4-memcached and php-8.4-redis are separated out to another PR as they are dependent on php-8.4-igbinary output.
Please merge this pull request ! |
@Danack any update on this? Sorry to rush but it's a major roadblock for many projects. |
when will be this merged ? my automated script need it :-) |
As a workaround until this is merged, add a macro to your configure command like so: ./configure CPPFLAGS='-Dphp_strtolower=zend_str_tolower' |
Or patch the sources with the patch from the commit: https://github.com/Imagick/imagick/commit/65e27f2bc02e7e8f1bf64e26e359e42a1331fca1.patch |
If anyone wondering why stuff like this doesn't get merged - it's an idealogical objection to the way current open source is run and funded: https://x.com/MrDanack/status/1469772981840891909 This is one of those projects at the bottom leg of the XKCD comic. Still, hope it get's merged soon for the sake of the 99.9% of devs who have zero influence over their company open source contribution policies. |
OK so what's the magic number to get a few hours of time to help review pending PRs and get a new release with full PHP 8.4 support published? Because at this point it's just easier for me to hit the sponsor button and help the ecosystem than to sit around waiting for companies to stop worrying about their profit margins and do nothing to support the toolchain they consume for free. |
There probably isn't one. It's likely just going to get ignored. If this is indeed a protest, it's never going to harm the audience it's intended to. Enterprise environments probably never updated past PHP7 and they're hardly going to essentially pay to merge a PR to restore functionality on a newer PHP version. |
With all respect to the maintainers views, it's absolutely crazy how a vital extension as @Danack Can you please merge this PR for the many people that depend on it? |
@cmb69 Can you do anything to get things in motion once again? |
I'm afraid I can't. I'm neither member of this organization/repository, nor did I have any contact with Dan for a long time. I hope he's doing well. The only option I'd see would be to create a fork under the php organization, but that would need to be discussed on the internals mailing list, and might require an RFC. And then there would still be the problem that the php organization is seriously understaffed for the rather huge amount of code and infrastructure that needs to be maintained. Given https://externals.io/message/123594, maybe @mvorisek wants to create a fork of this repo, where some rudimentary maintenance can happen (not too much, so it might later be backported to this repo). It is conceivable to (temporarily) switch PECL to use that fork, so new releases could be rolled out (would need discussion on internals or the pecl-dev mailing list, though). |
Thank you Christoph for the offer to maintain a fork but I have to be honest, Any project I contribute to or even maintain takes considerable amout of time and I have to balance my resources between my open souce interests, financial goals and my family. As this PR has shown quite large interest my question is if there is someone with a corporate budget to invest into the maintenance with a long term vision. If yes, I would be happy to work on the maintenance and invite more people in. My email is easily accesible on my commits. And yes, forking this repo under php GH organization has my support. Like https://github.com/php/pecl-database-pdo_oci for example. |
Please don't read this as being rude or ungrateful, that's not my goal. These are also my personal opinions and shouldn't be read as reflective of any company I may represent Most people with that sort of budget are part of mega corporations. The inherent problem there is most mega orgs aren't going to be happy to essentially sponsor what is, to them, a hobbyist project, because pouring money into that comes with a non insignificant risk of no returns and/or project abandonment. And therein lies the problem: it really doesn't make business sense to put money into something that is under no obligation to continue to provide you with that service. So instead, those corporations either a) stay on old PHP versions where this works, b) fork it internally for internal usage only, or c) switch to something with more guarantees. I 100% agree you're under no obligation to provide updates, but that's also the reason corporations won't invest in this project. I don't disagree the current open source ecosystem in general needs a reckoning when it comes to funding what we take for granted, because I too have felt the pain of trying to draw money from the masses for a free project that requires significant time investment. But I also understand why a business with that sort of investment capital may be hesitant to invest it here, or in similar projects. |
fixes #689
see https://github.com/php/php-src/pull/15391/files