Subject: Re: [PATCH 2/2] Makefile: run coccicheck on more source files

From: SZEDER Gábor <[email-masked]>
Date: Tue, 10 Sep 2019 16:18:28 +0200

On Tue, Sep 10, 2019 at 12:44:31AM -0700, Denton Liu wrote:
> Make the "coccicheck" target run on all C sources except for those that
> are taken from some upstream. We don't want to patch these files since
> we want them to be as close to upstream as possible so that it'll be
> easier to pull in upstream updates.

> diff --git a/Makefile b/Makefile
> index 708df2c289..d468b7c9c4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2802,12 +2802,8 @@ check: command-list.h
>  		exit 1; \
>  	fi
>  
> -C_SOURCES = $(patsubst %.o,%.c,$(C_OBJ))
> -ifdef DC_SHA1_SUBMODULE
> -COCCI_SOURCES = $(filter-out sha1collisiondetection/%,$(C_SOURCES))
> -else
> -COCCI_SOURCES = $(filter-out sha1dc/%,$(C_SOURCES))
> -endif
> +FIND_C_SOURCES = $(filter %.c,$(shell $(FIND_SOURCE_FILES)))

Hrm, so this uses FIND_SOURCE_FILES, which first attempts to run 'git
ls-files' and if that fails it falls back to run 'find'.

Unfortunately, the output of the two slightly differ: 'git ls-files'
prints 'abspath.c advice.c alias.c ...' why 'find' prints
'./upload-pack.c ./unpack-trees.c ./gpg-interface.c ...'.  Now, while
the order of files doesn't matter, the './' prefix does, because:

> +COCCI_SOURCES = $(filter-out $(UPSTREAM_SOURCES),$(FIND_C_SOURCES))

Here the paths/patterns in UPSTREAM_SOURCES don't have that './'
prefix, and thus won't match and won't filter out any of the upstream
files that they are supposed to.  IOW, if someone runs 'make
coccicheck' on a system without Git installed, then Coccinelle will
check all upstream sources as well, and will e.g. suggest using
COPY_ARRAY in 'compat/regex/regexec.c'.

Now, running 'make coccicheck' on a git.git clone without Git
installed might look like quite a pathological case on the first
sight, but I would argue that it is not that pathological: e.g.
consider someone running the recent Coccinelle version in a small-ish
Docker image containing just enough to run 'make coccicheck', but not
Git.  (yeah, you guessed right, I am that someone ;)

I don't know how to convince 'find' to omit that './' prefix from each
listed file, and in a portable way at that.  Piping its output through
'sed' or even 'cut' easily takes care of it, though.

>  %.cocci.patch: %.cocci $(COCCI_SOURCES)
>  	@echo '    ' SPATCH $<; \
> -- 
> 2.23.0.248.g3a9dd8fb08
>