[Spice-devel] [PATCH] m4: Add macro for --with-sasl

[Christophe Fergeau]

Hey,

On Tue, Nov 24, 2015 at 11:38:26AM +0100, Pavel Grunt wrote:
> Imported from spice-gtk e3c455b86dba0f924165fe0dc51583f48aecc8fb.
> It is not used by spice-common, but both server and client can use it.

Couple of comments, most of them are more things which could be added on
top of this change

> ---
>  m4/spice-deps.m4 | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/m4/spice-deps.m4 b/m4/spice-deps.m4
> index cb2b4c0..3579db5 100644
> --- a/m4/spice-deps.m4
> +++ b/m4/spice-deps.m4
> @@ -263,3 +263,56 @@ AS_IF([test "x$1" != x],
>               [missing_gstreamer_elements="no"])
>        ])
>  ])
> +
> +# SPICE_CHECK_SASL
> +# ----------------
> +# Adds a --with-sasl switch to allow using SASL for authentication.
> +# Checks whether the required library is available. If it is present,
> +# it will return the flags to use in SASL_CFLAGS and SASL_LIBS variables,
> +# and it will define a HAVE_SASL preprocessor symbol.

And a HAVE_SASL conditional

> +# ----------------
> +AC_DEFUN([SPICE_CHECK_SASL], [
> +    AC_ARG_WITH([sasl],
> +      [AS_HELP_STRING([--with-sasl=@<:@yes/no/auto@:>@],
> +                      [use cyrus SASL for authentication @<:@default=auto@:>@])],
> +                      [],
> +                      [with_sasl="auto"])
> +
> +    SASL_CFLAGS=
> +    SASL_LIBS=
> +    enable_sasl=no
> +    if test "x$with_sasl" != "xno"; then
> +      if test "x$with_sasl" != "xyes" && test "x$with_sasl" != "xauto"; then
> +        SASL_CFLAGS="-I$with_sasl"
> +        SASL_LIBS="-L$with_sasl"

This block seems to be meant to pass --with-sasl=/prefix/to/libsasl ,
which would make the help string incorrect.


> +      fi
> +      old_cflags="$CFLAGS"
> +      old_libs="$LIBS"
> +      CFLAGS="$CFLAGS $SASL_CFLAGS"
> +      LIBS="$LIBS $SASL_LIBS"
> +      AC_CHECK_HEADER([sasl/sasl.h],[with_sasl=yes])

The spice-server code had a bit more sophistication which caused
configure to fail if --with-sasl is passed and the header is not found
(I changed sasl/sasl.h to something non-existing in order to test that).
However, if autodetection is used, the failed detection of the header is
ignored, and if the library is found, then sasl will be enabled.

> +      if test "x$with_sasl" = "xyes" ; then
> +        AC_CHECK_LIB([sasl2], [sasl_client_init],[with_sasl2=yes],[with_sasl2=no])
> +      fi
> +      if test "x$with_sasl2" = "xno" && test "x$with_sasl" = "xyes" ; then
> +        AC_CHECK_LIB([sasl], [sasl_client_init],[with_sasl=yes],[with_sasl=no])
> +      fi

I'd say it's time to drop support for -lsasl which corresponds to
cyrus-sasl 1.5 versions, 1.5.28 was released in 2002 (and it seems to be
the most recent release).

One additional remark is that cyrus-sasl ships a .pc file since its most
recent release (2.1.26), which was released 3 years ago. This release is
available in debian stable, supported fedoras and el7. el6 does not have
this .pc file though. I'd still switch to it and drop this big block of
detection code.

This would make it easier to get the 'usual' behaviour:
- configure autodetects when nothing is specified
- configure fails if --with-sasl is specified but cyrus-sasl cannot be
  detected
- configure does not try to detect cyrus-sasl if --without-sasl is used.

The spice-deps.m4 macro you introduced could use the .pc file from the
start I think, do you mind updating this patch to do that?
(alternatively I can do it, using one of the other checks in this file
for inspiration ;)

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20151125/1ba5a9e7/attachment.sig>