Addition of Rule Checkers

Thu Aug 1 16:14:14 UTC 2019

On Thu, Aug 1, 2019 at 4:47 AM Manuel Coutinho
<Manuel.Coutinho at edisoft.pt> wrote:
>
> Hi all,
>
> My 2 cents.
>
> > -----Original Message-----
> > From: Sebastian Huber [mailto:sebastian.huber at embedded-brains.de]
> > Sent: terça-feira, 30 de julho de 2019 07:33
> > To: rtems-devel at rtems.org
> > Cc: joel at rtems.org; Gedare Bloom; Manuel Coutinho
> > Subject: Re: Addition of Rule Checkers
> >
> > 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). It
> > should enable someone else to set up the tool and run it in a
> > configuration used by the RTEMS project.
> >
> > 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.
> >
> > 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.
> >
> > 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.
> > Rule violations will result in patches which don't fix bugs.
> >
>
> I think Sebastian is right in this regard, find bugs should be good a thing no matter how they are found.
>
I'm also with Sebastian on both points above. For bugs (static
analysis), yes, I think it is totally fine/good to put whatever tools
to work that can be, but we can't allow any proprietary tools to
become part of a "requirement" to maintaining RTEMS. So, for example,
it is well to say we run Coverity on RTEMS, but we should not set any
kind of process that requires us to run it.

For the rule-checking, we must be able to freely produce the checking
reports. It is best if this is done by an open-source means.

> > >
> > >
> > >      > As a detail, once selected the tool needs to have an RSB recipe.
> > >      >
> > >     Only if you require that checkers must be built-from-source.
> > >
> > >
> > > If we can't get source, then they aren't FLOSS and we are depending on
> > > them coming with distributions and being provided for every host we care
> > > about.
> > >
> > > We need something like Phabricator so things can be run against the code
> > > as part of the review/submission process but I want every tool to be
> > > available
> > > to every user.
> >
> > If you set up a project infrastructure which automatically checks
> > patches, then not everyone needs to install the tools used for the
> > automatic checks. My experience is that in projects with good automatic
> > checks in the Git pull pipeline the developers learn to rely on this and
> > stop to check their patches locally. They just wait for the reports from
> > the automatic checkers and if a red flag appears, they get active.
> >
> > >
> > >
> > >      > For style checkers, I am concerned that they are not integrated
> > >     into the normal development and build process. There is already
> > >     enough "garbage collection" to do and making a special pass. If
> > >     that's the case, it will just be a burden.
> > >      >
> > >      > Just some thoughts. I'm not opposed to this, I just don't want
> > >     something like Coverity where it is only run periodically and only a
> > >     few people even look at it much less fix problems.
> > >      >
> > >     I think that the problem of only running periodically and having few
> > >     eyes/fixes are a bit orthogonal to things you pointed out earlier. I
> > >     think the inclusion of rule checkers will be a good thing, but I also
> > >     agree that how it gets done is important for long-term viability.
> > >
> > >
> > > The less available and less open the tool is, the less likely it is that
> > > anyone
> > > will look at it. I guess I think the two issues are tied together.
> >
> > Yes, the first burden is a need to log in to see something. The next
> > burden is if you cannot copy and paste things from the report to discuss
> > things with other developers. Inaccessible documentation and no good
> > support channel is another burden.
> >
>
> Yes, it is a burden. Perhaps with a little care we could choose rules that are understandable just from the title. Then it should be more or less straightforward to understand while the rule was broken in a particular line of code.
> If we choose rules that we don't fully understand then looking at the report (those who can) is the only option available.
> That is, by carefully choosing the rules that are applicable we can minimize misunderstandings.
>
>
> > >
> > > If we can add it to the RSB, then it can be built locally and integrated
> > > by us into
> > > the normal build process. Otherwise, we just become a project which is
> > > hard to
> > > submit code to because we are picky and have our own tools at the end.
> > >
> > > What does Chromium do with their checker? That would be a good
> > reference to
> > > see where on the spectrum a big project falls.
> >
> > Yes, I think we could learn a lot from the Chromium project.
> >
I haven't looked at this yet.

> > --
> > 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.