[SSSD] [PATCH] RFC: Support both cmocka 1.0 and pre-1.0 API at the same time

Lukas Slebodnik lslebodn at redhat.com
Tue Mar 10 13:52:56 UTC 2015


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=ea1575581b86cd3261f26cbe42f90ccf94b219e0
>> 
>> 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 at 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])
 ])


ACK to 2nd and 3rd patch.

LS




More information about the sssd-devel mailing list