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
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel