You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Peter Wojciechowski <pw...@valueclick.com> on 2006/04/01 00:53:54 UTC

[PATCH] correct a couple of typos in build/dbd.m4

I started mucking around with mod_dbd, and noticed a couple of typos
with dbd.m4, and a small update to INSTALL.MySQL.

-Peter

RE: [PATCH] correct a couple of typos in build/dbd.m4

Posted by Peter Wojciechowski <pw...@valueclick.com>.
So if I follow you correctly the file patch should check the extra areas where the libs and includes could reside?  Where I ran into problems building was with the mysql install tree where the includes are under mysql/include/mysql.h and the libs being under a similar structure lib/mysql/libmysqlclient_r.so.

The attached checks for mysql.h in either $withval/include or $withval/include/mysql, it does the same with lib before adding.

-Peter



From: Max Bowsher
Sent: Sat 4/1/2006 6:35 AM
To: Peter Wojciechowski
Cc: dev@apr.apache.org
Subject: Re: [PATCH] correct a couple of typos in build/dbd.m4


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Peter Wojciechowski wrote:
> I started mucking around with mod_dbd, and noticed a couple of typos
> with dbd.m4, and a small update to INSTALL.MySQL.

Thankyou for the patch! Some comments inline below:

> --- build/dbd.m4	2005-05-05 12:24:29.000000000 -0700
> +++ ../apr-util-1.2.6_new/build/dbd.m4	2006-03-31 06:48:03.000000000 -0800
> @@ -81,27 +81,27 @@
>        if test "$apu_have_mysql" == "0"; then
>          AC_CHECK_HEADER(mysql/mysql.h, AC_CHECK_LIB(mysqlclient_r, mysql_init, [apu_have_mysql=1]))
>          if test "$apu_have_mysql" != "0"; then
> -          APR_ADDTO(APRUTIL_INCLUDES, [-I$withval/include/myql])
> +          APR_ADDTO(APRUTIL_INCLUDES, [-I$withval/include])

A typo, sure, but examining the surrounding code reveals that this line
only executes when $withval = "yes". The flags -Iyes/include/myql and
- -Iyes/include are equally broken, and there is no value in fixing a typo
in broken code that needs to be removed anyway.

>          fi
>        fi
>      elif test "$withval" = "no"; then
>        apu_have_mysql=0
>      else
>        CPPFLAGS="-I$withval/include"
> -      LIBS="-L$withval/lib "
> +      LIBS="-L$withval/lib/mysql "

This is not a typo fix. It is a functional change, which based on the
file layout of the libraries on my Debian system, is in fact incorrect.

>  
>        AC_MSG_NOTICE(checking for mysql in $withval)
>        AC_CHECK_HEADER(mysql.h, AC_CHECK_LIB(mysqlclient_r, mysql_init, [apu_have_mysql=1]))
>        if test "$apu_have_mysql" != "0"; then
> -        APR_ADDTO(APRUTIL_LDFLAGS, [-L$withval/lib])
> +        APR_ADDTO(APRUTIL_LDFLAGS, [-L$withval/lib/mysql])

Ditto.

>          APR_ADDTO(APRUTIL_INCLUDES, [-I$withval/include])
>        fi
>  
>        if test "$apu_have_mysql" != "1"; then
>          AC_CHECK_HEADER(mysql/mysql.h, AC_CHECK_LIB(mysqlclient_r, mysql_init, [apu_have_mysql=1]))
>          if test "$apu_have_mysql" != "0"; then
> -          APR_ADDTO(APRUTIL_INCLUDES, [-I$withval/include/mysql])
> -          APR_ADDTO(APRUTIL_LDFLAGS, [-L$withval/lib])
> +          APR_ADDTO(APRUTIL_INCLUDES, [-I$withval/include])
> +          APR_ADDTO(APRUTIL_LDFLAGS, [-L$withval/lib/mysql])

Ditto on the LDFLAGS element of the above.

The INCLUDES bit seems to have merit to me, since it should agree with
the CPPFLAGS setting above.

>          fi
>        fi
>      fi
> 
> --- INSTALL.MySQL	2006-01-25 21:11:08.000000000 -0800
> +++ ../apr-util-1.2.6_new/INSTALL.MySQL	2006-03-31 14:46:45.000000000 -0800
> @@ -5,3 +5,3 @@
>  and copy it into the dbd directory.
> -Now run buildconf, followed by configure.
> +Now run buildconf, followed by configure --with-mysql=</path/to/mysql>.

/path/to/mysql is a little vague.

/path/to/mysql/installation/prefix would better express the type of path
that is expected.

Max.



-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (Cygwin)

iD8DBQFELo+pfFNSmcDyxYARAkYEAJ45gW/xrI/KiTJXA13XvLzJEgnyZQCglvvG
ocVaYpO42vtOqYkTkz7ysVA=
=SyiT
-----END PGP SIGNATURE-----

Re: [PATCH] correct a couple of typos in build/dbd.m4

Posted by Max Bowsher <ma...@ukf.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Peter Wojciechowski wrote:
> I started mucking around with mod_dbd, and noticed a couple of typos
> with dbd.m4, and a small update to INSTALL.MySQL.

Thankyou for the patch! Some comments inline below:

> --- build/dbd.m4	2005-05-05 12:24:29.000000000 -0700
> +++ ../apr-util-1.2.6_new/build/dbd.m4	2006-03-31 06:48:03.000000000 -0800
> @@ -81,27 +81,27 @@
>        if test "$apu_have_mysql" == "0"; then
>          AC_CHECK_HEADER(mysql/mysql.h, AC_CHECK_LIB(mysqlclient_r, mysql_init, [apu_have_mysql=1]))
>          if test "$apu_have_mysql" != "0"; then
> -          APR_ADDTO(APRUTIL_INCLUDES, [-I$withval/include/myql])
> +          APR_ADDTO(APRUTIL_INCLUDES, [-I$withval/include])

A typo, sure, but examining the surrounding code reveals that this line
only executes when $withval = "yes". The flags -Iyes/include/myql and
- -Iyes/include are equally broken, and there is no value in fixing a typo
in broken code that needs to be removed anyway.

>          fi
>        fi
>      elif test "$withval" = "no"; then
>        apu_have_mysql=0
>      else
>        CPPFLAGS="-I$withval/include"
> -      LIBS="-L$withval/lib "
> +      LIBS="-L$withval/lib/mysql "

This is not a typo fix. It is a functional change, which based on the
file layout of the libraries on my Debian system, is in fact incorrect.

>  
>        AC_MSG_NOTICE(checking for mysql in $withval)
>        AC_CHECK_HEADER(mysql.h, AC_CHECK_LIB(mysqlclient_r, mysql_init, [apu_have_mysql=1]))
>        if test "$apu_have_mysql" != "0"; then
> -        APR_ADDTO(APRUTIL_LDFLAGS, [-L$withval/lib])
> +        APR_ADDTO(APRUTIL_LDFLAGS, [-L$withval/lib/mysql])

Ditto.

>          APR_ADDTO(APRUTIL_INCLUDES, [-I$withval/include])
>        fi
>  
>        if test "$apu_have_mysql" != "1"; then
>          AC_CHECK_HEADER(mysql/mysql.h, AC_CHECK_LIB(mysqlclient_r, mysql_init, [apu_have_mysql=1]))
>          if test "$apu_have_mysql" != "0"; then
> -          APR_ADDTO(APRUTIL_INCLUDES, [-I$withval/include/mysql])
> -          APR_ADDTO(APRUTIL_LDFLAGS, [-L$withval/lib])
> +          APR_ADDTO(APRUTIL_INCLUDES, [-I$withval/include])
> +          APR_ADDTO(APRUTIL_LDFLAGS, [-L$withval/lib/mysql])

Ditto on the LDFLAGS element of the above.

The INCLUDES bit seems to have merit to me, since it should agree with
the CPPFLAGS setting above.

>          fi
>        fi
>      fi
> 
> --- INSTALL.MySQL	2006-01-25 21:11:08.000000000 -0800
> +++ ../apr-util-1.2.6_new/INSTALL.MySQL	2006-03-31 14:46:45.000000000 -0800
> @@ -5,3 +5,3 @@
>  and copy it into the dbd directory.
> -Now run buildconf, followed by configure.
> +Now run buildconf, followed by configure --with-mysql=</path/to/mysql>.

/path/to/mysql is a little vague.

/path/to/mysql/installation/prefix would better express the type of path
that is expected.

Max.



-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (Cygwin)

iD8DBQFELo+pfFNSmcDyxYARAkYEAJ45gW/xrI/KiTJXA13XvLzJEgnyZQCglvvG
ocVaYpO42vtOqYkTkz7ysVA=
=SyiT
-----END PGP SIGNATURE-----