----- Original Message -----
From: "Jakub Hrozek" <jhrozek(a)redhat.com>
To: sssd-devel(a)lists.fedorahosted.org
Sent: Wednesday, February 25, 2015 4:57:25 PM
Subject: Re: [SSSD] [PATCH] RFC: Support both cmocka 1.0 and pre-1.0 API at the same
time
On Wed, Feb 25, 2015 at 03:17:54PM +0100, Pavel Březina wrote:
> On 02/25/2015 03:10 PM, Lukas Slebodnik wrote:
> >On (25/02/15 10:55), Jakub Hrozek wrote:
> >>On Wed, Feb 25, 2015 at 10:14:18AM +0100, Jakub Hrozek wrote:
> >>>On Wed, Feb 25, 2015 at 09:51:24AM +0100, Lukas Slebodnik wrote:
> >>>>On (25/02/15 08:33), Jakub Hrozek wrote:
> >>>>>On Tue, Feb 24, 2015 at 09:16:09PM +0100, Lukas Slebodnik
wrote:
> >>>>>>On (24/02/15 19:52), Jakub Hrozek wrote:
> >>>>>>>On Tue, Feb 24, 2015 at 05:52:26PM +0100, Lukas
Slebodnik wrote:
> >>>>>>>>On (24/02/15 16:55), Jakub Hrozek wrote:
> >>>>>>>>>Hi,
> >>>>>>>>>
> >>>>>>>>>the attached two patches add a way to detect
pre-1.0 cmocka and
> >>>>>>>>>adds
> >>>>>>>>>compatible definitions in the first patch and
uses them to convert
> >>>>>>>>>a
> >>>>>>>>>single unit test.
> >>>>>>>>>
> >>>>>>>>>I found the approach ugly myself, so much that
I'm considering
> >>>>>>>>>converting
> >>>>>>>>>all cmocka-based tests to cmocka-1.0 and
don't compile the cmocka
> >>>>>>>>>tests at
> >>>>>>>>>all unless 1.0 or later is present on the
system. It's not
> >>>>>>>>>functionality
> >>>>>>>>>after all, "just" tests and for CI we
could add cmocka-1.0 to the
> >>>>>>>>>CI
> >>>>>>>>>system ourselves..
> >>>>>>>>>
> >>>>>>>>>Opinions?
> >>>>>>>>+1 for patch.
> >>>>>>>
> >>>>>>>Does +1 for the patch also mean -1 for the proposal to
*only*
> >>>>>>>support
> >>>>>>>cmocka-1.0 and later?
> >>>>>>>
> >>>>>>>>I already see deprecated warnings. cmocka 1.0 is in
f21
> >>>>>>>>updates-testing
> >>>>>>>
> >>>>>>>Yes, btw Andreas would update libcmocka on all releases,
including
> >>>>>>>private RHEL buildroots. So the "only" systems
running pre-1.0
> >>>>>>>cmocka
> >>>>>>>would be non-RH distributions.
> >>>>>>>
> >>>>>>>For instance Ubuntu contains 0.4..
> >>>>>>cmocka is optional dependency but we try to run CI build on
debian
> >>>>>>testing.
> >>>>>>Debian testing (Jessie) is frozen since 2014-Oct-05. So we
will need
> >>>>>>to wait
> >>>>>>for next debian release to have cmocka-1.0 in (next) Debian
testing)
> >>>>>>
> >>>>>>If we agree we disable cmocka tests in our CI on debian
> >>>>>>I'm fine with support *only* cmocka-1.0 and later.
> >>>>>
> >>>>>I was proposing to build cmocka-1.0 from source on the Debian
CI
> >>>>>machines.
> >>>>I haven't seen this proposal yet :-)
> >>>>
> >>>>IMHO, it's reasonable compromise.
> >>>>If Nikolai agrees let's go with cmocka-1.0+ way
> >>>
> >>>Does Nikolai agree? :-)
> >>
> >>btw what about the stable branches, do we just remove the test when
> >>backporting patches with new tests? I think that would be the easiest
> >>way..
> >>
> >>Alternatively, we could add the new tests to a new block HAVE_CMOCKA_1_0
> >>in
> >>Makefile.am -- that would bring some work when backporting patches, but
> >>we wouldn't have to diverge or change the code itself, only the
> >>Makefile.am hunk, where the conflict would be minimal.
> >
> >I would prefer adding strict requirements to cmocka-1.0 into configure
> >and if it is not detected then cmocka test will not be executed.
> >
> >We can ignore(disable) deprecated warning in stable branches:-)
> >
> >LS
>
> +1
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.
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.
I'm fine with a), do other developers agree?
Ah, sorry. The +1 was actualy for HAVE_CMOCKA_1_0 block. I don't think we have to
touch the patches at all if we add a new conditional block to Makefile.am in master that
will be executed only if new features are available.