Code review comment for lp:~laurynas-biveinis/percona-pam-for-mysql/autotools-fixes

2011/11/2 Sergei <email address hidden>:
>> -# mysql_config
>>  AC_ARG_WITH([mysql_config],
>>      AC_HELP_STRING(
>> -        [--with-mysql_config=PROGRAM],
>> +        [--with-mysql_config=PATH],
>>          [location of the mysql_config program]),
>> -    [mysql_config_prog="$withval"])
>> -AC_PATH_PROG([MYSQL_CONFIG], [mysql_config], [$mysql_config_prog])
>> +    [MYSQL_CONFIG="$with_mysql_config"])
>> +AC_PATH_PROG([MYSQL_CONFIG], [mysql_config])
>
> It looks like you first get MYSQL_CONFIG from the command line,
> and then overwrite it with AC_PATH_PROG(). So, whatever user has
> specified in --with-mysql_config will have no effect.

It is actually the other way when --with-mysql_config has no effect.
If MYSQL_CONFIG is set by the time AC_PATH_PROG is executed, (i.e.
specified on the command line, then AC_PATH_PROG will just use that
value. I have tested the current behaviour.

>> +# Replace -I with -isystem MySQL header include GCC options to silence warnings
>> +# originating from them
>> +MYSQL_INCLUDES=`"${MYSQL_CONFIG}" --include | "${SED}" 's@^-I@-isystem @'`
>
> Don't understand that :(

We would like to have -Wall -Wextra clean build. But some of the MySQL
headers give warnings and we have no choice but to silence them by
treating as system not user headers, by using -isystem instead of -I.

> I don't remember seeing
>
>  #ifdef HAVE_GETPASS
>
> in the sources. And if you don't use HAVE_GETPASS, there's no reason to
> use AC_CHECK_FUNCS([getpass])

Ack. (will probably just remove getpass with dialog plugin)

--
Laurynas