[PATCH weston 03/11] configure.ac: Correctly check for functions and libraries

[Ander Conselvan de Oliveira]

On 12/09/2013 12:44 PM, Quentin Glidic wrote:
> On 09/12/2013 11:28, Ander Conselvan de Oliveira wrote:
>> On 12/08/2013 08:45 PM, Quentin Glidic wrote:
>>> From: Quentin Glidic <sardemff7+git at sardemff7.net>
>>>
>>> AC_SEARCH_LIBS is preferred over AC_CHECK_FUNC and AC_CHECK_LIB
>>
>> Why is it preferred?
>
> I am trusting the Autoconf manual[1] on this one. Basically, the
> double-check we are doing for dlopen is exactly what AC_SEARCH_LIBS is
> done for: checking in both a library and in libC.

That is the kind of information I would like to see in the commit 
message if I stumble upon this 6 months from now. No need to dig through 
the Autoconf manual. :)


>>>
>>> Signed-off-by: Quentin Glidic <sardemff7+git at sardemff7.net>
>>> ---
>>>   configure.ac | 37 +++++++++++++++++++++----------------
>>>   1 file changed, 21 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/configure.ac b/configure.ac
>>> index 40ae145..c169311 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -41,9 +41,13 @@ AC_ARG_VAR([WESTON_SHELL_CLIENT],
>>>
>>>   PKG_PROG_PKG_CONFIG()
>>>
>>> -AC_CHECK_FUNC([dlopen], [],
>>> -              AC_CHECK_LIB([dl], [dlopen], DLOPEN_LIBS="-ldl"))
>>> -AC_SUBST(DLOPEN_LIBS)
>>> +AC_SEARCH_LIBS([dlopen], [dl])
>>> +case "$ac_cv_search_dlopen" in
>>> +    no) AC_MSG_ERROR([dlopen support required for Weston]) ;;
>>> +    "none required") ;;
>>> +    *) DLOPEN_LIBS="$ac_cv_search_dlopen" ;;
>>> +esac
>>> +AC_SUBST([DLOPEN_LIBS])
>>>
>>>   AC_CHECK_DECL(SFD_CLOEXEC,[],
>>>             [AC_MSG_ERROR("SFD_CLOEXEC is needed to compile weston")],
>>> @@ -258,13 +262,13 @@ fi
>>>   AM_CONDITIONAL(ENABLE_VAAPI_RECORDER, test "x$have_libva" = xyes)
>>>
>>>
>>> -AC_CHECK_LIB([jpeg], [jpeg_CreateDecompress], have_jpeglib=yes)
>>> -if test x$have_jpeglib = xyes; then
>>> -  JPEG_LIBS="-ljpeg"
>>> -else
>>> -  AC_ERROR([libjpeg not found])
>>> -fi
>>> -AC_SUBST(JPEG_LIBS)
>>> +AC_SEARCH_LIBS([jpeg_CreateDecompress], [jpeg])
>>> +case "$ac_cv_search_pam_open_session" in
>>
>> Copy & paste error, you got pam_open_session instead of
>> jpeg_CreateDecompress.
>
> Oops, fixed locally.
>
>
>>> +    no) AC_MSG_ERROR([libjpeg required but not found]) ;;
>>> +    "none required") ;;
>>> +    *) JPEG_LIBS="$ac_cv_search_jpeg_CreateDecompress" ;;
>>> +esac
>>> +AC_SUBST([JPEG_LIBS])
>>
>> Anyway, this seems like a change for the worse IMO.
>
> Following Autotools best practices/manual is *never* worse.
>
>  > Now you're typing jpeg_CreateDecompress three times instead of just one.
>
> 8 lines for the new version vs 7 lines for the old one…

My point was that having to type the same thing three times is error 
prone, and the copy & paste error above kind of proves my point. Anyway, 
if we can turn this into a one line macro that would be much better.

>  > Ideally this could be just a one-liner, since this construct has to be
>> repeated a few times.
>
> It is easy enough to create a macro to do the check. If so, do we want
> the error message to be accurate (e.g. Weston vs clients vs weston-launch)?

I think it would be good, if it's just a matter of adding one more 
parameter.


Cheers,
Ander

>
>
>> Cheers,
>> Ander
>>
>>>
>>>   PKG_CHECK_MODULES(CAIRO, [cairo])
>>>
>>> @@ -334,12 +338,13 @@ AS_IF([test "x$have_systemd_login_209" = "xyes"],
>>>   AC_ARG_ENABLE(weston-launch, [  --enable-weston-launch],,
>>> enable_weston_launch=yes)
>>>   AM_CONDITIONAL(BUILD_WESTON_LAUNCH, test x$enable_weston_launch ==
>>> xyes)
>>>   if test x$enable_weston_launch == xyes; then
>>> -  AC_CHECK_LIB([pam], [pam_open_session], [have_pam=yes],
>>> [have_pam=no])
>>> -  if test x$have_pam == xno; then
>>> -    AC_ERROR([weston-launch requires pam])
>>> -  fi
>>> -  PAM_LIBS=-lpam
>>> -  AC_SUBST(PAM_LIBS)
>>> +    AC_SEARCH_LIBS([pam_open_session], [pam])
>>> +    case "$ac_cv_search_pam_open_session" in
>>> +        no) AC_MSG_ERROR([pam support required for weston-launch]) ;;
>>> +        "none required") ;;
>>> +        *) PAM_LIBS="$ac_cv_search_pam_open_session" ;;
>>> +    esac
>>> +    AC_SUBST([PAM_LIBS])
>>>   fi
>>>
>>>   if test x$enable_egl = xyes; then
>>>
>>
>
> [1]
> http://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf.html#Libraries
>
>