On (10/03/15 14:20), Jakub Hrozek wrote:
>On Wed, Feb 25, 2015 at 11:20:45PM +0100, Lukas Slebodnik wrote:
>> >I'm not sure we understood each other, I was specifically asking about
>> >stabe branch. In master, we would add strict requirements for cmocka
>> >1.0+, convert all tests there and don't run any tests if cmocka 1.0+ is
>> >not found.
>> >
>> Agree with strict requirement
>> Agree with converting tests.
>
>See attached patches. CI -
>http://sssd-ci.duckdns.org/logs/job/9/24/summary.html
>
>>
>> >In sssd-1-12, we would keep the existing tests untouched and ignore the
>> >deprecation warnings. But what if someone submits a patch that needs to
>> >be included in sssd-1-12, too but adds a test that is written using
>> >cmocka-1.0 API? We could either:
>> > a) backport the patch without the test
>> > b) backport the patch as-is, but add the Makefile.am part of the
>> > patch into a new block that gets executed only if cmocka 1.0 is
>> > available. Currently we only have a global HAVE_CMOCKA if-endif.
>> c)
>> - add strict requirement for cmocka 1.0+
>> - do not convert tests
>> - ignore warnings caused by deprecated function in cmocka 1.0
>>
>> Result:
>> * change in stable branch will be small
>> * there will not be problem with backporting new tests
>> * makefile will not be poluted with another if else block
>> * if cmocka-1.0+ is not available then cmocka tests will not be executed
>>
>> Distributions:
>> arch - does not run tests at all
>> opensuse - does not run tests at all
>> gentoo - does not run cmocka test
>> debian - cmocka tests are disabled (due to failueres on some arch) [1]
>> ubuntu - cmocka tests are disabled (due to failueres on some arch)
>> fedora - cmocka-1.0 will be available
>> epel{6,7} - cmocka-1.0 will be available
>>
>> [1]
http://anonscm.debian.org/cgit/pkg-sssd/sssd.git/commit/?id=ea1575581b86c...
>>
>> cmocka is an optional dependency. So I don't think we need to complicate
>> Makefile in stable branches with with cmocka < 1.0 and cmocka >= 1.0.
>
>I think we should just push the first and second patch to sssd-1-12,
>then.
>From 6cdaf7234c8f1b4e28aeb92129c015ec336c24bb Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek(a)redhat.com>
>Date: Mon, 9 Mar 2015 21:25:47 +0100
>Subject: [PATCH 1/3] build: Only run cmocka tests if cmocka 1.0 or newer is
> available
>
>Rename the Makefile.am conditionals to HAVE_CMOCKA_1 to make it clear
>only the latest release is suported.
>---
> Makefile.am | 12 ++++++------
> src/external/libcmocka.m4 | 19 +++++++++++++++++--
> src/tests/cwrap/Makefile.am | 4 ++--
> 3 files changed, 25 insertions(+), 10 deletions(-)
>
>diff --git a/Makefile.am b/Makefile.am
>index
ba24343c3a2be9edc94ec817d3709a0c78599847..0b063e1f357174f1eecc0b37243f53e4e5040eba 100644
>--- a/Makefile.am
>+++ b/Makefile.am
>@@ -192,7 +192,7 @@ endif # BUILD_DBUS_TESTS
>
> endif # HAVE_CHECK
>
>-if HAVE_CMOCKA
>+if HAVE_CMOCKA_1
> non_interactive_cmocka_based_tests = \
> nss-srv-tests \
> test-find-uid \
>@@ -237,7 +237,7 @@ non_interactive_cmocka_based_tests += \
> ad_gpo_tests
> endif
>
>-endif # HAVE_CMOCKA
>+endif # HAVE_CMOCKA_1
>
> check_PROGRAMS = \
> stress-tests \
>@@ -245,9 +245,9 @@ check_PROGRAMS = \
> $(non_interactive_cmocka_based_tests) \
> $(non_interactive_check_based_tests)
>
>-if HAVE_CMOCKA
>+if HAVE_CMOCKA_1
> check_PROGRAMS += dummy-child
>-endif # HAVE_CMOCKA
>+endif # HAVE_CMOCKA_1
>
> PYTHON_TESTS =
>
>@@ -1716,7 +1716,7 @@ sss_config_tests_LDADD = \
> endif # BUILD_CONFIG_LIB
> endif # BUILD_IFP
>
>-if HAVE_CMOCKA
>+if HAVE_CMOCKA_1
>
> TEST_MOCK_RESP_OBJ = \
> src/tests/cmocka/common_mock_resp.c \
>@@ -2281,7 +2281,7 @@ test_fo_srv_LDADD = \
> libsss_test_common.la \
> $(NULL)
>
>-endif # HAVE_CMOCKA
>+endif # HAVE_CMOCKA_1
>
> noinst_PROGRAMS = pam_test_client
> if BUILD_SUDO
>diff --git a/src/external/libcmocka.m4 b/src/external/libcmocka.m4
>index
740d9ad254b50563d4d1cbcab3d10f5920ffd79f..b45ce49a56b0690010bbd7fc6356d77bffb3dcd4 100644
>--- a/src/external/libcmocka.m4
>+++ b/src/external/libcmocka.m4
>@@ -10,9 +10,24 @@ AC_DEFUN([AM_CHECK_CMOCKA],
> )
> AS_IF([test x"$cmocka_required_headers" != x"no"],
> [PKG_CHECK_MODULES([CMOCKA], [cmocka],
[have_cmocka="yes"])]
>- )],
>+ )
>+
>+ dnl Check if we have the new runner or the legacy one
>+ SAVE_CFLAGS=$CFLAGS
>+ SAVE_LIBS=$LIBS
>+ CFLAGS="$CFLAGS $CMOCKA_CFLAGS"
>+ LIBS="$LIBS $CMOCKA_LIBS"
>+
>+ AS_IF([test x"$have_cmocka" == x"yes"],
>+ [AC_CHECK_FUNC([_cmocka_run_group_tests],
[have_cmocka_1="yes"]])
>+ )
>+
>+ CFLAGS=$SAVE_CFLAGS
>+ LIBS=$SAVE_LIBS
>+
>+ ],
> dnl PKG_CHECK_EXISTS ACTION-IF-NOT-FOUND
> [AC_MSG_WARN([No libcmocka library found, cmocka tests will not be built])]
> )
>- AM_CONDITIONAL([HAVE_CMOCKA], [test x$have_cmocka = xyes])
>+ AM_CONDITIONAL([HAVE_CMOCKA_1], [test x$have_cmocka_1 = xyes])
We agreed to drop support for older version of cmocka.
So we can let pkg-config to detect cmocka 1.0 and we do not need
special case makefile with HAVE_CMOCKA_1.
The 1st patch can be reduced to next diff
dnl A macro to check presence of cmocka on the system
AC_DEFUN([AM_CHECK_CMOCKA],
[
- PKG_CHECK_EXISTS(cmocka,
+ PKG_CHECK_EXISTS(cmocka >= 1.0.0,
[AC_CHECK_HEADERS([stdarg.h stddef.h setjmp.h],
[], dnl We are only intrested in action-if-not-found
[AC_MSG_WARN([Header files stdarg.h stddef.h setjmp.h are required by
cmocka])
@@ -12,7 +12,7 @@ AC_DEFUN([AM_CHECK_CMOCKA],
[PKG_CHECK_MODULES([CMOCKA], [cmocka], [have_cmocka="yes"])]
)],
dnl PKG_CHECK_EXISTS ACTION-IF-NOT-FOUND
- [AC_MSG_WARN([No libcmocka library found, cmocka tests will not be built])]
+ [AC_MSG_WARN([No libcmocka-1.0.0 library found, cmocka tests will not be
built])]
)
AM_CONDITIONAL([HAVE_CMOCKA], [test x$have_cmocka = xyes])
])
Ah, true. I'm not sure why my first patch was so complex, I think I
tried to support both again..anyway, your version is correct.