Re: [PATCH 2/2] Makefile: run coccicheck on more source files
From: SZEDER Gábor <hidden>
Date: 2019-09-10 14:18:34
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.
quoted hunk ↗ jump to hunk
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