[U-Boot] [STATUS] "Quality" of patches / testing.

[Anton Staaf]

On Tue, Oct 18, 2011 at 7:05 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi,
>
> On Tue, Oct 18, 2011 at 2:44 AM, Wolfgang Denk <wd at denx.de> wrote:
>> Dear Andreas,
>>
>> In message <4E9D4552.5040506 at gmail.com> you wrote:
>>>
>>> We should first get a state of "all boards build clean" for a sort of
>>> toolchains (I think arm is now at this state after a lot of tumult in
>>> the last two releases).
>>
>> PowerPC has traditionally always been build-clean (all boards succeed
>> to build, with very few [2...3] causing harmelss build warnings).  Now
>> ARM is close to that, too.  MIPS has also always been building mostly
>> fine.
>>
>>> This is some kind of CI as suggested by Lukasz Majewski. I also favor CI
>>> for fast feedback to the submitter if his change breaks something.
>>
>> I'm not sure how to do that.  To provide feedback to individual
>> submitters, you would have to runn a full build cycle for each patch
>> we apply.  That would obviously be best, but I don't have machine
>> power to do that.
>>
>> What I do is testing batches - say, after applying 20...30 patches, or
>> after major changes.
>>
>> I have Jenkins running every night for a few selected CPU families,
>> which already catches a number of issues, but even then it's directly
>> pointing to a submitted patch.
>>
>>> I guess it will be doable to have some scripts/prepare-patch which runs
>>>  a) git format-patch
>>>  b) checkpatch on the patch
>>>  c) (configurable subset of) MAKEALL on some clean tree with that patch
>>>     applied
>>>  d) append the results to the patch
>>>
>>> This tool would implement some kind of CI but utilize the computing
>>> power of submitter.
>>
>> Sounds good.  Any takers?
>
> I'm keen to take a look at this, as I have a and b already and a tool
> which munges patches. Also it something that causes me a bit of pain.
>
> I have so far written a tool which automates patch creation based on
> tags in the commits - it works out how many patches are in your branch
> (or you can tell it), creates the patches with format-patch, runs
> patches through checkpatch, its own checks and does a 'git am' on
> each. Then it creates a cover letter and (if the patches are clean)
> optionally does a 'git send-email' on the series. Information is
> collected from tags in the commits (which are removed from the patch).
> Version changes are collected from each commit and added to the cover
> letter, the subject is set correctly, etc.
>
> For example, here is my top commit for branch us-printf:
>
>    Series-to: u-boot
>    Series-cc: wolfgang
>    Cover-letter:
>    Buffer overruns in printf
>    The printf family of functions in U-Boot cannot deal with a situation where
>    the caller provides a buffer which turns out to be too small for the format
>    string. This can result in buffer overflows, stack overflows and other bad
>    behavior.
>
>    This patch series tidies this up in the common vsprintf.c code.
>
>    END
>
>    Series-version: 3
>
>    Series-changes: 2
>    - Use sizeof(printbuffer) instead of CONFIG_SYS_PBSIZE
>    - Drop patch which changes network code to use snprintf()
>
> and another commit has:
>
>    Series-changes: 3
>    - Move prototypes from common.h to vsprintf.h
>
> Other tags are Series-prefix (for 'RFC' for example) and Series-notes
> for notes which should appear at the end of the cover letter. It also
> looks for tags like 'arm:' and 'net:' in the in the subject field and
> CCs the maintainer.
>
> It does enforce a certain workflow, but allows you to easily change
> branches, make a change and create some new patches. It does not
> handle In-reply-to yet.
>
> Anyway, with this and Mike's little script for calling MAKEALL it
> doesn't seem like a huge job to add building to this script. and then
> append the results into the patch.
>
> I have already been wondering how to send this out for review -
> perhaps as something in tools/scripts?
>
> Regads,
> Simon
>

Amusingly (since Simon and I work a few feet apart from each other), I
have something similar that I've written and am using to automate
generation of patches and running of checkpatch.  I'll sync up with him
today and see if it makes sense to merge our efforts and post the result.

-Anton

>>
>> Best regards,
>>
>> Wolfgang Denk
>>
>> --
>> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
>> Drun'? 'm not drun'! You woudn' dare call m' drun' if I was sober!
>>                                     - Terry Pratchett, _Men at Arms_
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>