On Wed, Feb 25, 2015 at 11:20:45PM +0100, Lukas Slebodnik wrote:
> On (25/02/15 16:57), Jakub Hrozek wrote:
> >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.
> >
> Agree with strict requirement
> Agree with converting tests.
>
> >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
OK, I didn't propose this change because it would /decrease/ the test
coverage. But seeing the summary above, it does seem like the best
solution.
Thanks.
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
