[serf] r1864 committed - On the 1.2.x branch:...

[se...@googlecode.com]

Revision: 1864
Author: lieven....@gmail.com
Date: Mon May 20 06:44:26 2013
Log: On the 1.2.x branch:
Fix issue #95 by applying the patch attached to that issue.
Applied directly to this branch as the autoconf build files were removed
from trunk.

Patch by: Andreas Stieger

* configure.in: Add gssapi option to build Kerberos/Negotiate authn module.

http://code.google.com/p/serf/source/detail?r=1864

Modified:
/branches/1.2.x/configure.in

=======================================
--- /branches/1.2.x/configure.in Sat Dec 8 11:45:28 2012
+++ /branches/1.2.x/configure.in Mon May 20 06:44:26 2013
@@ -194,6 +194,45 @@
AC_CHECK_HEADER([openssl/opensslv.h], [],
[AC_MSG_ERROR([We require OpenSSL; try --with-openssl])])

+dnl Look for Kerberos 5 for GSSAPI
+AC_ARG_WITH(gssapi,
+ APR_HELP_STRING([--with-gssapi=PATH],[build with GSSAPI support; needs
krb5-config in PATH/bin (eg. /usr/lib/mit)]),
+[
+ if test "$withval" = "yes"; then
+ AC_MSG_ERROR([--with-gssapi requires a path])
+ else
+ gssapi_prefix=$withval/
+
+ if test "x$gssapi_prefix" != "x" -a ! -d "$gssapi_prefix"; then
+ AC_MSG_ERROR('--with-gssapi requires a path to a directory')
+ fi
+ AC_MSG_CHECKING([for krb5-config])
+ if test -x "$gssapi_prefix/bin/krb5-config"; then
+ krb5conf=$gssapi_prefix/bin/krb5-config
+ AC_MSG_RESULT([$krb5conf])
+ AC_MSG_CHECKING([for gssapi support in krb5-config])
+ if "$krb5conf" | grep gssapi > /dev/null; then
+ AC_MSG_RESULT([yes])
+ GSSAPI_confopts=gssapi
+ else
+ AC_MSG_RESULT([no])
+ GSSAPI_confopts=
+ fi
+ GSSAPI_cflags=`"$krb5conf" --cflags $GSSAPI_confopts`
+ APR_ADDTO(CFLAGS, "$GSSAPI_cflags")
+ APR_ADDTO(CFLAGS, [-DSERF_HAVE_GSSAPI])
+ APR_ADDTO(CFLAGS, [-g])
+ GSSAPI_libs=`"$krb5conf" --libs $GSSAPI_confopts`
+ APR_ADDTO(LDFLAGS, "$GSSAPI_libs")
+ else
+ AC_MSG_RESULT([no])
+ AC_MSG_WARN([--with-gssapi specified but krb5-config not found])
+ fi
+
+ fi
+])
+
+
dnl CuTest requires libm on Solaris
AC_SEARCH_LIBS(fabs, m)

[Ben Reser]

I've been meaning to look into improving this since the Subversion 1.8.0 release is coming up and as a result Subversion's support for Kerberos will be entirely offloaded to Serf.  So I'm glad to see this commit.  However...

On Monday, May 20, 2013 6:44:36 AM UTC-7, se...@googlecode.com wrote:

Revision: 1864
Author:   lieven....@gmail.com
Date:     Mon May 20 06:44:26 2013
Log:      On the 1.2.x branch:
Fix issue #95 by applying the patch attached to that issue.
Applied directly to this branch as the autoconf build files were removed  
from trunk.

Patch by: Andreas Stieger

* configure.in: Add gssapi option to build Kerberos/Negotiate authn module.

http://code.google.com/p/serf/source/detail?r=1864

[snip]

 

+        if "$krb5conf" | grep gssapi > /dev/null; then
+                AC_MSG_RESULT([yes])
+                GSSAPI_confopts=gssapi
+        else
+                AC_MSG_RESULT([no])
+                GSSAPI_confopts=
+        fi

Ok so I think the above block is trying to avoid the problem on Solaris (and maybe other operating systems) where there is no gssapi option to the krb5-config command.  The problem with this is while it'll get the Kerberos linking arguments it'll not get the GSSAPI library options.  In these circumstances there is no way to discover the correct values for this so you either have to guess or include some way for the user to set them.

Hopefully I'll have a patch forthcoming to deal with this.

+        GSSAPI_cflags=`"$krb5conf" --cflags $GSSAPI_confopts`
+        APR_ADDTO(CFLAGS, "$GSSAPI_cflags")
+        APR_ADDTO(CFLAGS, [-DSERF_HAVE_GSSAPI])
+        APR_ADDTO(CFLAGS, [-g])

Why is this adding -g to the CFLAGS?  I don't see any particular reason why GSSAPI support should require debugging info.

[Ben Reser]

On Mon, May 20, 2013 at 4:18 PM, Ben Reser <b...@reser.org> wrote:
> Hopefully I'll have a patch forthcoming to deal with this.

Patch attached.

Works for me on Solaris and OS X by passing --with-gssapi=/usr to configure.

