spkg-configure.m4 for libgd

[dimpase]

use pkg-config to get it.

configure tarball is here:
http://users.ox.ac.uk/~coml0531/sage/configure-2bd1bda5f85b8f8e4a7fc7499f8529795099160a.tar.gz

CC: @orlitzky @jdemeyer @kiwifb

Component: build: configure

Author: Dima Pasechnik

Branch/Commit: 8c97d9b

Reviewer: François Bissey, Erik Bray

Issue created by migration from https://trac.sagemath.org/ticket/27825

[dimpase]

comment:1

I'm trying

SAGE_SPKG_CONFIGURE([libgd], [
    AC_REQUIRE([SAGE_SPKG_CONFIGURE_FREETYPE])
    AC_MSG_CHECKING([Installing freetype? ])
    if test x$sage_spkg_install_freetype= xyes; then
      AC_MSG_RESULT([Yes. Install libgd as well.])
      sage_spkg_install_libgd=yes
    else
      AC_MSG_RESULT([No.])
      PKG_CHECK_MODULES([LIBGD], [gdlib >= 2.1], [], [sage_spkg_install_libgd=yes])
    fi
])

to see whether this produces a working Sage (on my system libgd is not linked to iconv, according to ldd).

[orlitzky]

comment:2

(Wall-of-text alert. There's a tl;dr at the end, but I've dumped my reasoning for review so that we don't accidentally break some case I haven't thought of.)

For posterity, we discussed the iconv dependency on the mailing list.

The libgd package has an automagic dependency on iconv, meaning that it has optional support for iconv that is enabled automatically if a suitable iconv is present at build time. At that point, iconv is no longer optional, because libgd will be compiled to call some iconv functions, and will potentially be linked to libiconv.

Moreover, there are a few ways that "libiconv" can be implemented. On modern linux systems, the calls are all part of glibc, which is why you don't see libgd being explicitly linked against iconv on those systems. However, on other operating systems, GNU libiconv is indeed installed as a separate package, and you will find libgd linked against it on those systems.

Now, I don't believe that SageMath uses any of the iconv support in libgd. The only SageMath file to call a GD function is matrix/matrix_mod2_dense.pyx (someone confirm this?), and it doesn't pass any strings to the API. The only place iconv is used in libgd is in src/gdkanji.c, and it's used for manipulate string encodings. So, SageMath shouldn't need it.

As a result, I don't think iconv should be a dependency of the SageMath libgd package, but there are some conflicting goals here that I'll catalogue:

1. We'd like the libgd built by sage to be consistent across platforms. That is, it should either have iconv support or not, regardless of the OS.
2. We'd like the detected system libgd to be as consistent as possible with the one that sage builds; however...
3. We don't want to force the system libgd to be built with a specific configuration with regards to iconv, because that will prevent the system package from being used in a lot of cases where it otherwise could.

There's no perfect solution for all three, and in my opinion the third is the most important, which forces us to choose between (1) and (2). The two associated plans of action are,

1. Leave the iconv dependency in libgd, and accept a system libgd with no such support.
2. Drop the iconv dependency in libgd, and take whatever happens.

The second option means that the libgd built by sage won't be consistent, but it does bring the sage package to parity with the system package check. And, it's simpler. So finally,

tl;dr I think the practical solution is to

• Remove iconv from libgd's dependencies
• Remove iconv from libgd's SPKG.txt
• Write spkg-configure.m4 without mention of iconv (like Dima just did).

[orlitzky]

comment:3

Replying to @dimpase:

    AC_MSG_CHECKING([Installing freetype? ])

libpng too?

[dimpase]

comment:4

yes, sure, I didn't spell out all the dependencies yet.

[dimpase]

comment:5

No need to explicitly check for libpng being from the system, as this is done by the freetype macro run.

[dimpase]

Branch: u/dimpase/packages/libgd-config

[dimpase]

Commit: 0635672

[dimpase]

Author: Dima Pasechnik

[dimpase]

Dependencies: #27168

[orlitzky]

comment:6

Replying to @dimpase:

No need to explicitly check for libpng being from the system, as this is done by the freetype macro run.

We should still check it ourselves, for the same reason we need the redundant libpng in libgd's dependencies: if freetype ever drops the libpng dependency, someone would be within his rights to remove that check from the freetype macro, and would have no idea that it's going to break libgd.

That breakage will manifest on some weird system a year later, and it will take us a lot more time to track it down then than it will to play it safe now.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

33a1441

check for libpng explicitly

[sagetrac-git]

Changed commit from 0635672 to 33a1441

[dimpase]

comment:8

Replying to @orlitzky:

Replying to @dimpase:

No need to explicitly check for libpng being from the system, as this is done by the freetype macro run.

We should still check it ourselves, for the same reason we need the redundant libpng in libgd's dependencies: if freetype ever drops the libpng dependency, someone would be within his rights to remove that check from the freetype macro, and would have no idea that it's going to break libgd.

OK, fixed.

[kiwifb]

comment:10

Hum... Feels circular at first sight (libpng <-> zlib). Also libgd actually picks many other formats- it is one of its point. Is libpng really the only one we care about? jpeg or anything else?

[dimpase]

comment:11

this is what we build in Sage, just look at its libgd/dependencies.

[kiwifb]

Reviewer: François Bissey

[kiwifb]

comment:12

Replying to @dimpase:

this is what we build in Sage, just look at its libgd/dependencies.

Fair enough. The chain of various checks between libpng, zlib and freetype is hurting my head. I am glad we can just abstract it and let autoconf deal with it until it complains. If it can digest it and produce a working configure script that's already a strong test.

[dimpase]

comment:13

Thanks, but please review its dependencies too!

[kiwifb]

comment:14

I see, it is part of the same bundle.

[dimpase]

comment:15

To release manager - this might need a configure bump.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

2a9d203

configure bump

[sagetrac-git]

Changed commit from 33a1441 to 2a9d203

[dimpase]

comment:36

rebased over 8.8.rc0; no configure bump done

[embray]

comment:37

I still have some nitpicks about the AC_MSG_CHECKING message formatting being inconsistent in some cases. But I'd rather just get this done for now, and apply my nitpicks in the next round of configure updates.

[embray]

comment:38

This should obviously be postponed until the next release (hopefully early on).

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

e84c459

(re)added missing space in if

[sagetrac-git]

Changed commit from a957fde to e84c459

[dimpase]

Changed reviewer from François Bissey to François Bissey, Erik Bray

[dimpase]

comment:40

I guess this has fallen through rebasing cracks...

[sagetrac-git]

Changed commit from e84c459 to 8c97d9b

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. Last 10 new commits:

57c6e5c	a straightforward version with pkg-config only
d07d326	add SAGE_FREETYPE_PREFIX for libgd
2b9d77c	spkg-configure for libgd
113830d	remove dependence on iconv, deemed unneeded on #27825
da9af50	check for libpng explicitly
e3a4f75	For consistency's sake, set SAGE_FREETYPE_PREFIX to empty when using the system lib
6c88682	As we are updating libgd, go ahead and add an spkg-legacy-uninstall for removing old versions
4cbd9a6	Update these messages to match stylistically with the similar messages
2bd1bda	(re)added missing space in if
8c97d9b	configure bump

[dimpase]

comment:42

rebased over latest beta

[dimpase]

comment:43

removed "wont fix" deps from the description

[dimpase]

Changed dependencies from #27168, #27277 to none

[vbraun]

Changed branch from u/dimpase/packages/libgd-config to 8c97d9b