[SSSD] [PATCH v3] CI: Enforce Valgrind check

Nikolai Kondrashov Nikolai.Kondrashov at redhat.com
Thu Sep 18 07:11:01 UTC 2014


On 09/17/2014 07:48 PM, Lukas Slebodnik wrote:
> On (17/09/14 18:48), Nikolai Kondrashov wrote:
>> +	$(NULL)
>   ^^^
>   '\t' instead of spaces
> Please follow our conventions.

Fixed, thank you.

>>          stage make-check-valgrind \
>>                  make-check-wrap -j $CPU_NUM check -- \
>>                                  libtool --mode=execute \
>> -                                    valgrind-condense 99 '!(*.py)' -- \
>> +                                    valgrind-condense 99 \
>> +                                    '!(*.py|*dlopen-tests)' -- \
>>                                      --vgdb=no \
>                                        ^^^^^^^^^
>                      Why do we need to have disabled vgdb?
> It is true that vgdb is enabled by default, but default value of vgdb-error is
> 999999999.

Yeah, it is almost impossible it will be hit. And vgdb is enabled to make it
easier to enable it by lowering vgdb-error. But I'd prefer not to care about
CI hanging, waiting on input for whatever reason.

>>                                      --trace-children=yes \
>>                                      --trace-children-skip='*/bin/*,*/sbin/*' \
>
> Could you explain your style of indentation here?
> I cannot find any rule for indentation. Somewhere are 4 spaces, elsewhere 8 ...
> As a result of this unusual indentation  almost everything is near the 80
> column limit.

I try to place arguments past the end of their command's name, or at least
past the start, to make it more visible. In case the arguments don't fit, I
move them to the left.

The command above is pretty nested, that's why the arguments are so far to the
right.

> ----------------
>
> You are mixing pates related to ticket #2428 with others
>     1st) contains: "Related to https://fedorahosted.org/sssd/ticket/2428"
>     3rd) contains: "Fixes https://fedorahosted.org/sssd/ticket/2428"
>     2nd, 4th) does not contain anything

Yes. Because the ticket subject says "Prepare suppression database for
valgrind test" and nothing about enforcing Valgrind check.

> It seems that you don't use our git commit template
> @see .git-commit-template
> @see git show 3d9edb4c510028def2df41aa7b0ce705b197e6fc

Fixed, thank you.

Nick
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-CI-Add-Valgrind-suppression-support.patch
Type: text/x-patch
Size: 2779 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140918/c90caa05/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-CI-Don-t-run-dlopen-tests-under-Valgrind.patch
Type: text/x-patch
Size: 1201 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140918/c90caa05/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-CI-Suppress-all-detected-Valgrind-issues.patch
Type: text/x-patch
Size: 2358 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140918/c90caa05/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-CI-Enforce-Valgrind-check.patch
Type: text/x-patch
Size: 1067 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140918/c90caa05/attachment-0003.bin>


More information about the sssd-devel mailing list