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

[Christophe Fergeau]

On Thu, Nov 24, 2016 at 10:43:50AM -0500, Frediano Ziglio wrote:
> > 
> > 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 ...

-a has portability issues iirc. Does not hurt to stick with the current
test ... && test ... in my opinion.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20161129/24d52d3b/attachment.sig>