----- Original Message -----
> On 2014-02-05 16:15, Omair Majid wrote:
> > * Andrew Hughes <gnu.and...@redhat.com> [2014-02-04 19:26]:
> >>> On 2014-02-03 18:43, Omair Majid wrote:
> >>>> The following webrevs modify the build system to allow building against
> >>>> the system-installed copy of libpng as well as using the bundled copy of
> >>>> libpng
> >>>>
> >>>> ROOT: http://cr.openjdk.java.net/~omajid/webrevs/system-libpng/01/
> >>>> JDK: http://cr.openjdk.java.net/~omajid/webrevs/system-libpng/01-jdk/
> >>>>
> >>>> A new configure option `--with-libpng` is introduced which defaults to
> >>>> `bundled` but can be set to `system` to use the system-installed copy of
> >>>> libpng.
> >> In this respect, why does this not just use AC_ARG_ENABLE as there are
> >> only
> >> two options here (system libpng or bundled libpng)?
> >>
> >> Also, this patch hardcodes the libpng cflags and ldflags when these should
> >> be obtained from pkg-config. This is how these values are obtained in
> >> IcedTea
> >> and using this patch would thus be a regression.
> > The answer to both of these is the same: I followed how the existing code
> > handles zlib. If this needs fixing, then it needs to be fixed in the
> > other cases (zlib and giflib) too.
>
> I agree with your thinking here. Following the existing patterns is
> good. If they should be extended with using pkg-config (which probably
> is a good idea), then that should extend to all these libraries.
You're already using it:
PKG_CHECK_MODULES([LIBFFI], [libffi])
Why that's in LIB_SETUP_STATIC_LINK_LIBSTDCPP, I have no idea.
>
> As an answer to Andrew as to why --with-libX=system/bundle instead of
> enable/disable:
> * First, the semantics of enable/disable inherited from autoconf is
> that enable/disable is about "features", but with is for external
> "packages". While this difference is not always applicable, I do think
> that external libraries should indeed be specified using --with-X.
> * Second, I believe the original intention was to allow for a third
> option, --with-libX=<dir>, which would point to an location in which the
> library is installed, similar to how e.g. --with-alsa works.
>
Yes, but you don't allow that:
AC_MSG_ERROR([Invalid value for --with-zlib: ${with_zlib}, use 'system' or
'bundled'])
so using --with-x is confusing if someone does specify a directory.
> So, once again, from a build perspective I believe this is a sound patch.
>
> /Magnus
>
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
PGP Key: 248BDC07 (https://keys.indymedia.org/)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F 8F91 3B96 A578 248B DC07
