Re: [PoC] coccinelle: make Coccinelle-related make targets more fine-grained

["René Scharfe" <l.s.r@web.de>]

Am 02.08.2018 um 13:55 schrieb SZEDER Gábor:
> Let's add a bit of Makefile metaprogramming to generate finer-grained
> make targets applying one semantic patch to only a single source file,
> and specify these as dependencies of the targets applying one semantic
> patch to all source files.  This way that shell loop can be avoided,
> semantic patches will only be applied to changed source files, and the
> same semantic patch can be applied in parallel to multiple source
> files.  The only remaining sequential part is aggregating the
> suggested transformations from the individual targets into a single
> patch file, which is comparatively cheap (especially since ideally
> there aren't any suggestions).
> 
> This change brings spectacular speedup in the scenario described in
> point (1) above.  When the results of a previous 'make coccicheck' are
> there, the time needed to run
> 
>    touch apply.c ; time make -j4 coccicheck
> 
> went from 3m42s to 1.848s, which is just over 99% speedup, yay!, and
> 'apply.c' is the second longest source file in our codebase.  In the
> scenario in point (2), running
> 
>    touch contrib/coccinelle/array.cocci ; time make -j4 coccicheck
> 
> went from 56.364s to 35.883s, which is ~36% speedup.

Awesome!

> Unfortunately, this new approach has some disadvantages compared to
> the current situation:
> 
>    - [RFC]
>      With this patch 'make coccicheck's output will look like this
>      ('make' apparently doesn't iterate over semantic patches in
>      lexicographical order):
> 
>        SPATCH commit.cocci              abspath.c
>        SPATCH commit.cocci              advice.c
>        <... lots of lines ...>
>        SPATCH array.cocci               http-walker.c
>        SPATCH array.cocci               remote-curl.c
> 
>      which means that the number of lines in the output grows from
>      "Nr-of-semantic-patches" to "Nr-of-semantic-patches *
>      Nr-of-source-files".

The output is mostly interesting to see if something is happening, to
see if some semantic patch generated a non-empty diff and to get
error details.  We have ca. 400 .c files and 8 .cocci files -- that
means the lines would fill up my scroll-back buffer.  Hmm.

Would it be possible to have a phony target (or directory) per source
file that just prints a line and depends on the individual file+cocci
targets?  (I don't know much make, you probably thought about it
already.)

Or to select one random .cocci file (let's say the first one) and
only print SPATCH lines for this one (without mentioning its name)?
(A dirty hack..)

> ---
>   Makefile                      | 52 ++++++++++++++++++++++++-----------
>   contrib/coccinelle/.gitignore |  3 +-
>   2 files changed, 38 insertions(+), 17 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index d616c04125..f516dd93d1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2674,25 +2674,44 @@ COCCI_SOURCES = $(filter-out sha1collisiondetection/%,$(C_SOURCES))
>   else
>   COCCI_SOURCES = $(filter-out sha1dc/%,$(C_SOURCES))
>   endif
> +COCCI_SEM_PATCHES = $(wildcard contrib/coccinelle/*.cocci)
>   
> -%.cocci.patch: %.cocci $(COCCI_SOURCES)
> -	@echo '    ' SPATCH $<; \
> -	ret=0; \
> -	for f in $(COCCI_SOURCES); do \
> -		$(SPATCH) --sp-file $< $$f $(SPATCH_FLAGS) || \
> -			{ ret=$$?; break; }; \
> -	done >$@+ 2>$@.log; \
> -	if test $$ret != 0; \
> +define cocci_template
> +$(cocci_sem_patch)_dirs := $$(addprefix $(cocci_sem_patch).patches/,$$(sort $$(dir $$(COCCI_SOURCES))))
> +
> +$$($(cocci_sem_patch)_dirs):
> +	@mkdir -p $$@
> +
> +# e.g. 'contrib/coccinelle/strbuf.cocci.patches/builtin/commit.c.patch'
> +# Applies one semantic patch to _one_ source file.
> +$(cocci_sem_patch).patches/%.patch: % $(cocci_sem_patch)
> +	@printf '     SPATCH %-25s %s\n' $$(notdir $(cocci_sem_patch)) $$<; \
> +	if ! $$(SPATCH) --sp-file $(cocci_sem_patch) $$< $$(SPATCH_FLAGS) >$$@ 2>$$@.log; \
>   	then \
> -		cat $@.log; \
> +		rm -f $$@; \
> +		cat $$@.log; \
>   		exit 1; \
> -	fi; \
> -	mv $@+ $@; \
> -	if test -s $@; \
> -	then \
> -		echo '    ' SPATCH result: $@; \
>   	fi
> -coccicheck: $(addsuffix .patch,$(wildcard contrib/coccinelle/*.cocci))
> +
> +# e.g. 'contrib/coccinelle/strbuf.cocci.patch'
> +# Applies one semantic patch to _all_ source files.
> +$(cocci_sem_patch).patch: $(cocci_sem_patch) $$($(cocci_sem_patch)_dirs) $$(patsubst %,$(cocci_sem_patch).patches/%.patch,$(COCCI_SOURCES))

Are the dependencies correct (before and after the patch)?  E.g. are all
semantic patches reapplied after cache.h was changed?  I think we ignore
header files currently, and that becomes more of a problem if the check
becomes more fine-grained.

René