Improve the incremental rebuilding support of "coccicheck" by
piggy-backing on the computed dependency information of the
corresponding *.o file, rather than rebuilding all <RULE>/<FILE> pairs
if either their corresponding file changes, or if any header changes.
This in effect uses the same method that the "sparse" target was made
to use in c234e8a0ecf (Makefile: make the "sparse" target non-.PHONY,
2021-09-23), except that the dependency on the *.o file isn't a hard
one, we check with $(wildcard) if the *.o file exists, and if so we'll
depend on it.
This means that the common case of:
make
make coccicheck
Will benefit from incremental rebuilding, now changing e.g. a header
will only re-run "spatch" on those those *.c files that make use of
it:
By depending on the *.o we piggy-back on
COMPUTE_HEADER_DEPENDENCIES. See c234e8a0ecf (Makefile: make the
"sparse" target non-.PHONY, 2021-09-23) for prior art of doing that
for the *.sp files. E.g.:
make contrib/coccinelle/free.cocci.patch
make -W column.h contrib/coccinelle/free.cocci.patch
Will take around 15 seconds for the second command on my 8 core box if
I didn't run "make" beforehand to create the *.o files. But around 2
seconds if I did and we have those "*.o" files.
Notes about the approach of piggy-backing on *.o for dependencies:
* It *is* a trade-off since we'll pay the extra cost of running the C
compiler, but we're probably doing that anyway. The compiler is much
faster than "spatch", so even though we need to re-compile the *.o to
create the dependency info for the *.c for "spatch" it's
faster (especially if using "ccache").
* There *are* use-cases where some would like to have *.o files
around, but to have the "make coccicheck" ignore them. See:
https://lore.kernel.org/git/20220826104312.GJ1735@xxxxxxxxxx/
For those users a:
make
make coccicheck SPATCH_USE_O_DEPENDENCIES=
Will avoid considering the *.o files.
* If that *.o file doesn't exist we'll depend on an intermediate file
of ours which in turn depends on $(FOUND_H_SOURCES).
This covers both an initial build, or where "coccicheck" is run
without running "all" beforehand, and because we run "coccicheck"
on e.g. files in compat/* that we don't know how to build unless
the requisite flag was provided to the Makefile.
Most of the runtime of "incremental" runs is now spent on various
compat/* files, i.e. we conditionally add files to COMPAT_OBJS, and
therefore conflate whether we *can* compile an object and generate
dependency information for it with whether we'd like to link it
into our binary.
Before this change the distinction didn't matter, but now one way
to make this even faster on incremental builds would be to peel
those concerns apart so that we can see that e.g. compat/mmap.c
doesn't depend on column.h.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
---
Makefile | 15 +++++++++++++--
contrib/coccinelle/README | 16 ++++++++++++++++
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index 7cdbd050339..531077031d8 100644
--- a/Makefile
+++ b/Makefile
@@ -1304,6 +1304,13 @@ 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)
@@ -3179,14 +3186,18 @@ COCCI_TEST_RES = $(wildcard contrib/coccinelle/tests/*.res)
$(call mkdir_p_parent_template)
$(QUIET_GEN) >$@
+ifeq ($(COMPUTE_HEADER_DEPENDENCIES),no)
+SPATCH_USE_O_DEPENDENCIES =
+endif
define cocci-rule
## Rule for .build/$(1).patch/$(2); Params:
# $(1) = e.g. "free.cocci"
# $(2) = e.g. "grep.c"
+# $(3) = e.g. "grep.o"
COCCI_$(1:contrib/coccinelle/%.cocci=%) += .build/$(1).patch/$(2)
.build/$(1).patch/$(2): GIT-SPATCH-DEFINES
-.build/$(1).patch/$(2): .build/contrib/coccinelle/FOUND_H_SOURCES
+.build/$(1).patch/$(2): $(if $(and $(SPATCH_USE_O_DEPENDENCIES),$(wildcard $(3))),$(3),.build/contrib/coccinelle/FOUND_H_SOURCES)
.build/$(1).patch/$(2): $(1)
.build/$(1).patch/$(2): .build/$(1).patch/% : %
$$(call mkdir_p_parent_template)
@@ -3203,7 +3214,7 @@ endef
define cocci-matrix
-$(foreach s,$(COCCI_SOURCES),$(call cocci-rule,$(1),$(s)))
+$(foreach s,$(COCCI_SOURCES),$(call cocci-rule,$(c),$(s),$(s:%.c=%.o)))
endef
ifdef COCCI_GOALS
diff --git a/contrib/coccinelle/README b/contrib/coccinelle/README
index f0e80bd7f03..09ea8298e19 100644
--- a/contrib/coccinelle/README
+++ b/contrib/coccinelle/README
@@ -41,3 +41,19 @@ There are two types of semantic patches:
This allows to expose plans of pending large scale refactorings without
impacting the bad pattern checks.
+
+Git-specific tips & things to know about how we run "spatch":
+
+ * The "make coccicheck" will piggy-back on
+ "COMPUTE_HEADER_DEPENDENCIES". If you've built a given object file
+ the "coccicheck" target will consider its depednency to decide if
+ it needs to re-run on the corresponding source file.
+
+ This means that a "make coccicheck" will re-compile object files
+ before running. This might be unexpected, but speeds up the run in
+ the common case, as e.g. a change to "column.h" won't require all
+ coccinelle rules to be re-run against "grep.c" (or another file
+ that happens not to use "column.h").
+
+ To disable this behavior use the "SPATCH_USE_O_DEPENDENCIES=NoThanks"
+ flag.
--
2.38.0.1251.g3eefdfb5e7a
