Re: [Cocci] [PATCH v3 1/1] runchecks: Generalize make C={1, 2} to support multiple checkers

[Knut Omang]

On Fri, 2018-01-05 at 16:08 -0200, Mauro Carvalho Chehab wrote:
> Em Thu, 04 Jan 2018 21:15:31 +0100
> Knut Omang  escreveu:
> 
> > > I'm surprised the commit message and the provided documentation say
> > > nothing about using CHECK=foo on the command line. That already supports
> > > arbitrary checkers.   
> > 
> > The problem, highlighted by Jim Davis in
> > 
> > https://lkml.org/lkml/2017/11/20/638
> > 
> > is that the current solution isn't flexible enough - that discussion 
> > is what lead me to this reimplementation of what I originally intended 
> > to be a checkpatch only solution.
> > 
> > > How does this relate to that? Is this supposed to be
> > > a complete replacement? Or what?  
> > 
> > It has evolved into a complete replacement of the intention of CHECK.
> > 
> > > 'make help' also references $CHECK, and this patch doesn't update the
> > > help text.  
> > 
> > I realize now that this needs to be handled in some way due to the way I 
> > split the 
> > arguments with '--' - the intention was to keep it for bw compatibility.
> > 
> > It would be good to know if people rely on using CHECK with C={1,2} for 
> > anything beside the checkers supported by runchecks today
> 
> I do. Here, I use:
> 
> $ make ARCH=i386  CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y C=1 
> W=1
> CHECK='compile_checks' M=drivers/media
> 
> Where "compile_checks" is actually a small script that calls both
> smatch and sparse:
> 
>   #!/bin/bash
>   /devel/smatch/smatch -p=kernel $@

I suppose you here refer to this:
https://blogs.oracle.com/linuxkernel/smatch-static-analysis-tool-overview,-by-dan-carpenter

Good idea! I'll have a look at how that plays with this.

>   /devel/sparse/sparse $@
> 
> So, I'm not sure why we need something else. 

The core functionality is the selective suppression logic and output unification
which makes checking with automated build tools more flexible and 
applicable right away (not when every warning from every checker is fixed...)

> That said, I didn't look
> on its code, but looking on its diffstat:
> 
>  Makefile   |  23 +-
>  scripts/Makefile.build |   4 +-
>  scripts/runchecks  | 734 ++-
>  scripts/runchecks.cfg  |  63 ++-
>  scripts/runchecks_help.txt |  43 ++-
> 
> Using a 734 lines python program just to do an exec on an external checker
> seems too much!

Sure, if that was the case I would be the first to agree :-)

Thanks,
Knut

> Thanks,
> Mauro
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH v3 1/1] runchecks: Generalize make C={1, 2} to support multiple checkers

[Jani Nikula]

On Thu, 04 Jan 2018, Knut Omang  wrote:
> On Thu, 2018-01-04 at 17:50 +0200, Jani Nikula wrote:
>> On Thu, 04 Jan 2018, Knut Omang  wrote:
>> > Add scripts/runchecks which has generic support for running
>> > checker tools in a convenient and user friendly way that
>> > the author hopes can contribute to rein in issues detected
>> > by these tools in a manageable and convenient way.
>> >
>> > scripts/runchecks provides the following basic functionality:
>> >
>> > * Makes it possible to selectively suppress output from individual
>> >   checks on a per file or per subsystem basis.
>> > * Unifies output and suppression input from different tools
>> >   by providing a single unified syntax and presentation for the
>> >   underlying tools in the style of "scripts/checkpatch.pl --show-types".
>> > * Allows selective run of one, or more (or all) configured tools
>> >   for each file.
>> >
>> > In the Makefile system, the sparse specific setup has been replaced
>> > by setup for runchecks.
>> >
>> > This version of runchecks together with a "global" configuration
>> > file in "scripts/runchecks.cfg" supports sparse, checkpatch, and checkdoc,
>> > a trivial abstraction above a call to 'kernel-doc -none'.
>> > It also supports forwarding calls to coccicheck for coccinelle support
>> > but this is not quite as worked through as the three other checkers,
>> > mainly because of lack of error data as all checks pass by default
>> > right now.
>> >
>> > The code is designed to be easily extensible to support more checkers
>> > as they emerge, and some generic checker support is even available
>> > just via simple additions to "scripts/runchecks.cfg".
>> >
>> > The runchecks program unifies configuration, processing
>> > and output for multiple checker tools to make them
>> > all run as part of the C=1 or C=2 option to make.
>> >
>> > Currently with full support and unified behaviour for
>> > sparse: sparse
>> > checkpatch: scripts/checkpatch.pl
>> > checkdoc:   kernel-doc -none
>> >
>> > In principle supported but not unified in output(yet):
>> > coccinelle: scripts/coccicheck
>> >
>> > Introduces a new documentation section titled
>> > "Makefile support for running checkers"
>> >
>> > Also updates documentation for the make C= option
>> > in some other doc files, as the behaviour has
>> > been changed to be less sparse specific and more
>> > generic. The coccinelle documentation also had the
>> > behaviour of C=1 and C=2 swapped.
>> 
>> I'm surprised the commit message and the provided documentation say
>> nothing about using CHECK=foo on the command line. That already supports
>> arbitrary checkers. 
>
> The problem, highlighted by Jim Davis in
>
> https://lkml.org/lkml/2017/11/20/638
>
> is that the current solution isn't flexible enough - that discussion 
> is what lead me to this reimplementation of what I originally intended 
> to be a checkpatch only solution.
>
>> How does this relate to that? Is this supposed to be
>> a complete replacement? Or what?
>
> It has evolved into a complete replacement of the intention of CHECK.
>
>> 'make help' also references $CHECK, and this patch doesn't update the
>> help text.
>
> I realize now that this needs to be handled in some way due to the way I 
> split the 
> arguments with '--' - the intention was to keep it for bw compatibility.
>
> It would be good to know if people rely on using CHECK with C={1,2} for 
> anything beside the checkers supported by runchecks today, if not, 
> it could either be removed or simply replace by an expansion into a 
> '--run:$CHECK'
> argument to runchecks 
>
> Then runchecks' implicit method of declaring 
>
> checker  
>
> in scripts/runchecks.cfg could be used for people with checkers that
> need no further input/output adaptation.
>
> Further suggestions appreciated on this matter.

