ext/gd/config.m4: don't forget GDLIB_CFLAGS in feature tests

[orlitzky]

In commit 85e5635, a feature test for the various libgd image formats was added. That test however erroneously omits the GDLIB_CFLAGS (from pkg-config) during compilation. This can lead to build failures and therefore false negatives from the test.

Here, we add $GDLIB_CFLAGS to $CFLAGS for the duration of the test.

Moreover, we replace $GD_SHARED_LIBADD with $GDLIB_LIBS in the same test. The variable $GD_SHARED_LIBADD is actually empty when we try to use it. Fortunately(?), the library flags are appended elsewhere, so this was not causing a problem. But of course it's better to explicitly do the right thing.

Closes GH-12019

[petk]

Thanks for fixing this one so quick @orlitzky 🎉 This seems to be working fine, yes. Even when doing customization like this:

./configure --enable-gd --with-external-gd \
    GDLIB_CFLAGS=-I/path/to/libgd/src \
    GDLIB_LIBS="-L/path/to/libgd/src/.libs -lgd"

We'll add it to PHP-8.2 branch, since this was added there also.

[petk]

(not related to this PR) Otherwise, in the future, perhaps it would be better to go with a bit different approach in these cases. Because that GD_SHARED_LIBADD is populated when building ext/gd as shared and INCLUDES might include things that are important for the check (for example includes from a separate libavif). I'll see if I can add the PHP_PUSH and PHP_POP macros that manage all these CPPFLAGS, CFLAGS, LIBS, LDFLAGS and INCLUDES like this. And it might be simpler and less error prone to only do this and be confident that flags are saved and restored properly:

PHP_PUSH() dnl store CFLAGS, LIBS, LDFLAGS, INCLUDES
  dnl ... change CFLAGS, LIBS, LDFLAGS, INCLUDES as needed for the next checks
  dnl ... autoconf checks with changed flags 
PHP_POP() dnl Restore CFLAGS, LIBS, LDFLAGS, INCLUDES

[orlitzky]

I force-pushed a fix to overwrite my misleading commit message. I noticed #13727 during testing but it's unrelated.

[petk]

Works great. Thanks @orlitzky

[rs-bc]

This does not change behaviour of the tests at all:

configure:37620: checking for working gdImageCreateFromWebp in libgd
configure:37655: cc -o conftest -g -O2 -fvisibility=hidden -D_GNU_SOURCE -Wl,-
rpath,/usr/local/lib -L/usr/local/lib conftest.c -lutil -lrt -lm -lxml2 -Wl,-rp
ath,/usr/local/lib -L/usr/local/lib -lgd >&5
conftest.c:250:10: fatal error: 'gd.h' file not found
#include <gd.h>
^~~~~~
1 error generated.

There is still missing the include path. Maybe GDLIB_CFLAGS are not yet accessible when running the tests?

[petk]

@rs-bc have you recreated configure script? I've checked this on FreeBSD and this seemed to have found it: ./configure --enable-gd --with-external-gd

The GDLIB_CFLAGS was populated properly but I can recheck what's happening.

[rs-bc]

What do you mean be "recreated configure script"?
I applied the patch https://github.com/php/php-src/commit/3a6353830c400c94868f14e496eadd45fb58951f.patch to the source tree and ran configure like before. Could you check your config.log for occurence of "gdImageCreateFromWebp" and if this test does not fail for you?

[petk]

To recreate configure script this needs to be run ./buildconf.

git checkout PHP-8.2
wget https://github.com/php/php-src/pull/13724.patch
git apply
./buildconf
./configure --enable-gd --with-external-gd
...
checking whether to enable JIS-mapped Japanese font support in GD... no
checking for gdlib >= 2.1.0... yes
checking for working gdImageCreateFromPng in libgd... yes
checking for working gdImageCreateFromAvif in libgd... no
checking for working gdImageCreateFromWebp in libgd... yes
checking for working gdImageCreateFromJpeg in libgd... yes
checking for working gdImageCreateFromXpm in libgd... no
checking for working gdImageCreateFromBmp in libgd... yes
checking for working gdImageCreateFromTga in libgd... yes
checking for gdFontCacheShutdown in -lgd... yes
...

In config.log I see these:

cc -o conftest -g -O2 -fvisibility=hidden -I/usr/local/include -I/usr/local/include/libpng16 -I/usr/local/include/freetype2 -I/usr/local/include/webp   -D_GNU_SOURCE  -Wl,-rpath,/usr/local/lib -L/usr/local/lib conftest.c -lutil -lrt -lm  -lxml2 -lsqlite3 -lxml2 -lgd  >&5

[petk]

And this isn't available in the released PHP-8.2 archive downloadable file, yet. PHP release needs to be done on the next schedule - 8.2.18 in about a week or so.

[rs-bc]

Ah, that did the trick.
After patching, I now did a ./buildconf -f to recreate the configure script forcefully.
Now the tests pass correctly.

Thank you very much.

[petk]

Yes, sorry, ./buildconf -f needs to be done for the released files in your case, yes. Ok, great.

[petk]

Applied via 0079932 to PHP-8.2 and up.