Use uppercase format string for GMagick

[ausi]

This prevents the GMagick error No encode delegate for this image format

See #732

[ausi]

The failing test on Travis doesn’t seem to be related to this pull request.

[ausi]

@mlocati it would be great if this could be merged ☺️

[leofeyer]

I second this, because we are also having issues with this bug (see contao/image#67).

[mlocati]

We don't need to second a change, we need a test case that shows the problem.

The correct way to submit a pull request should be:

1. create a pull request with a commit that makes test fail because of a bug
2. wait for TravisCI tests to fail
3. add to the same pull request a second commit that fixes the bug
4. tests should now succeed

I just created #737 that tests saving images with different cases of the format parameter (see here), and the tests are passing for all the 3 drivers GD, GMagick and Imagick (see here).

So, how can we replicate the problem that this pull request is addressing?

[leofeyer]

@mlocati No hard feelings, but I don't understand your reaction. @ausi has explained the issue very detailed in #732 and you have explicitly asked him to create a PR:

In the original issue, @ausi states that the problem is not reproducible on every system. Apparently, it does not occur on Travis. 🤷‍♂

It still does occur on @ausi's system and on our servers. And the GraphicsMagick documentation, which @ausi has also linked in #732, says that the "magick string" is an uppercase string. Isn't that reason enough to merge such a little change?

I understand that unit tests are important to prevent regression bugs. But IMHO it is not worth the effort to fiddle with Travis until we have a setup in which we can reproduce this, just so we can add a simple strtoupper() in a place where – at least according to the docs – it should have been an upper-case string from the beginning.

Again, no hard feelings. 🙈

[mlocati]

No hard feelings, but I don't understand your reaction.

I didn't meant to be rude, of course. Maybe using one of those great emoticons would have made my reaction much more polite 😉

In the documentation we don't have a clear note about upper/lower case, only an example with 'GIF'.

Isn't that reason enough to merge such a little change?

Well, are you sure that other systems would not break if we use JPEG instead of jpeg?

I'm going to check the GraphicsMagic and GMagick source code. Maybe something changed betweem the GraphicsMagick version 1.3.23 (used in TravisCI) and 1.4 (as reported in #732)

[leofeyer]

Well, are you sure that other systems would not break if we use JPEG instead of jpeg?

Well, no. 😞 Good point.

[mlocati]

The setimageformat function of the GMagick PHP extension is a bare wrapper around the MagickSetImageFormat function of the GraphicsMagick library.
This MagickSetImageFormat function simply set the magick property of the GraphicsMagick Image object.
That magick property is then used to determine the image info, using the GetMagickInfo function. That function calls GetMagickInfoEntryLocked, which uses LocaleCompare to check the format, which performs a case-insensitive match.
So, I really don't understand why or when the issue may occur...

[ausi]

@mlocati in d8bc9bf I created a reproduction of the issue, see Travis build 1436.

In f71dff2 the issue gets fixed, see Travis build 1438.

(The build itself fails but the tests pass).

[mlocati]

Superseded by #750