On (25/02/15 23:29), Jakub Hrozek wrote:
>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.
BTW my current workaround to get rid of deprecated warnings is to modify
cmocka.h
#define CMOCKA_DEPRECATED
instead of
#define CMOCKA_DEPRECATED __attribute__ ((deprecated))
I know it's not nice solution.
LS
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
