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

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Sep 10, 2019, 14:18 UTC
Message-ID: <20190910141828.GK32087@szeder.dev>
In-Reply-To: <f62b0c7d1774cefc66e519430515eeb64acad1e0.1568101393.git.liu.denton@gmail.com>

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.

Show 15 quoted lines

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

Show 5 quoted lines

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