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])
])
ACK to 2nd and 3rd patch.
LS