Addition of Rule Checkers

[Sebastian Huber]

On 02/08/2019 03:35, Chris Johns wrote:
> On 30/7/19 4:32 pm, Sebastian Huber wrote:
>> On 30/07/2019 01:01, Joel Sherrill wrote:
>>>
>>>
>>> On Mon, Jul 29, 2019 at 5:23 PM Gedare Bloom <gedare at rtems.org
>>> <mailto:gedare at rtems.org>> wrote:
>>>
>>>      On Wed, Jul 24, 2019 at 7:16 AM Joel Sherrill <joel at rtems.org
>>>      <mailto:joel at rtems.org>> wrote:
>>>       >
>>>       > Hi
>>>       >
>>>       > I just wanted to make sure we followed proper procedures and
>>>      policies when considering rule checkers.
>>>       >
>>>      I guess these comments are germane to the other thread, but I want to
>>>      reflect here a bit.
>>>
>>>       > 1. The license must appropriate.
>>>       >
>>>      Can you expand what you mean by this?
>>>
>>>
>>> It must be a FLOSS tool -- it can't be proprietary. We use Scan but it has
>>> issues.
>>>
>>> It also must be available on the normal set of commonly used hosts.
>>>
>>>
>>>       > 2. There should be some basic requirements that the tool is
>>>      expected to meet to be fit for purpose
>>>       >
>>>       > 3. The tool must support the range of development hosts used in
>>>      the Community.
>>>       >
>>>      Only if we make it a requirement for developers to check these rules
>>>      locally before submitting patches/commits. If we had a server for
>>>      example that could accept submissions from developers, this would be
>>>      irrelevant?
>>>
>>>
>>> If we relax this rule, then we will end up with a one host tool and have to have
>>> a server that runs it. Our infrastructure is FreeBSD so saying it supports the
>>> range
>>> of hosts is likely saying it supports our servers, Linux, MacOS, Cygwin, and
>>> MingW
>>
>> I would not make it a requirement that every user must be able to run the tools
>> on a wide range of platforms. We should clearly identify the tools (e.g.
>> version) and the tool configuration (rules, scope).
> 
> I am not sure I understand what this means, specifically ...
> 
>> It should enable someone
>> else to set up the tool and run it in a configuration used by the RTEMS project.
> 
> What does "set up the tool" mean? Do I need to have a suitable host available
> that runs the tool if my preferred host is not supported?

You don't "have to". Using this stuff should be optional, but it should 
be reproducible.

> 
>> What is the overall goal of this checker stuff? We want that they find bugs for
>> us which we can fix. For the checker tools we have at least to categories: 1.
>> static analysis and 2. coding rule checkers.
> 
> We need to balance this with needing to maintain a low effort bar for patches or
> we will not see them.
> 
>> 1. For the static analysis tools I think it doesn't matter how they are run or
>> by whom. If they find a bug, we fix it.
> 
> Is this a precondition for submission or pushing of a patch? I think you are
> saying it is not but I cannot tell.

I would not make it mandatory to run static analysis tools before you 
submit a patch.

If the project has a patch check pipeline, then this is another thing. 
For example in Doorstop, you submit a pull request, this triggers some 
CI tools, e.g. style checker, documentation checker, static analysis, 
test suite runs and then gives you a passed or failed indication. The 
maintainer will not look at pull requests which are in a failed state.

>> 2. This is a bit different to the old style coding rule checkers. There aim is
>> to write the code with rules so that bugs are prevented in the first place (e.g.
>> MISRA). I think these tools must be freely available.
> 
> Agreed which brings us back to Joel's conditions. I support those as they are
> based on our long history and being bitten before by not being as strict as we
> should have. I am not sure we need to match the host list we support for
> development of RTEMS applications however this discussion become circular.
> 
>> Rule violations will result in patches which don't fix bugs.
> 
> What happens to patches that do not meet the rules but fix a bug? Are the rules
> that full proof?

We already review patches with respect to the coding guidelines. In the 
score at least they were quite strictly followed.

> Is such a patch rejected, ignored, or is it expected a developer sorts this out
> so the patch can be pushed? I am concerned about an increase in the open source
> tax for developers with open commit access.
> 
> We need to maintain workable solution for all the unfunded effort and a bar that
> is suitably low so patches from the community can be posted and accepted.
> Manuel's comment about needing rules that are understandable is important,
> knowing there are rules is also important and then being able to check those
> rules becomes important.

Yes, this is also my point of view.

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber at embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.