Re: [PATCH] Update the build system on *nix to support Cyrus SASL

On 5/31/06, Vlad Georgescu <vgeorgescu@gmail.com> wrote:
> I changed the build system to handle passing --with-sasl[=PATH] to configure.
>
> [[[
> Update the build system on *nix to support Cyrus SASL.
>
> * build/ac-macros/sasl.m4: New file.
> * configure.in: Call SVN_SASL_LIB.
> * Makefile.in
> (SVN_SASL_LIBS, SVN_SASL_INCLUDES): New AC_SUBSTed variables.
> Add SVN_SASL_INCLUDES to INCLUDES.
> * aclocal.m4
> Add build/ac-macros/sasl.m4 to the list of supplementary macros.
> * build.conf
> (sasl): New entry.
> (libsvn_ra_svn, svnserve): Add sasl to the list of dependencies.
> ]]]
>
> Any comments would be appreciated.

Nice patch, Vlad! I have a few suggestions on how we can improve this patch:
  - If the user does not supply a --with-sasl option, we should look
in the standard locations for the SASL library (e.g. /usr,
/usr/local). If SASL is found, we should use it.
  - If the user supplies a --with-sasl option, but does not specify a
directory, we should look in the standard locations (e.g. /usr,
/usr/local).
 - If the user supplies a --with-sasl option, and SASL is not found,
configure should abort with an error message.

Also, I found a few tab characters in your patch. Please replace these
characters with spaces.

Other than these minor issues, your patch looks excellent. Keep up the
good work -- I'm looking forward to the next version of your patch!

Cheers,

David

-- 
David James -- http://www.cs.toronto.edu/~james
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org