Hi, On Wed, 2015-11-25 at 14:58 +0100, Christophe Fergeau wrote: > 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. Ok, everything will be easier > > 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? I will update it, Thanks, Pavel > (alternatively I can do it, using one of the other checks in this file > for inspiration ;) > > Christophe _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel