Skip to contentSkip to conversation

lz4: Fix confused version check

PATCH In review David Sommerseth 2 participants 2 messages Oct 2, 2017

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 lz4: Fix confused version check

Older LZ4 library versions used a version number > 100 and not the current x.y.z versioning scheme. This results in version 122 being numberically higher than the check we have liblz4 > 1.7.1. And since that old version (122) does not have the LZ4_compress_default(), the building explodes later on.

This patch enhances the version check to also ensure the version number is lower than 100. In addition the function checking we had was not triggered if system library was found via pkg-config, so this have now been reworked to really check if we have at least two of the most important LZ4 functions - as long as a system library have been found or been accepted via the LZ4_{CFLAGS,LIBS} variables.

There are more ways to check for functions in autoconf. I opted for AC_CHECK_LIB() instead of AC_CHECK_FUNC{,S}() as the latter ones does not test if a function exists in a specific library. This have the downside of needing to tests instead of AC_CHECK_FUNCS() which could test for more functions in one go. We also do not overwrite the LZ4_LIBS variable on success, as that could change already set library paths (-L)

Finally, a stupid typo got fixed as well.

Trac: 939

---
 configure.ac | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
configure.ac +10 -6
index 74443353..394f5b09 100644
@@ -1091,7 +1091,7 @@ if test "$enable_lz4" = "yes" && test "$enable_comp_stub" = "no"; then
     if test -z "${LZ4_CFLAGS}" -a -z "${LZ4_LIBS}"; then
 	# if the user did not explicitly specify flags, try to autodetect
 	PKG_CHECK_MODULES([LZ4],
-			  [liblz4 >= 1.7.1],
+			  [liblz4 >= 1.7.1 liblz4 < 100],
 			  [have_lz4="yes"],
 			  [] # If this fails, we will do another test next
 	)
@@ -1131,16 +1131,20 @@ if test "$enable_lz4" = "yes" && test "$enable_comp_stub" = "no"; then
 	fi
     fi
 
-    # if LZ4_LIBS is set, we assume it will work, otherwise test
-    if test -z "${LZ4_LIBS}"; then
+    # Double check we have a few needed functions
+    if test "${have_lz4}" = "yes" ; then
 	AC_CHECK_LIB([lz4],
-		     [LZ4_compress],
-		     [LZ4_LIBS="-llz4"],
+		     [LZ4_compress_default],
+		     [],
+		     [have_lz4="no"])
+	AC_CHECK_LIB([lz4],
+		     [LZ4_decompress_safe],
+		     [],
 		     [have_lz4="no"])
     fi
 
     if test "${have_lz4}" != "yes" ; then
-	AC_MSG_RESULT([		usuable LZ4 library or header not found, using version in src/compat/compat-lz4.*])
+	AC_MSG_RESULT([		usable LZ4 library or header not found, using version in src/compat/compat-lz4.*])
 	AC_DEFINE([NEED_COMPAT_LZ4], [1], [use copy of LZ4 source in compat/])
 	LZ4_LIBS=""
     fi
Signature · 1 line
2.13.5
#1 lore raw

ACK. Makes sense, got tested on our problem child (Debian 8) already, and I added more tests of my own (Ubuntu 14.04 with "too old lz4 library", FreeBSD with 1.8.0, Gentoo with "no lz4 library") and everything worked nicely. thanks.

Your patch has been applied to the master and release/2.4 branch.

commit f91e4863bc138213a07a2cf53ad71d8a4532abef (master) commit c8a7cc646571165101a43b02a3f0676661d4d011 (release/2.4)

Author: David Sommerseth
Date:   Mon Oct 2 18:18:12 2017 +0200
     lz4: Fix confused version check

     Signed-off-by: David Sommerseth <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/search?l=mid&[email protected]...
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering