[Spice-devel] [PATCH spice-common] Make LZ4 dependency check more robust

[Eduardo Lima (Etrunko)]

On 06/16/2016 10:11 AM, Christophe Fergeau wrote:
> On Thu, Jun 16, 2016 at 10:02:24AM -0300, Eduardo Lima (Etrunko) wrote:
>> On 06/16/2016 07:03 AM, Christophe Fergeau wrote:
>>> On Wed, Jun 15, 2016 at 04:35:14PM -0300, Eduardo Lima (Etrunko) wrote:
>>>> Add a new 'have_lz4' variable to really tell if we have the dependency
>>>
>>> "Add a new 'HAVE_LZ4' automake conditional"
>>>
>>>> installed on the system. It will later be used in Makefile to decide
>>>> whether or not the specific files related to LZ4 should be built.
>>>>
>>>> Signed-off-by: Eduardo Lima (Etrunko) <etrunko at redhat.com>
>>>> ---
>>>>  m4/spice-deps.m4 | 12 ++++++++----
>>>>  1 file changed, 8 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/m4/spice-deps.m4 b/m4/spice-deps.m4
>>>> index 2e2fcf5..a114e4f 100644
>>>> --- a/m4/spice-deps.m4
>>>> +++ b/m4/spice-deps.m4
>>>> @@ -185,14 +185,18 @@ AC_DEFUN([SPICE_CHECK_LZ4], [
>>>>  
>>>>      if test "x$enable_lz4" != "xno"; then
>>>>        PKG_CHECK_MODULES([LZ4], [liblz4],
>>>> -        [enable_lz4=yes
>>>> +        [have_lz4="yes"
>>>>           AC_DEFINE(USE_LZ4, [1], [Define to build with lz4 support])
>>>>          ],
>>>> -        [if test "x$enable_lz4" = "xyes"; then
>>>> -          AC_MSG_ERROR([lz4 support requested but liblz4 could not be found])
>>>> -        fi]
>>>> +        [have_lz4="no"
>>>> +         if test "x$enable_lz4" = "xyes"; then
>>>> +            AC_MSG_ERROR([lz4 support requested but liblz4 could not be found])
>>>> +         fi]
>>>>        )
>>>> +    else
>>>> +        have_lz4=no
>>>>      fi
>>>> +    AM_CONDITIONAL(HAVE_LZ4, test "x$have_lz4" = "xyes")
>>>>  ])
>>>
>>> I think this whole change could just be:
>>>
>>> +   AM_CONDITIONAL(HAVE_LZ4, test "x$enable_lz4" = "xyes")
>>>
>>> If you prefer to go with an additional have_lz4, I'd prefer to make it
>>> closer to the SPICE_CHECK_SMARTCARD check for consistency reasons
>>> (ie don't nest AC_DEFINE/AC_MSG_ERROR in PKG_CHECK_MODULES() but only
>>> set have_lz4 in PKG_CHECK_MODULES() and do the rest after it).
>>
>> I prefer it this way too. I will follow the smartcard check.
> 
> One big advantage of making it one line is that you then don't need any
> changes to spice-gtk configure.ac output, avoiding Pavel's concerns.
> 

It would be indeed a big advantage if we did not need it for both
spice-gtk and spice server, but spice-gtk can live without that change
for a while.

By the way, what is the policy of updating the submodule? I thought
whenever we updated it for one repository it should also be updated for
the other one?

> Christophe
> 


-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etrunko at redhat.com