Re: Patch to neon library detection code.

On Mon, 27 Feb 2006, Paul de Vrieze wrote:

> On Monday 27 February 2006 14:50, Marcus Rueckert wrote:
> > On 2006-02-27 14:24:04 +0100, Paul de Vrieze wrote:
> > > I've identified an issue with the gentoo package for subversion. The
> > > configure script continues hapilly when external neon support was
> > > specified but not found (wrong version). I've created a small patch
> > > to remedy this.
> >
> > That is incorrect. An user can run svn without neon happily. he can
> > still use file:// or svn:// or svn+ssh://. so enforcing neon is not
> > needed. i would suggest applying this patch in your ebuild and pass an
> > USE flag for it. so only if the user specifies to use neon your build
> > should fail. otherwise it can happily ignore neon.
>
> I've checked, and the patch is indeed incorrect. It fails always when neon
> is not found. Even with --without-neon. This patch should be better,
> although it duplicates some code.

Hi Paul, thanks for the patch. Some comments...

--- subversion-1.3.0/build/ac-macros/neon.m4.orig 2006-02-27 16:13:32.000000000 +0100
+++ subversion-1.3.0/build/ac-macros/neon.m4 2006-02-27 16:19:34.000000000 +0100
@@ -142,7 +142,7 @@
       if test -z $svn_allowed_neon_on_system; then
         echo "You have neon version $NEON_VERSION,"
         echo "but Subversion needs neon $NEON_LATEST_WORKING_VER."
- SVN_DOWNLOAD_NEON()
+ SVN_DOWNLOAD_NEON2()
       fi
 
     else
@@ -177,3 +177,23 @@
   AC_MSG_RESULT([no suitable neon found])
   svn_lib_neon="no"
 ])
+
+dnl SVN_DOWNLOAD_NEON()

This comment doesn't match the name of the macro, and while the style
already exists in neon.m4 for the other incarnation of the macro, it
also strikes me as completely extraneous. Let's ditch it in both
spots.

+dnl no neon found, print out a message telling the user what to do
+AC_DEFUN(SVN_DOWNLOAD_NEON2,
+[
+ echo ""
+ echo "An appropriate version of neon could not be found, so libsvn_ra_dav"
+ echo "will not be built. If you want to build libsvn_ra_dav, please either"
+ echo "install neon ${NEON_LATEST_WORKING_VER} on this system"
+ echo ""
+ echo "or"
+ echo ""
+ echo "get neon ${NEON_LATEST_WORKING_VER} from:"
+ echo " ${NEON_URL}"
+ echo "unpack the archive using tar/gunzip and rename the resulting"
+ echo "directory from ./neon-${NEON_LATEST_WORKING_VER}/ to ./neon/"
+ echo ""
+ AC_MSG_FAILURE([no suitable neon found])
+ svn_lib_neon="no"
+])

The only difference between this new macro and the pre-existing
SVN_DOWNLOAD_NEON() macro is the use of AC_MSG_FAILURE()
vs. AC_MSG_RESULT(). The names of these macros should reflect the
differences -- something like SVN_DOWNLOAD_NEON_AND_FAIL() would be a
better name for the new macro.

These macros could also be better factored. How about use of a shared
helper macro which contains the 'echo' of all that text to avoid so
much duplication?

Thanks, Dan