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

[1] 
http://anonscm.debian.org/cgit/pkg-sssd/sssd.git/commit/?id=ea1575581b86cd3261f26cbe42f90ccf94b219e0

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.

LS
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to