----- Original Message ----- > From: "Jakub Hrozek" <jhro...@redhat.com> > To: sssd-devel@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. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
