On Mon, Apr 4, 2016 at 11:08 AM, Anders Roxell <anders.rox...@linaro.org> wrote: > On 26 March 2016 at 02:06, Brian Brooks <brian.bro...@linaro.org> wrote: >> On 04/01 22:15:34, Anders Roxell wrote: >>> On 25 March 2016 at 20:25, Brian Brooks <brian.bro...@linaro.org> wrote: >>> > If --with-cunit-path=DIR is used, skip the AC_CHECK_LIB and modify the >>> > linker >>> > flags accordingly. >>> >>> Why should we skip the AC_CHECK_LIB and AC_CHECK_HEADERS step >>> don't feel right. >>> >>> > >>> > Signed-off-by: Brian Brooks <brian.bro...@linaro.org> >>> > --- >>> > test/m4/validation.m4 | 8 ++++++-- >>> > 1 file changed, 6 insertions(+), 2 deletions(-) >>> > >>> > diff --git a/test/m4/validation.m4 b/test/m4/validation.m4 >>> > index b137118..f6c93f7 100644 >>> > --- a/test/m4/validation.m4 >>> > +++ b/test/m4/validation.m4 >>> > @@ -33,12 +33,16 @@ AC_HELP_STRING([--with-cunit-path=DIR path to CUnit >>> > libs and headers], >>> > >>> > ########################################################################## >>> > # Check for CUnit availability >>> > >>> > ########################################################################## >>> > -if test x$cunit_support = xyes >>> > +if test x$cunit_support = xyes -a -z "$CUNIT_PATH" >>> > then >>> > AC_CHECK_LIB([cunit],[CU_get_error], [], >>> > [AC_MSG_ERROR([CUnit libraries required])]) >>> > AC_CHECK_HEADERS([CUnit/Basic.h], [], >>> > [AC_MSG_FAILURE(["can't find cunit headers"])]) >>> > else >>> > - cunit_support=no >>> > + if test -z "$CUNIT_PATH"; then >>> > + cunit_support=no >>> > + else >>> > + AM_LDFLAGS="$AM_LDFLAGS -lcunit" >>> > + fi >>> >>> this patch is still incomplete you miss to add the headers right. >> >> Hi Anders, >> >> Can you help me understand why this patch is incomplete? > > I was wrong! > I forgot that we add cunits header and lib path above. > >> >> The include & lib path for OOT CUnit is taken care of via AM_CPPFLAGS and >> AM_LDFLAGS when --with-cunit-path=DIR is used. Only "-lcunit" is required >> if AC_CHECK_LIB is skipped. > > You are correct that is enough. > >> >> AC_CHECK_LIB has side effect of LIBS+="-lcunit" which makes subsequent >> autoconf checking, e.g. for GCC atomics, fail because the include & lib >> path for OOT CUnit is in AM_CPPFLAGS and AM_LDFLAGS not CPPFLAGS and >> LDFLAGS. The previous workaround is rather tricky to do cleanly here--if >> possible at all. The reason is the location of the WAR and CUnit checking >> was at the very bottom of the configure stage. So, no subsequent building >> of tiny programs for checking feature support was happening. Now, the >> CUnit checking is happening earlier in configure stage which makes the >> previous WAR very difficult to support. I do not think it is possible. > > If we think its acceptable to fail when build and not early during > configure then > we should remove AM_CHECK_* for other OOT as well, and not only this. > >> >> The proposal here is to simply skip the AC_CHECK_LIB & AC_CHECK_HEADERS >> and add the "-lcunit" side effect via AM_LDFLAGS _only_ when >> --with-cunit-path=DIR is used. If DIR is incorrect, the build will still >> fail, just at later stage instead of in configure stage. This might be >> acceptable to developers who choose to work on ODP with an OOT CUnit. > > I like the idea of failing early in the configure step and not when building.
Here we should fail when running configure instead of waiting for the build itself to fail. We shouldn't avoid AC_CHECK_LIB as that will check if the path given contains the library, besides setting up the lib list as a consequence. I see two ways for us to fix this situation. First is by just fixing the check order, and second is by making sure that the cunit path is available for the following checks. Cheers, -- Ricardo Salveti _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
