[PATCH v3 00/11] cocci: make "incremental" possible + a ccache-like tool
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Oct 14, 2022, 15:31 UTC
- Message-ID
- <cover-v3-00.11-00000000000-20221014T152552Z-avarab@gmail.com>
- In-Reply-To
- <cover-v2-0.9-00000000000-20220831T205130Z-avarab@gmail.com>
A re-roll of the series to have "make coccicheck" run incrementally. For the v1 see: https://lore.kernel.org/git/cover-0.5-00000000000-20220825T141212Z-avarab@gmail.com/
The big change in this belated v3 is that we now run against a concatenated version of all our *.cocci files. This is something I discussed with Jeff King at Git Merge, after having confirmed the viability of that approach on the cocci mailing list.
The result is that even with a from-scratch run of "make coccicheck" this is faster than on "master", while of course adding the ability to run incrementally, so a subsequent "coccicheck" is only run against what we'd recompile.
The churn here is because I had to move/change some variables around early in the series to make the eventual 08/11 smaller.
There's also improvements here to the bundled "spatchcache", it can now optionally interoperate with non--very-quiet output (but won't cache stderr), which is handy for debugging it. It can also be configured to tell it where the .depend/* files are, which is a small change to make it posible to use it outside of git.git..
Ævar Arnfjörð Bjarmason (11): Makefile + shared.mak: rename and indent $(QUIET_SPATCH_T) cocci rules: remove unused "F" metavariable from pending rule Makefile: add ability to TAB-complete cocci *.patch rules Makefile: have "coccicheck" re-run if flags change Makefile: split off SPATCH_BATCH_SIZE comment from "cocci" heading cocci: split off include-less "tests" from SPATCH_FLAGS cocci: split off "--all-includes" from SPATCH_FLAGS cocci: make "coccicheck" rule incremental cocci: optimistically use COMPUTE_HEADER_DEPENDENCIES cocci: run against a generated ALL.cocci spatchcache: add a ccache-alike for "spatch"
.gitignore | 1 + Makefile | 159 ++++++++-- contrib/coccinelle/.gitignore | 1 + contrib/coccinelle/spatchcache | 272 ++++++++++++++++++ .../coccinelle/the_repository.pending.cocci | 1 - shared.mak | 10 +- 6 files changed, 416 insertions(+), 28 deletions(-) create mode 100755 contrib/coccinelle/spatchcache
Range-diff against v2:
-: ----------- > 1: 4494c91df9a Makefile + shared.mak: rename and indent $(QUIET_SPATCH_T)
1: 72b6a8e4e0b = 2: 8219b1b12f2 cocci rules: remove unused "F" metavariable from pending rule
2: 0998948b881 ! 3: 6dbfafa08fd Makefile: add ability to TAB-complete cocci *.patch rules
@@ Makefile: check: $(GENERATED_H)
exit 1; \
fi
-+COCCI = $(wildcard contrib/coccinelle/*.cocci)
-+COCCI_PATCHES = $(addsuffix .patch,$(COCCI))
-+COCCICHECK_PENDING = $(filter %.pending.cocci.patch,$(COCCI_PATCHES))
-+COCCICHECK = $(filter-out $(COCCICHECK_PENDING),$(COCCI_PATCHES))
++COCCI_GLOB = $(wildcard contrib/coccinelle/*.cocci)
++COCCI_RULES = $(COCCI_GLOB)
++
++COCCICHECK_PENDING = $(filter %.pending.cocci,$(COCCI_RULES))
++COCCICHECK = $(filter-out $(COCCICHECK_PENDING),$(COCCI_RULES))
++
++COCCICHECK_PATCHES = $(COCCICHECK:%=%.patch)
++COCCICHECK_PATCHES_PENDING = $(COCCICHECK_PENDING:%=%.patch)
+
COCCI_TEST_RES = $(wildcard contrib/coccinelle/tests/*.res)
-%.cocci.patch: %.cocci $(COCCI_SOURCES)
++COCCI_PATCHES = $(COCCI_RULES:%=%.patch)
+$(COCCI_PATCHES): $(COCCI_SOURCES)
+$(COCCI_PATCHES): %.patch: %
$(QUIET_SPATCH) \
@@ Makefile: $(COCCI_TEST_RES_GEN): .build/contrib/coccinelle/tests/%.res : contrib
coccicheck: coccicheck-test
-coccicheck: $(addsuffix .patch,$(filter-out %.pending.cocci,$(wildcard contrib/coccinelle/*.cocci)))
-+coccicheck: $(COCCICHECK)
++coccicheck: $(COCCICHECK_PATCHES)
# See contrib/coccinelle/README
coccicheck-pending: coccicheck-test
-coccicheck-pending: $(addsuffix .patch,$(wildcard contrib/coccinelle/*.pending.cocci))
-+coccicheck-pending: $(COCCICHECK_PENDING)
++coccicheck-pending: $(COCCICHECK_PATCHES_PENDING)
.PHONY: coccicheck coccicheck-pending
-@@ Makefile: profile-clean:
-
- cocciclean:
- $(RM) -r .build/contrib/coccinelle
-- $(RM) contrib/coccinelle/*.cocci.patch*
-+ $(RM) $(COCCI_PATCHES)*
-
- clean: profile-clean coverage-clean cocciclean
- $(RM) -r .build
3: 63cf9f58d99 ! 4: f779a2d22aa Makefile: have "coccicheck" re-run if flags change
@@ Makefile: SANITIZE_ADDRESS =
include config.mak.uname
-include config.mak.autogen
-include config.mak
-@@ Makefile: COCCICHECK = $(filter-out $(COCCICHECK_PENDING),$(COCCI_PATCHES))
-
+@@ Makefile: COCCICHECK_PATCHES_PENDING = $(COCCICHECK_PENDING:%=%.patch)
COCCI_TEST_RES = $(wildcard contrib/coccinelle/tests/*.res)
+ COCCI_PATCHES = $(COCCI_RULES:%=%.patch)
+$(COCCI_PATCHES): GIT-SPATCH-DEFINES
$(COCCI_PATCHES): $(COCCI_SOURCES)
$(COCCI_PATCHES): %.patch: %
@@ Makefile: $(COCCI_PATCHES): %.patch: %
$(COCCI_TEST_RES_GEN): .build/%.res : %.c
$(COCCI_TEST_RES_GEN): .build/%.res : %.res
$(COCCI_TEST_RES_GEN): .build/contrib/coccinelle/tests/%.res : contrib/coccinelle/%.cocci
+@@ Makefile: profile-clean:
+ $(RM) $(addsuffix *.gcno,$(addprefix $(PROFILE_DIR)/, $(object_dirs)))
+
+ cocciclean:
++ $(RM) GIT-SPATCH-DEFINES
+ $(RM) -r .build/contrib/coccinelle
+ $(RM) contrib/coccinelle/*.cocci.patch*
+
4: 54d6bae3984 = 5: ab25b586f38 Makefile: split off SPATCH_BATCH_SIZE comment from "cocci" heading
5: 4f165bf6128 ! 6: 691be73b6fb cocci: split off include-less "tests" from SPATCH_FLAGS
@@ Makefile: $(COCCI_TEST_RES_GEN): .build/%.res : %.c
$(COCCI_TEST_RES_GEN): .build/%.res : %.res
$(COCCI_TEST_RES_GEN): .build/contrib/coccinelle/tests/%.res : contrib/coccinelle/%.cocci
$(call mkdir_p_parent_template)
-- $(QUIET_SPATCH_T)$(SPATCH) $(SPATCH_FLAGS) \
-+ $(QUIET_SPATCH_T)$(SPATCH) $(SPATCH_TEST_FLAGS) \
+- $(QUIET_SPATCH_TEST)$(SPATCH) $(SPATCH_FLAGS) \
++ $(QUIET_SPATCH_TEST)$(SPATCH) $(SPATCH_TEST_FLAGS) \
--very-quiet --no-show-diff \
--sp-file $< -o $@ \
$(@:.build/%.res=%.c) && \
6: c74d09f4825 = 7: 2ca5ea5beca cocci: split off "--all-includes" from SPATCH_FLAGS
7: 120607b5da6 ! 8: 2072a508064 cocci: make "coccicheck" rule incremental
@@ Commit message
This could be mitigated by combining "make -jN" with
SPATCH_BATCH_SIZE, see 960154b9c17 (coccicheck: optionally batch
- spatch invocations, 2019-05-06). But doing so required careful
- juggling, as e.g. setting both to 4 would yield 16 workers.
+ spatch invocations, 2019-05-06).
There will be cases where getting rid of "SPATCH_BATCH_SIZE" makes
things worse, but a from-scratch "make coccicheck" with the default
@@ Commit message
setting that makes sense when doing a non-incremental run of "make
coccicheck".
+ * Before the "make coccicheck" rule would have to clean
+ "contrib/coccinelle/*.cocci.patch*", since we'd create "*+" and
+ "*.log" files there. Now those are created in
+ .build/contrib/coccinelle/, which is covered by the "cocciclean" rule
+ already.
+
Outstanding issues & future work:
* We could get rid of "--all-includes" in favor of manually
@@ Makefile: TRACK_SPATCH_DEFINES += $(SPATCH)
@FLAGS='$(TRACK_SPATCH_DEFINES)'; \
if test x"$$FLAGS" != x"`cat GIT-SPATCH-DEFINES 2>/dev/null`" ; then \
@@ Makefile: check: $(GENERATED_H)
- exit 1; \
- fi
+ COCCI_GLOB = $(wildcard contrib/coccinelle/*.cocci)
+ COCCI_RULES = $(COCCI_GLOB)
++COCCI_NAMES = $(COCCI_RULES:contrib/coccinelle/%.cocci=%)
+
+ COCCICHECK_PENDING = $(filter %.pending.cocci,$(COCCI_RULES))
++COCCICHECK_PATCHES_PENDING = $(COCCICHECK_PENDING:%=%.patch)
+ COCCICHECK = $(filter-out $(COCCICHECK_PENDING),$(COCCI_RULES))
++COCCICHECK_PATCHES = $(COCCICHECK:%=%.patch)
++
+# It's expensive to compute the many=many rules below, only eval them
+# on $(MAKECMDGOALS) that match these $(COCCI_RULES)
-+COCCI_RULES =
-+COCCI_RULES += cocci%
-+COCCI_RULES += contrib/coccinelle/%
-+COCCI_RULES += .build/contrib/coccinelle/%
-+
- COCCI = $(wildcard contrib/coccinelle/*.cocci)
-+COCCI_NAMES = $(COCCI:contrib/coccinelle/%.cocci=%)
- COCCI_PATCHES = $(addsuffix .patch,$(COCCI))
- COCCICHECK_PENDING = $(filter %.pending.cocci.patch,$(COCCI_PATCHES))
--COCCICHECK = $(filter-out $(COCCICHECK_PENDING),$(COCCI_PATCHES))
++COCCI_RULES_GLOB =
++COCCI_RULES_GLOB += cocci%
++COCCI_RULES_GLOB += .build/contrib/coccinelle/%
++COCCI_RULES_GLOB += $(COCCICHECK_PATCHES)
++COCCI_RULES_GLOB += $(COCCICHEC_PATCHES_PENDING)
++COCCI_GOALS = $(filter $(COCCI_RULES_GLOB),$(MAKECMDGOALS))
+
+ COCCICHECK_PATCHES = $(COCCICHECK:%=%.patch)
+ COCCICHECK_PATCHES_PENDING = $(COCCICHECK_PENDING:%=%.patch)
-+COCCICHECK = $(filter-out $(COCCICHECK_PENDING),$(COCCI_PATCHES))
COCCI_TEST_RES = $(wildcard contrib/coccinelle/tests/*.res)
+-COCCI_PATCHES = $(COCCI_RULES:%=%.patch)
-$(COCCI_PATCHES): GIT-SPATCH-DEFINES
-$(COCCI_PATCHES): $(COCCI_SOURCES)
-$(COCCI_PATCHES): %.patch: %
@@ Makefile: check: $(GENERATED_H)
+ fi
+endef
+
-+define cocci-matrix-2
++define cocci-matrix
+
-+$(foreach s,$(COCCI_SOURCES),$(call cocci-rule,$(c),$(s)))
-+endef
-+define cocci-matrix-1
-+$(foreach c,$(COCCI),$(call cocci-matrix-2,$(c)))
++$(foreach s,$(COCCI_SOURCES),$(call cocci-rule,$(1),$(s)))
+endef
+
-+ifneq ($(filter $(COCCI_RULES),$(MAKECMDGOALS)),)
-+$(eval $(call cocci-matrix-1))
++ifdef COCCI_GOALS
++$(eval $(foreach c,$(COCCI_RULES),$(call cocci-matrix,$(c))))
+endif
+
+define spatch-rule
+
+contrib/coccinelle/$(1).cocci.patch: $$(COCCI_$(1))
-+ $(QUIET_SPATCH_M)cat $$^ >$$@ && \
++ $(QUIET_SPATCH_CAT_TMPL)cat $$^ >$$@ && \
+ if test -s $$@; \
then \
- echo ' ' SPATCH result: $@; \
@@ Makefile: check: $(GENERATED_H)
fi
+endef
+
-+ifneq ($(filter $(COCCI_RULES),$(MAKECMDGOALS)),)
++ifdef COCCI_GOALS
+$(eval $(foreach n,$(COCCI_NAMES),$(call spatch-rule,$(n))))
+endif
COCCI_TEST_RES_GEN = $(addprefix .build/,$(COCCI_TEST_RES))
$(COCCI_TEST_RES_GEN): GIT-SPATCH-DEFINES
@@ Makefile: profile-clean:
-
cocciclean:
+ $(RM) GIT-SPATCH-DEFINES
$(RM) -r .build/contrib/coccinelle
-- $(RM) $(COCCI_PATCHES)*
-+ $(RM) $(COCCI_PATCHES)
+- $(RM) contrib/coccinelle/*.cocci.patch*
++ $(RM) $(COCCICHECK_PATCHES)
++ $(RM) $(COCCICHECK_PATCHES_PENDING)
clean: profile-clean coverage-clean cocciclean
$(RM) -r .build
## shared.mak ##
@@ shared.mak: ifndef V
- QUIET_SP = @echo ' ' SP $<;
- QUIET_HDR = @echo ' ' HDR $(<:hcc=h);
QUIET_RC = @echo ' ' RC $@;
-- QUIET_SPATCH = @echo ' ' SPATCH $<;
-+ QUIET_SPATCH = @echo ' ' SPATCH $@;
- QUIET_SPATCH_T = @echo ' ' SPATCH TEST $(@:.build/%=%);
-+ QUIET_SPATCH_M = @echo ' ' SPATCH MERGE $$@;
+ ## Used in "Makefile": SPATCH
+- QUIET_SPATCH = @echo ' ' SPATCH $<;
++ QUIET_SPATCH = @echo ' ' SPATCH $@;
+ QUIET_SPATCH_TEST = @echo ' ' SPATCH TEST $(@:.build/%=%);
+
++## Used in "Makefile": SPATCH_*TMPL (quoted for use in "define"'s)
++ QUIET_SPATCH_CAT_TMPL = @echo ' ' SPATCH CAT $$$$^ \>$$@;
++
## Used in "Documentation/Makefile"
QUIET_ASCIIDOC = @echo ' ' ASCIIDOC $@;
+ QUIET_XMLTO = @echo ' ' XMLTO $@;
8: 536dce45eef ! 9: 739652eada9 cocci: optimistically use COMPUTE_HEADER_DEPENDENCIES
@@ Commit message
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## Makefile ##
+@@ Makefile: SPATCH_INCLUDE_FLAGS = --all-includes
+ SPATCH_FLAGS =
+ SPATCH_TEST_FLAGS =
+
++# If *.o files are present, have "coccicheck" depend on them, with
++# COMPUTE_HEADER_DEPENDENCIES this will speed up the common-case of
++# only needing to re-generate coccicheck results for the users of a
++# given API if it's changed, and not all files in the project. If
++# COMPUTE_HEADER_DEPENDENCIES=no this will be unset too.
++SPATCH_USE_O_DEPENDENCIES = YesPlease
++
+ # Rebuild 'coccicheck' if $(SPATCH), its flags etc. change
+ TRACK_SPATCH_DEFINES =
+ TRACK_SPATCH_DEFINES += $(SPATCH)
@@ Makefile: COCCI_TEST_RES = $(wildcard contrib/coccinelle/tests/*.res)
$(call mkdir_p_parent_template)
$(QUIET_GEN) >$@
-+SPATCH_USE_O_DEPENDENCIES = yes
+ifeq ($(COMPUTE_HEADER_DEPENDENCIES),no)
+SPATCH_USE_O_DEPENDENCIES =
+endif
-+
define cocci-rule
## Rule for .build/$(1).patch/$(2); Params:
@@ Makefile: COCCI_TEST_RES = $(wildcard contrib/coccinelle/tests/*.res)
$$(call mkdir_p_parent_template)
@@ Makefile: endef
- define cocci-matrix-2
+ define cocci-matrix
--$(foreach s,$(COCCI_SOURCES),$(call cocci-rule,$(c),$(s)))
+-$(foreach s,$(COCCI_SOURCES),$(call cocci-rule,$(1),$(s)))
+$(foreach s,$(COCCI_SOURCES),$(call cocci-rule,$(c),$(s),$(s:%.c=%.o)))
endef
- define cocci-matrix-1
- $(foreach c,$(COCCI),$(call cocci-matrix-2,$(c)))
+
+ ifdef COCCI_GOALS
-: ----------- > 10: 52177ea2a68 cocci: run against a generated ALL.cocci
9: 2b978676a56 ! 11: f7ca3f9c9af spatchcache: add a ccache-alike for "spatch"
@@ contrib/coccinelle/spatchcache (new)
+#
+# Note that the "--very-quiet" flag is currently critical. The cache
+# will refuse to cache anything that has output on STDERR (which might
-+# be errors from spatch). The STDERR (and exit code) could in
-+# principle be cached (as with ccache), but then the simple structure
-+# in the Redis cache would need to change, so just supply
-+# "--very-quiet" for now.
++# be errors from spatch), but see spatchCache.cacheWhenStderr below.
++#
++# The STDERR (and exit code) could in principle be cached (as with
++# ccache), but then the simple structure in the Redis cache would need
++# to change, so just supply "--very-quiet" for now.
+#
+# To use this, simply set SPATCH to
+# contrib/coccinelle/spatchcache. Then optionally set:
@@ contrib/coccinelle/spatchcache (new)
+#
+# As well as this trace config (debug implies trace):
+#
++# cacheWhenStderr = true
+# trace = false
+# debug = false
+#
++# The ".depend/grep.o.d" can also be customized, as a string that will
++# be eval'd, it has access to a "$dirname" and "$basename":
++#
++# [spatchCache]
++# dependFormat = "$dirname/.depend/${basename%.c}.o.d"
++#
+# Setting "trace" to "true" allows for seeing when we have a cache HIT
+# or MISS. To debug whether the cache is working do that, and run e.g.:
+#
@@ contrib/coccinelle/spatchcache (new)
+#
+# A subsequent "make cocciclean && make coccicheck" should then have
+# all "HIT"'s and "CANTCACHE"'s.
++#
++# The "spatchCache.cacheWhenStderr" option is critical when using
++# spatchCache.{trace,debug} to debug whether something is set in the
++# cache, as we'll write to the spatch logs in .build/* we'd otherwise
++# always emit a NOCACHE.
+
+set -e
+
@@ contrib/coccinelle/spatchcache (new)
+ trace=true
+fi
+
++cacheWhenStderr=$(git config --bool "spatchCache.cacheWhenStderr")
++if test "$cacheWhenStderr" != "true"
++then
++ cacheWhenStderr=
++fi
++
+trace_it () {
+ if test -z "$trace"
+ then
@@ contrib/coccinelle/spatchcache (new)
+ spatch=spatch
+fi
+
++dependFormat='$dirname/.depend/${basename%.c}.o.d'
++dependFormatCfg=$(git config "spatchCache.dependFormat" || :)
++if test -n "$dependFormatCfg"
++then
++ dependFormat="$dependFormatCfg"
++fi
++
+set=$(git config spatchCache.setCmd || :)
+get=$(git config spatchCache.getCmd || :)
+
@@ contrib/coccinelle/spatchcache (new)
+
+hash_for_cache() {
+ # Parameters that should affect the cache
-+ echo "spatch=$spatch"
+ echo "args=$args"
++ echo "config spatchCache.spatch=$spatch"
++ echo "config spatchCache.debug=$debug"
++ echo "config spatchCache.trace=$trace"
++ echo "config spatchCache.cacheWhenStderr=$cacheWhenStderr"
+ echo
+
+ # Our target file and its dependencies
@@ contrib/coccinelle/spatchcache (new)
+fi
+
+# Main logic
-+d=$(dirname "$arg_file")
-+b=$(basename "$arg_file")
-+bwoext="${b%.c}"
-+dep="$d/.depend/$bwoext.o.d"
++dirname=$(dirname "$arg_file")
++basename=$(basename "$arg_file")
++eval "dep=$dependFormat"
+
+if ! test -f "$dep"
+then
@@ contrib/coccinelle/spatchcache (new)
+err="$(mktemp)"
+
+set +e
-+"$spatch" "$@" >"$out" 2>"$err"
++"$spatch" "$@" >"$out" 2>>"$err"
+ret=$?
++cat "$out"
++cat "$err" >&2
+set -e
+
-+if test $ret = 0 && ! test -s "$out" && ! test -s "$err"
++nocache=
++if test $ret != 0
++then
++ nocache="exited non-zero: $ret"
++elif test -s "$out"
++then
++ nocache="had patch output"
++elif test -z "$cacheWhenStderr" && test -s "$err"
++then
++ nocache="had stderr (use --very-quiet or spatchCache.cacheWhenStderr=true?)"
++fi
++
++if test -n "$nocache"
+then
-+ rm -f "$out" "$err"
++ trace_it "$0: NOCACHE ($nocache): for '$arg_file' with '$arg_sp'"
++ exit "$ret"
++fi
+
-+ trace_it "$0: SET: for '$arg_file' with '$arg_sp'"
++trace_it "$0: SET: for '$arg_file' with '$arg_sp'"
+
-+ setret=
-+ if test -z "$set"
++setret=
++if test -z "$set"
++then
++ if test $(redis-cli SADD spatch-cache "$sum") = 1
+ then
-+ if test $(redis-cli SADD spatch-cache "$sum") = 1
-+ then
-+ setret=0
-+ else
-+ setret=1
-+ fi
++ setret=0
+ else
-+ "$set" "$sum"
-+ setret=$?
-+ fi
-+
-+ if test "$setret" != 0
-+ then
-+ echo "FAILED to set '$sum' in cache!" >&2
-+ exit 128
++ setret=1
+ fi
+else
-+ trace_it "$0: NOCACHE (have changes): for '$arg_file' with '$arg_sp'"
-+ cat "$out"
-+ cat "$err" >&2
-+ rm -f "$out" "$err"
-+ exit "$ret"
++ "$set" "$sum"
++ setret=$?
+fi
-+rm -f "$out" "$err"
++
++if test "$setret" != 0
++then
++ echo "FAILED to set '$sum' in cache!" >&2
++ exit 128
++fi
++
++exit "$ret"-- 2.38.0.1092.g8c0298861b0