[Spice-devel] [PATCH spice-common v2 1/2] spice-deps: Make LZ4 check depending on function

[Frediano Ziglio]

> 
> On Thu, 2016-11-24 at 09:33 -0500, Frediano Ziglio wrote:
> > > 
> > > LZ4 changed versioning scheme from r131 to v1.7.3 making our
> > > configure
> > > fail with (1.7.3 < 129).
> > > 
> > > Switch from version checking to checking that the necessary
> > > function
> > > is available.
> > > ---
> > > v2: Added some comments, Switched to AC_CHECK_FUNC
> > > ---
> > >  m4/spice-deps.m4 | 22 ++++++++++++++++++----
> > >  1 file changed, 18 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/m4/spice-deps.m4 b/m4/spice-deps.m4
> > > index adedec4..6827c7f 100644
> > > --- a/m4/spice-deps.m4
> > > +++ b/m4/spice-deps.m4
> > > @@ -185,12 +185,26 @@ AC_DEFUN([SPICE_CHECK_LZ4], [
> > >  
> > >      have_lz4="no"
> > >      if test "x$enable_lz4" != "xno"; then
> > > -      PKG_CHECK_MODULES([LZ4], [liblz4 >= 129], [have_lz4="yes"],
> > > [have_lz4="no"])
> > > +      # LZ4_compress_default is available in liblz4 >= 129,
> > > however liblz
> > > has changed
> > > +      # versioning scheme making the check failing. Rather check
> > > for
> > > function definition
> > > +      PKG_CHECK_MODULES([LZ4], [liblz4], [have_lz4="yes"],
> > > [have_lz4="no"])
> > >  
> > >        if test "x$have_lz4" = "xyes"; then
> > > -        AC_DEFINE(USE_LZ4, [1], [Define to build with lz4
> > > support])
> > > -      elif test "x$enable_lz4" = "xyes"; then
> > > -        AC_MSG_ERROR([lz4 support requested but liblz4 could not
> > > be found])
> > > +        # For cross-compilers may be necessary to save & restore
> > > LIBS and
> > > CFLAGS before AC_SEARCH_LIBS
> > > +        old_LIBS="$LIBS"
> > > +        old_CFLAGS="$CFLAGS"
> > > +        CFLAGS="$CFLAGS $LZ4_CFLAGS"
> > > +        LIBS="$LIBS $LZ4_LIBS"
> > > +
> > > +        AC_CHECK_FUNC([LZ4_compress_default], [
> > > +            AC_DEFINE(USE_LZ4, [1], [Define to build with lz4
> > > support])],
> > > +            [have_lz4="no"])
> > > +
> > > +        LIBS="$old_LIBS"
> > > +        CFLAGS="$old_CFLAGS"
> > > +      fi
> > > +      if test "x$enable_lz4" = "xyes" && test "x$have_lz4" =
> > > "xno"; then
> > 
> > I think this xyes/xno it's kind of a fix for shell bug quite long
> > ago, a
> > 
> >    if test "$enable_lz4" = yes -a "$have_lz4" = no; then
> > 
> > will work. Even [ is more readable leading to
> > 
> >    if [ "$enable_lz4" = yes -a "$have_lz4" = no ]; then
> > 
> > configure try to use very old style to make sure compatibility is
> > good
> > but this lead to really ugly syntax sometimes.
> 
> it is a style used in all our configure.ac and m4 files. It may be
> ugly/not easy to follow but I don't think it is worth spending time on
> improving it everywhere...
> 

I agree but usually when you decide to change a style you
define the new style and instead of changing everything
you change using the new style when you change the code.

By the way, not this not that important, patch is fine
even with this style. Perhaps the -a usage is compatible
with current style and more readable than test ... && test ...

> 
> > 
> > > +        AC_MSG_ERROR([lz4 support requested but liblz4 >= 129
> > > could not be
> > > found])
> > >        fi
> > >      fi
> > >      AM_CONDITIONAL(HAVE_LZ4, test "x$have_lz4" = "xyes")
> > 

Can you send an updated patch with everything? I'm a bit lost.

Frediano