Re: Patch review completed.

On Fri 2002-10-25 at 15:49:10 -0500, Karl Fogel wrote:
> Okay. As far as I know, all pending patches are either applied,
> tossed because no longer applicable, or filed in the issue tracker
> (either as a new issue of type "PATCH", or attached to an existing
> issue).

The patch in the attached mail is still open from my point of
view. There was no feedback (at all) on it, which may be the reason
why you skipped it in your patch review. At least I did not find it in
the issue tracker and it is not applied as of r3576.

So, any feedback on the issue would be welcome. Btw, the patch was
against r3221, but still works for r3576.

Regards,

        Benjamin.

attached mail follows:

Hello.

First svn patch, so please bear with me. ;-)

I tried to follow all the recommendations from HACKING and so on, but
cannot guarantee that I catched everything. Well, I am sure, you will
tell me, if I did not.

As the subject says, the patch allows configure (for subversion) to be
called with --with-neon=PREFIX, where PREFIX is the build dir of neon
instead of the (in my case non-existing) installation dir, which is
already possible with apr and apr-util.

Without the patch, one is required to have neon either as sub-directory
of subversion or to have it installed somewhere via "make install".
The patch prevents that one has to do this install, in case such an
install is not desired.

To be more illustrative, I have:

/usr/local/src/phi/apr
/usr/local/src/phi/apr-util
/usr/local/src/phi/neon
/usr/local/src/phi/svn-r2947
/usr/local/src/phi/svn-r2984
/usr/local/src/phi/svn-r3200

and in, e.g. /usr/local/src/phi/svn-r3200, configure with:

./configure --disable-shared --enable-maintainer-mode \
            --with-apr=/usr/local/src/phi/apr \
            --with-apr-util=/usr/local/src/phi/apr-util \
            --with-neon=/usr/local/src/phi/neon

Without the patch, configure fails, because it searches neon-config in
neon/bin which does not exist in the build dir, and additionally,
neon-config returns the pathes which would be correct, if it had been
installed, e.g. /usr/local/include.

I am not completely happy with the patch, because it is the third
iteration of some similar code in ac-helpers/neon.m4. But I did see no
way to avoid this without a major rewrite, which I am not ready to do.

From a glance, the ideal solution seems to be to make neon/neon-config
smarter, in the same way as apr/apr-config seems to be: it handles the
case that it is called with the build dir and returns adapted pathes.

That said, the patch works for me and the result passes "make check",
but I only tested it with --disable-shared.

Bye,

        Benjamin.

2002-09-21 Benjamin Pflugmann <benjamin-svn-log@pflugmann.de>

        * ac-helpers/neon.m4:
        (SVN_LIB_NEON): Allow --with-neon=PREFIX to point to a build
        directory of neon, as is allowed with apr and apr-util, in order
        to not require a global or temporary install.

Index: ac-helpers/neon.m4
===================================================================
--- ac-helpers/neon.m4
+++ ac-helpers/neon.m4 2002-09-21 07:22:02.000000000 +0200
@@ -34,11 +34,45 @@
   [
     if test "$withval" = "yes" ; then
       AC_MSG_ERROR([--with-neon requires an argument.])
+ SVN_NEON_CONFIG()
     else
- neon_config="$withval/bin/neon-config"
- fi
+ if test ! -x "$withval/neon-config"; then
+ neon_config="$withval/bin/neon-config"
+ SVN_NEON_CONFIG()
+ else
+ # supposed to be a build dir
+ # Tweak variables to point to the right places. This would probably be
+ # superflous, if neon-config was smarter, like apr-config appears to be
+ AC_MSG_CHECKING([neon library version])
+ NEON_VERSION=`$withval/neon-config --version | sed -e 's/^neon //'`
+ AC_MSG_RESULT([$NEON_VERSION])
+
+ case "$NEON_VERSION" in
+ $NEON_WANTED_REGEX)
+ SVN_NEON_INCLUDES=-"I$withval/src"
+ NEON_LIBS="$withval/src/libneon.la"
 
- SVN_NEON_CONFIG()
+ AC_MSG_CHECKING([for any extra libraries neon needs])
+ # this is not perfect since it will pick up extra -L flags too,
+ # but that shouldn't do any real damage.
+ NEON_LIBS_NEW=`$SHELL $withval/neon-config --libs | sed -e"s/-lneon//g"`
+ AC_MSG_RESULT([$NEON_LIBS_NEW])
+ NEON_LIBS="$NEON_LIBS $NEON_LIBS_NEW"
+ # Also find out which macros neon defines (but ignore extra include paths):
+ # this will include -DNEON_SSL if neon was built with SSL support
+ changequote(<<, >>)dnl
+ CFLAGS="$CFLAGS `$SHELL $withval/neon-config --cflags | sed -e 's/-I[^ ]*//g'`"
+ changequote([, ])dnl
+ ;;
+
+ *)
+ echo "You have neon version $NEON_VERSION,"
+ echo "but Subversion needs neon ${NEON_LATEST_WORKING_VER}."
+ SVN_DOWNLOAD_NEON()
+ ;;
+ esac
+ fi
+ fi
   ],
   [
     if test -d $abs_srcdir/neon ; then