[lttng-dev] [PATCH lttng-tools 2/2] Fix: Missing libs dependencies in configure check for lttng-ust-ctl

[Christian Babeux]

Hi Mathieu,

> not sure I understand why all the changes you are doing are needed. What
> happens if you just do:
>
> -             [AC_MSG_ERROR([Cannot find LTTng-UST. Use [LDFLAGS]=-Ldir to specify its location, or specify --disable-lttng-ust to build lttng-tools without LTTng-UST support.])]
> +             [AC_MSG_ERROR([Cannot find LTTng-UST. Use [LDFLAGS]=-Ldir to specify its location, or specify --disable-lttng-ust to build lttng-tools without LTTng-UST support.])],
> +             [-lurcu-common -lurcu-bp -lurcu-cds]
>
> ?

If we only change the other-lib case of the AC_CHECK_LIB, subsequent
calls to this macro will fail unexpectedly. For example, the next call
to this macro is the check for dlopen in the dl lib. Since the previous
call (the check for lttng-ust-ctl) succeeded, -llttng-ust-ctl has been
prepended to the $LIBS variable.

The problem arise when trying to link a test program
(the actual test of AC_CHECK_LIB) to check for the dl lib. The linking
will fail for the same reason, because the dependencies to
urcu-{common,bp,cds} have not been manually specified and lttng-ust-ctl
has been added to the $LIBS flags passed to the linker. Hence,
the test for the dl lib will fail, not because it can't find the dl lib,
but because the linking fail as a side effect of adding the lttng-ust-ctl lib.

There is possibly three other ways to fix this issue that I know of:

- The check for lttng-ust-ctl lib should be the last AC_CHECK_LIB.
  This is exactly the situation I was trying to avoid. Someone could
  inadvertently add one after and silently break cross-compilation
  builds.

- Add AC_CHECK_LIB checks for the dependents URCU libs.
  This will add a -lurcu-{common,bp,cds} when linking.
  I'm not fully aware of the repercussions this could have.

- Use PKG_CONFIG support in configure.ac.
  This might be a more elegant and long-term solution so we don't have
  to rely on these kind of trickery to check for libraries
  and correctly support cross-compilation builds.


>> +     AC_CHECK_LIB([lttng-ust-ctl], [ustctl_create_session],
>> +             [
>> +                     AC_DEFINE([HAVE_LIBLTTNG_UST_CTL], [1], [has LTTng-UST control support])
>
> wasn't HAVE_LIBLTTNG_UST_CTL already defined by the AM_CONDITIONAL below ?
>
>> +                     lttng_ust_ctl_found=yes
>
> why are you adding this ?
>

The default behavior of the action-if-found in AC_CHECK_LIB is to
prepend '-llibrary' and defining 'HAVE_LIBlibrary' [1].
Since we don't want to prepend -llttng-ust-ctl, we must manually define
HAVE_LIBlibrary.

The AM_CONDITIONAL does not define the HAVE_LIBLTTNG_UST_CTL
(as in the preprocessor sense) it is the AC_CHECK_LIB check
that normally does (via an underlying call to AC_DEFINE).
AM_CONDITIONAL only make the HAVE_LIBLTTNG_UST_CTL variable
available to use in Makefile.am [2].

>> -
>> -AM_CONDITIONAL([HAVE_LIBLTTNG_UST_CTL], [ test "x$ac_cv_lib_lttng_ust_ctl_ustctl_create_session" = "xyes" ])
>> -
>> +AM_CONDITIONAL([HAVE_LIBLTTNG_UST_CTL], [test "x$lttng_ust_ctl_found" = xyes])
>
> why are you changing this line ?
>

I only changed the test flag to lttng_ust_ctl_found for clarity sake.
It might not be obvious to everyone that the
$ac_cv_lib_lttng_ust_ctl_ustctl_create_session variable is a cached
test result by autoconf.

I think this patch is not the most obvious. I would like to know
if anyone has any other ideas to resolve this issue.

Thoughts?

Thanks,

Christian

1 - https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/Libraries.html#Libraries
2 - https://www.gnu.org/software/automake/manual/html_node/Usage-of-Conditionals.html