Skip PNG tests if the system libgd is missing PNG support #13040
Conversation
Three of our gd tests could be skipped with a message about requiring bundled GD, but those tests don't actually require bundled GD. We update the messages to mention the specific functions that are required.
The bundled libgd always has PNG support, but an external one may not.
ext/gd/tests/bug39366.phpt
Outdated
| if (!function_exists("imagerotate")) die("skip requires bundled GD library\n"); | ||
| if (!function_exists("imagerotate")) die("skip requires imagerotate function"); |
There was a problem hiding this comment.
Does it just require this function?
There was a problem hiding this comment.
Well, obviously not. But instead of adding more checks, maybe we need fewer. Can this fail at all?
My inspiration for skip requires imagerotate function was a combination of the test description and some other tests that do the same thing. But I think these may all date back to a time when the bundled libgd had gdImageRotateInterpolated() but the external libgd might not have. These days both should have it (it's ten years old, and not guarded by an #ifdef), so the function_exists() check is probably pointless. None of the other image* functions are #ifdefed either.
I can remove the check if you think that's best.
There was a problem hiding this comment.
Removing them is probably best then yes.
There was a problem hiding this comment.
Easy enough. I tracked down the exact commit where the #ifdef was removed for imagerotate(), it was even older than I thought.
Following 59ec80c, the imagerotate() function is always available. We may therefore remove its function_exists() checks without harm.
|
Thank you ! |
|
Any chance this will make it into 8.3.x? Otherwise our tests can fail during (and preventing) installation. I can patch in the meantime but it hits a lot of files. |
|
Sure we can backport the fixes, I'll see if I can get round to it, is this also an issue on 8.2? |
Yes, technically, but I'm only switching to the system libgd for our 8.3 packages, so probably no one else will notice. |
|
png was set as a minimal requirement within php.
Other formats also need an external lib, less common ones. gd2 formats is
not suitable :)
…On Wed, Jul 10, 2024, 9:02 PM Michael Orlitzky ***@***.***> wrote:
Sure we can backport the fixes, I'll see if I can get round to it, is this
also an issue on 8.2?
Yes, technically, but I'm only switching to the system libgd for our 8.3
packages, so probably no one else will notice.
—
Reply to this email directly, view it on GitHub
<#13040 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACE6KAYGLCPT5XZLUVMZ43ZLU5FPAVCNFSM6AAAAABKT2GBJWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRQGU4TEOJRG4>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
ping :) I see that #14905 made it into 8.3.10 but the others are still failing. |
|
Same thing in 8.3.11, can this pretty please be merged for 8.3.x? |
Now that the system image formats are being detected properly (85e5635a), there are several PNG tests that need to be skipped if the system libgd is missing PNG support. Nothing exciting here I hope.