[[[
Improve gssapi configure support, especially on Solaris where
krb5-config can't tell you where the gssapi library is.

Patch by: Ben Reser

* configure.in: When gssapi option is not supported on krb5-config
guess as to the location of the gssapi library/headers.
Remove the debug flag when building with gssapi.
Convert tabs to spaces and match indentation of the rest of the file.
Test the linkage against the gssapi at configure time.
]]]

[Lieven Govaerts]

Hi,


thanks for the patch.

On Tue, May 21, 2013 at 1:18 AM, Ben Reser <b...@reser.org> wrote:
> I've been meaning to look into improving this since the Subversion 1.8.0
> release is coming up and as a result Subversion's support for Kerberos will
> be entirely offloaded to Serf. So I'm glad to see this commit. However...
>

r1864 brought the 1.2.0 build config up to date with trunk, so that
basically means trunk won't have gssapi support on Solaris either. I
only test on mac, linux & windows, would be nice to have someone build
serf trunk on Solaris from time to time.

So in your patch you take:
> + GSSAPI_cflags="$GSSAPI_cflags -I/usr/include/gssapi"
> + GSSAPI_libs="$GSSAPI_libs -lgss"

If these are Solaris specific, what about including them only on Solaris?

A user can always explicitly set the library to use on other platforms
where krb5-config doesn't not recognise gssapi:
export LDFLAGS="-lgssapi_krb5" ; ./configure

> Hopefully I'll have a patch forthcoming to deal with this.
>
>> + GSSAPI_cflags=`"$krb5conf" --cflags $GSSAPI_confopts`
>> + APR_ADDTO(CFLAGS, "$GSSAPI_cflags")
>> + APR_ADDTO(CFLAGS, [-DSERF_HAVE_GSSAPI])
>> + APR_ADDTO(CFLAGS, [-g])
>
>
> Why is this adding -g to the CFLAGS? I don't see any particular reason why
> GSSAPI support should require debugging info.

No reason, oversight from my part.

Lieven

[Ben Reser]

On Wed, May 22, 2013 at 12:22 PM, Lieven Govaerts
<lieven....@gmail.com> wrote:
> r1864 brought the 1.2.0 build config up to date with trunk, so that
> basically means trunk won't have gssapi support on Solaris either. I
> only test on mac, linux & windows, would be nice to have someone build
> serf trunk on Solaris from time to time.

I realized that. I haven't gotten to looking at how to deal with this
with scons. I'll see what I can do though.

I don't use Solaris musch myself, I just helped a customer get
Kerberos working on Solaris with Subversion 1.7 and realized they'd
have the same problem in 1.8 if I didn't get a fix for them in Serf.

> So in your patch you take:
>> + GSSAPI_cflags="$GSSAPI_cflags -I/usr/include/gssapi"
>> + GSSAPI_libs="$GSSAPI_libs -lgss"
>
> If these are Solaris specific, what about including them only on Solaris?

Ok change this.

> A user can always explicitly set the library to use on other platforms
> where krb5-config doesn't not recognise gssapi:
> export LDFLAGS="-lgssapi_krb5" ; ./configure

True.

[Ivan Zhakov]

On Wed, May 29, 2013 at 8:13 PM, Ben Reser <b...@reser.org> wrote:
> On Wed, May 22, 2013 at 12:22 PM, Lieven Govaerts
> <lieven....@gmail.com> wrote:
>> r1864 brought the 1.2.0 build config up to date with trunk, so that
>> basically means trunk won't have gssapi support on Solaris either. I
>> only test on mac, linux & windows, would be nice to have someone build
>> serf trunk on Solaris from time to time.
>
> I realized that. I haven't gotten to looking at how to deal with this
> with scons. I'll see what I can do though.
>
> I don't use Solaris musch myself, I just helped a customer get
> Kerberos working on Solaris with Subversion 1.7 and realized they'd
> have the same problem in 1.8 if I didn't get a fix for them in Serf.
>

It would be nice if serf build system will be able to find GSSAPI
automatically on popular platforms where it always present. Like Mac
OS X and Solaris. Is it possible? There is no reason to build serf
without Kerberos platform by default IMHO.

--
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com

[Ben Reser]

Improved

On Wed, May 29, 2013 at 9:13 AM, Ben Reser <b...@reser.org> wrote:
> On Wed, May 22, 2013 at 12:22 PM, Lieven Govaerts
> <lieven....@gmail.com> wrote:
>> If these are Solaris specific, what about including them only on Solaris?
>
> Ok change this.

Attached is a version of the patch that only guesses the GSSAPI
library info on Solaris. This is against the current 1.2.x HEAD.

[Greg Stein]

Should CFLAGS/LDFLAGS be restored from their old_* values? Or should
old_* be removed?

> --
> You received this message because you are subscribed to the Google Groups "Serf Development List" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to serf-dev+u...@googlegroups.com.
> To post to this group, send email to serf...@googlegroups.com.
> Visit this group at http://groups.google.com/group/serf-dev?hl=en.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

[Ben Reser]

On Mon, Jun 3, 2013 at 10:43 AM, Greg Stein <gst...@gmail.com> wrote:
> Should CFLAGS/LDFLAGS be restored from their old_* values? Or should
> old_* be removed?

Sorry old is unnecessary. Habit from writing Subversion autoconf
stuff. We're erroring out if the flags don't work so no reason to set
them back.