On Sat, 31 Jan 2015 10:55:54 -0800 Bill Spitzak <[email protected]> wrote:
> On 01/31/2015 04:07 AM, [email protected] wrote: > > On 2015-01-31 02:23, Bill Spitzak wrote: > >> On older expat versions there is no pkg-config file, so fall back > >> to the previous test for the header file and library if it fails. > > > > I agree this a good practice to fallback in this case. > > >> This paritally reverts commit > >> a4afd90f9f0c27ed5f3f313b915c260673f8be34. > > >> -wayland_scanner_CFLAGS = $(EXPAT_CFLAGS) $(AM_CFLAGS) > > > > This hunk is bad. If the pkg-config file provide CFLAGS, they must > > be used. And there is no harm at all to use an undefined variable > > here. (And I think we should just define it empty in the fallback.) > > I included that by mistake when I did the revert. Will fix. > > >> + [AC_CHECK_HEADERS(expat.h, [AC_DEFINE(HAVE_EXPAT_H)], > > > > HAVE_EXPAT_H is defined by AC_CHECK_HEADERS itself so please remove > > the AC_DEFINE. > > > > > >> + [AC_MSG_ERROR([Can't find expat.h. Please install > >> expat.])]) > >> + AC_CHECK_LIB(expat, XML_ParserCreate, [EXPAT_LIBS="-lexpat"], > > > > If we re-introduce such check, we should use AC_SEARCH_LIBS (as > > advised by the Autoconf manual[1]). > > Here is the code I would use : > > > > > > EXPAT_CFLAGS="" > > EXPAT_LIBS="-lexpat" > > AC_SEARCH_LIBS([XML_ParserCreate], [expat], > > [ > > AC_SUBST(EXPAT_CFLAGS) > > AC_SUBST(EXPAT_LIBS) > > ], > > [ > > AC_MSG_ERROR([Can't find expat library. Please install expat.]) > > ]) > > I just reverted the previous patch, but if the non-pkg-config test > can be improved it probably is a good idea, I will test this and > submit a new patch. Bryce, I don't know if Bill already sent a new patch, but I think this issue should be dealt with before the 1.7.0 release. If nothing better, just revert the original patch that removed the "awkward" expat test. I think we need to keep the non-pkg-config test or fallback working until we start requiring an expat version that provides the .pc file upstream. Thanks, pq _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