FWIW, I'd think it would be sufficient the documentation ('make help'
and your runchecks.rst et al) gets updated to reflect the changes to
$CHECK. But up to whoever is the maintainer here.

>
>> > Signed-off-by: Knut Omang 
>> > Reviewed-by: Håkon Bugge 
>> > Reviewed-by: Åsmund Østvold 
>> > Reviewed-by: John Haxby 
>> > ---
>> >  Documentation/dev-tools/coccinelle.rst |  12 +-
>> >  Documentation/dev-tools/index.rst  |   1 +-
>> >  Documentation/dev-tools/runchecks.rst  | 215 -
>> >  Documentation/dev-tools/sparse.rst |  30 +-
>> >  Documentation/kbuild/kbuild.txt|   9 +-
>> >  Makefile   |  23 +-
>> >  scripts/Makefile.build |   4 +-
>> >  scripts/runchecks  | 734 ++-
>> >  scripts/runchecks.cfg  |  63 ++-
>> >  scripts/runchecks_help.txt |  43 ++-
>> 
>> Please get rid of runchecks_help.txt and use the usual python mechanisms
>> to specify and parse command line 

Re: [Cocci] [PATCH v3 1/1] runchecks: Generalize make C={1, 2} to support multiple checkers

[Markus Heiser]

> Am 05.01.2018 um 15:30 schrieb Jani Nikula :
> 
> On Thu, 04 Jan 2018, Knut Omang  wrote:
>> On Thu, 2018-01-04 at 17:50 +0200, Jani Nikula wrote:

[...]

>> Hmm - I have been burnt by the use of unstable interfaces in Python before,
>> when I needed it to work on a (Linux) system with Python v.2.6.x only
>> - argparse was introduced in v.2.7. and alternative choices are not 
>> at all clear to me, see for instance:
>> 
>>  https://dmerej.info/blog/post/docopt-v-argparse/
>> 
>> If this program was part of a "standalone" python project with a well 
>> defined python
>> environment, I would probably have used argparse, which I have used in other 
>> projects.
>> 
>> In fact I hesitated even to use python for this, because of fear of 
>> versioning issues..
>> When I was tempted anyway, and after looking at the existing examples in 
>> scripts/ 
>> ruling out python v.3.x, it felt safer to stay with the bare minimum of 
>> module 
>> features for this simple logic.
>> 
>> I do feel confident that the benefits of python for this outweighs the 
>> drawbacks
>> compared to my initial shell script implementation, or using perl or even C.
>> 
>> Further advice on this appreciated,
> 
> Again, I can only offer my opinion of requiring Python v2.7 and using
> argparse, but it doesn't carry much weight. Up to the kbuild
> maintainers.

FYI: Py2.6 support has ended and Py2 expected to be end in 2020, the
countdown is running ;)

  https://pythonclock.org/

If you wan't a command-line parser which is "builtin" use argparse,
this is what I do mostly. If you are able to use requirements from
outside use click [1]. For more infos read e.g. [2]. Docopt [3] has
its charm but I would prefer the decorators from click.

[1] http://click.pocoo.org/5/
[2] 
https://realpython.com/blog/python/comparing-python-command-line-parsing-libraries-argparse-docopt-click/
[3] https://github.com/docopt

-- Markus --
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [v3] runchecks: Generalize make C={1, 2} to support multiple checkers

[SF Markus Elfring]

> I do feel confident that the benefits of python for this outweighs the 
> drawbacks
> compared to my initial shell script implementation, or using perl or even C.
>
> Further advice on this appreciated,

I got further ideas around this software situation. I am curious on how they fit
to your vision.

One of the challenges for integration of your contribution can also be the
acceptance of corresponding configuration files. There are design choices
available to work with suggested data structures.

How do you think about the develop an official data format specification
for which the shown Python script could be the first reference implementation?
Alternative implementations could be applied on concrete demand.


I would like to add another development concern:

* Do all (Python) classes need to be stored within the same script file so far?

* Do we need to think about structures around plug-ins (or add-ons) already?

Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci