patch, 5 partscocci: make "incremental" possible + a ccache-like tool

This series makes the "coccicheck" rule incremental by only re-running "spatch" for those file/rule combinations that changed.

The result is that e.g. running "make coccicheck" followed by a:

	make -W grep.c coccicheck

Will take around 2s instead of a *lot* longer. The 3/5 change has all the details about this, and why it's safe.

Furthermore we'll properly handle re-building if any of the required headers change (e.g. column.h).

I had a previous stab at trying to make "coccicheck" faster a while ago in [1], but that was fatally flawed because it would break in cases where grep.c e.g. dependen on a header that changed.

This series doesn't suffer from that issue, because we piggy-back on the corresponding .depend/* files to extract dependency info. This does mean that the incremental speed-ups depend on COMPUTE_HEADER_DEPENDENCIES=yes (which is the case with "gcc" and "clang" by default).

I've been running with this series locally for a while. I think it should fully solve the lamentations about the coccicheck being too slow (e.g. in [2]), particularly when paired with the new "spatchcache" helper added in 5/5.

Even without that helper running "make coccicheck" e.g. after applying a patch series now becomes viable, provided the patches don't modify one of the "rebuild everything" files we have (such as cache.h and git-compat-util.h).

1. https://lore.kernel.org/git/20210302205103.12230-1-avarab@gmail.com/
2. https://lore.kernel.org/git/xmqqh7249b8d.fsf@gitster.g/

Ævar Arnfjörð Bjarmason (5):
  Makefile: add ability to TAB-complete cocci *.patch rules
  Makefile: have "coccicheck" re-run if flags change
  cocci: make "coccicheck" rule incremental
  cocci: make incremental compilation even faster
  spatchcache: add a ccache-alike for "spatch"

 .gitignore                                    |   1 +
 Makefile                                      | 114 +++++++--
 contrib/coccinelle/spatchcache                | 217 ++++++++++++++++++
 .../coccinelle/the_repository.pending.cocci   |   1 -
 shared.mak                                    |   4 +-
 5 files changed, 310 insertions(+), 27 deletions(-)
 create mode 100755 contrib/coccinelle/spatchcache

Declare the contrib/coccinelle/<rule>.cocci.patch rules in such a way as to allow TAB-completion, and slightly optimize the Makefile by cutting down on the number of $(wildcard) in favor of defining "coccicheck" and "coccicheck-pending" in terms of the same incrementally filtered list.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

Fix an issue with the "coccicheck" family of rules that's been here since 63f0a758a06 (add coccicheck make target, 2016-09-15), unlike e.g. "make grep.o" we wouldn't re-run it when $(SPATCH) or $(SPATCH_FLAGS) changed. To test new flags we needed to first do a "make clean".

This now uses the same (copy/pasted) pattern as other "DEFINES" rules. As a result we'll re-run properly. This can be demonstrated e.g. on the issue noted in [1]:

	$ make contrib/coccinelle/xcalloc.cocci.patch COCCI_SOURCES=promisor-remote.c V=1
	[...]
	    SPATCH contrib/coccinelle/xcalloc.cocci
	$ make contrib/coccinelle/xcalloc.cocci.patch COCCI_SOURCES=promisor-remote.c SPATCH_FLAGS="--all-includes --recursive-includes"
	    * new spatch flags
	    SPATCH contrib/coccinelle/xcalloc.cocci
	     SPATCH result: contrib/coccinelle/xcalloc.cocci.patch
	$

1. https://lore.kernel.org/git/20220823095602.GC1735@szeder.dev/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .gitignore |  1 +
 Makefile   | 14 ++++++++++++++
 2 files changed, 15 insertions(+)

On Thu, Aug 25, 2022 at 04:36:14PM +0200, Ævar Arnfjörð Bjarmason wrote:

Nit: make cocciclean

Make the incremental compilation of "make coccicheck" even faster, at the cost of losing dependency discovery for a small whitelist of *.c files which we can't create *.o files for.

This will break obscure edge cases since e.g.:

    time make -W block-sha1/sha1.h coccicheck

Would correctly re-build before, as block-sha1/sha1.c uses block-sha1/sha1.h, but as it and these other $(USE_TINY_FOUND_H_SOURCES) files depended on the union of all such headers (and more, all of $(FOUND_H_SOURCES)) the above command would take ~10 seconds before.

As noted in the preceding commit these obscure cases were broken before the preceding commit fixed them. I think it makes sense to make a correctness v.s. performance trade-off here. By doing this it becomes viable to e.g. run "make coccicheck" in the same loop that merges various topic branches down (along with "ccache" sped-up compilation).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Optimize the very slow "coccicheck" target to take advantage of incremental rebuilding, and fix outstanding bugs and dependency problems with the existing rule.

The rule is now faster both on the initial run as we can make better use of GNU make's parallelism than the old ad-hoc combination of make's parallelism combined with $(SPATCH_BATCH_SIZE) and "xargs -n".

It also makes us *much* faster when incrementally building, it's now viable to "make coccicheck" as topic branches are merged down.

The rule didn't use FORCE (or its equivalents) before, so a:

	make coccicheck
	make coccicheck

Would report nothing to do on the second iteration. But all of our patch output depended on all $(COCCI_SOURCES) files, therefore e.g.:

    make -W grep.c coccicheck

Would be the same as doing a full re-run. A change in a single file would force us to do a full re-run.

The reason for this (not the initial rationale, but my analysis) is:

* Since we create a single "*.cocci.patch+" we don't know where to
  pick up where we left off. Note that (per [1]) we've had a race
  condition since 960154b9c17 (coccicheck: optionally batch spatch
  invocations, 2019-05-06) which might result in us producing corrupt
  patches to to the concurrent appending to "$@+" in the pre-image.

  That bug is now fixed.

* We've been carrying forward the dependency on the *.c files since
  63f0a758a06 (add coccicheck make target, 2016-09-15) the rule was
  initially added as a sort of poor man's dependency discovery.

  As we don't include other *.c files depending on other *.c files
  has always been broken, as could be trivially demonstrated
  e.g. with:

       make coccicheck
       make -W strbuf.h coccicheck

  However, depending on the corresponding *.c files has been doing
  something, namely that *if* an API change modified both *.c and *.h
  files we'd catch the change to the *.h we care about via the *.c
  being changed.

  For API changes that happened only via *.h files we'd do the wrong
  thing before this change, but e.g. for function additions (not
  "static inline" ones) catch the *.h change by proxy.

Now we'll instead:

 * Create a <RULE>/<FILE> pair in the .build directory, E.g. for
   swap.cocci and grep.c we'll create
   .build/contrib/coccinelle/swap.cocci.patch/grep.c.

   That file is the diff we'll apply for that <RULE>-<FILE>
   combination, if there's no changes to me made (the common case)
   it'll be an empty file.

 * Our generated *.patch
   file (e.g. contrib/coccinelle/swap.cocci.patch) is now a simple "cat
   $^" of all of all of the <RULE>/<FILE> files for a given <RULE>.

   See 1cc0425a27c (Makefile: have "make pot" not "reset --hard",
   2022-05-26) for another recent rule that used that technique.

 * We depend on a .build/<RULE>.ok file, which in turn depends on our
   <RULE>. This emulates the previous "test -s" test, but in a more
   Makefile-native way (e.g. -k works as expected).

As noted above this change fixes existing dependency issues, e.g. changing strbuf.h will now trigger a re-build. Our default is to depend on $(FOUND_H_SOURCES). This makes incremental re-building possible, but it's still fairly slow, as modifying one header will cause a full re-build of everything.

Which is why we'll not depend on $(FOUND_H_SOURCES) but the *.o file corresponding to the *.c file, if it exists already. This means that we can do:

    make all
    make coccicheck
    make -W column.h coccicheck

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

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

Further implementation details & notes:

 * We can take better advantage of parallelism, while making sure that
   we don't racily append to the contrib/coccinelle/swap.cocci.patch
   file from multiple workers.

   Before this change running "make coccicheck" would by default end
   up pegging just one CPU at the very end for a while, usually as
   we'd finish whichever *.cocci rule was the most expensive.

   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.

   There will be cases where getting rid of "SPATCH_BATCH_SIZE" makes
   things worse, but a from-scratch "make coccicheck" with the default
   of SPATCH_BATCH_SIZE=1 (and tweaking it doesn't make a difference)
   is faster (~3m36s v.s. ~3m56s) with this approach, as we can feed
   the CPU more work in a less staggered way.

 * Getting rid of "SPATCH_BATCH_SIZE" particularly helps in cases
   where the default of 1 yields parallelism under "make coccicheck",
   but then running e.g.:

       make -W contrib/coccinelle/swap.cocci coccicheck

   I.e. before that would use only one CPU core, until the user
   remembered to adjust "SPATCH_BATCH_SIZE" differently than the
   setting that makes sense when doing a non-incremental run of "make
   coccicheck".

 * The change here to
   "contrib/coccinelle/the_repository.pending.cocci" is because we've
   been spewing out this warning into our $@.log since that rule was
   added:

       warning: rule starting on line 21: metavariable F not used in the - or context code

   It's being fixed here because we'll now notice on "make
   coccicheck-pending", since we're not redirecting all of STDERR to a
   log file, instead we use the "--very-quiet" option.

Outstanding issues & future work:

 * We could get rid of "--all-includes" in favor of manually
   specifying a list of includes to give to "spatch(1)".

   As noted upthread of [2] a naïve removal of "--all-includes" will
   result in broken *.cocci patches, but if we know the exhaustive
   list of includes via COMPUTE_HEADER_DEPENDENCIES we don't need to
   re-scan for them, we could grab the headers to include from the
   .depend.d/<file>.o.d and supply them with the "--include" option to
   spatch(1).q

 * We can generalize COMPUTE_HEADER_DEPENDENCIES to spew out the list
   of *.h dependencies without making a *.o object, which is slightly
   faster, and would also benefit the "make sparse" target. We'd still
   need to run the compiler, it would just have less work to do.

1. https://lore.kernel.org/git/877dlwotjc.fsf@evledraar.gmail.com/
2. https://lore.kernel.org/git/87ft18tcog.fsf@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile                                      | 92 +++++++++++++------
 .../coccinelle/the_repository.pending.cocci   |  1 -
 shared.mak                                    |  4 +-
 3 files changed, 66 insertions(+), 31 deletions(-)

On Thu, Aug 25, 2022 at 04:36:15PM +0200, Ævar Arnfjörð Bjarmason wrote:

There is no bug, because there is no concurrent appending to "$@+". The message you mention seems to be irrelevant, as it talks about 'xargs -P', but the invocation in '%.cocci.patch' targets never used '-P'.

This assumption doesn't hold, and I very much dislike the idea of depending on *.o files:

  - Our static-analysis CI job doesn't build Git, now it will have to.

  - I don't have Coccinelle installed, because my distro doesn't ship
    it, and though the previous release did ship it, it was outdated.
    Instead I use Coccinelle's most recent version from a container
    which doesn't contain any build tools apart from 'make' for 'make
    coccicheck'.

    With this patch series I can't use this containerized Coccinelle
    at all, because even though I've already built git on the host,
    the dependency on *.o files triggers a BUILD-OPTIONS check during
    'make coccicheck', and due to the missing 'curl-config' the build
    options do differ, triggering a rebuild, which in the absence of a
    compiler fails.

    And then the next 'make' on the host will have to rebuild
    everything again...

No, setting both to 4 does yield 4 workers.

SPATCH_BATCH_SIZE has nothing to do with parallelism; it is merely the number of C source files that we pass to a single 'spatch' invocation, but for any given semantic patch it's still a sequential loop.

On Thu, Aug 25 2022, SZEDER Gábor wrote:

Thanks for taking a look!

I think this is just confusing, I'll amend/rephrase.

And at this point I honestly can't remember if I'm conflating this with an issue with my earlier proposed series here (I drafted this a while ago), or a rather obscure "hidden" feature that I did use to speed up coccicheck for myself for a while. Which is that you can do e.g.:

	make coccicheck SPATCH_BATCH_SIZE="8 -P 8"

Which will invoke xargs with the "-P 8" option in batches of 8 files.

That's never a thing that 960154b9c17 expected, and it only worked as an accident, but it *would* work, and unless you got unlucky with the races involved would generally speed up your coccicheck.

It's my fault for not calling this out more explicitly, but it *optionally* depends on the *.o files, but if you don't have them compiled already a "make coccicheck" will just use "spatch", and nothing else.

See the CI run/output for this series: https://github.com/avar/git/runs/8017916844?check_suite_focus=true

Aside from this series which doesn't change how it works, maintaining this doesn't seem important to me. E.g. as I noted in [1] coccinelle will happily run on code that doesn't even compile.

So running the compiler during "static-analysis" (or another step that it would depend on, maybe "pedantic" or "sparse") seems like a good (but separate change).

You don't want to wonder about odd coccinelle output, only to see it's trying to make sense of C source that doesn't even compile.

There may be some odd interaction here, but it's unclear if you've actually tried to do this with this series, because unless I've missed some edge case this should all still work, per the above.

What *won't* work is avoiding potential re-compilation of *.o files if you *have them already* when you run "make coccicheck". I'm not familiar with this type of setup, are you saying you're running "make coccicheck" on a working directory that already has *.o files, but you want it to ignore the *.o?

That could easily be made optional, but I just assumed that nobody would
care. If you want that can you try this on top and see if it works for
you?:
	
	diff --git a/Makefile b/Makefile
	index 9410a587fc0..11d83c490b4 100644
	--- a/Makefile
	+++ b/Makefile
	@@ -3174,6 +3174,11 @@ TINY_FOUND_H_SOURCES += strbuf.h
	 	$(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:
	@@ -3181,7 +3186,7 @@ define cocci-rule
	 # 2 = $(2)
	 COCCI_$(1:contrib/coccinelle/%.cocci=%) += .build/$(1).patch/$(2)
	 .build/$(1).patch/$(2): GIT-SPATCH-DEFINES
	-.build/$(1).patch/$(2): $(if $(wildcard $(3)),$(3),$(if $(filter $(USE_TINY_FOUND_H_SOURCES),$(3)),$(TINY_FOUND_H_SOURCES),.build/contrib/coccinelle/FOUND_H_SOURCES))
	+.build/$(1).patch/$(2): $(if $(and $(SPATCH_USE_O_DEPENDENCIES),$(wildcard $(3))),$(3),$(if $(filter $(USE_TINY_FOUND_H_SOURCES),$(3)),$(TINY_FOUND_H_SOURCES),.build/contrib/coccinelle/FOUND_H_SOURCES))
	 .build/$(1).patch/$(2): $(1)
	 .build/$(1).patch/$(2): .build/$(1).patch/% : %
	 	$$(call mkdir_p_parent_template)

I'd need to split up the already long line, but with *.o files compiled
and:
	
	$ time make -W column.h contrib/coccinelle/free.cocci.patch COMPUTE_HEADER_DEPENDENCIES=yes
	    CC wt-status.o
	    CC builtin/branch.o
	    CC builtin/clean.o
	    CC builtin/column.o
	    CC builtin/commit.o
	    CC builtin/tag.o
	    CC column.o
	    CC help.o
	    SPATCH .build/contrib/coccinelle/free.cocci.patch/builtin/commit.c
	    SPATCH .build/contrib/coccinelle/free.cocci.patch/builtin/tag.c
	    SPATCH .build/contrib/coccinelle/free.cocci.patch/builtin/column.c
	    SPATCH .build/contrib/coccinelle/free.cocci.patch/column.c
	    SPATCH .build/contrib/coccinelle/free.cocci.patch/wt-status.c
	    SPATCH .build/contrib/coccinelle/free.cocci.patch/builtin/branch.c
	    SPATCH .build/contrib/coccinelle/free.cocci.patch/builtin/clean.c
	    SPATCH .build/contrib/coccinelle/free.cocci.patch/help.c
	    SPATCH MERGE contrib/coccinelle/free.cocci.patch
	
	real    0m0.550s
	user    0m0.505s
	sys     0m0.096s

But if you set COMPUTE_HEADER_DEPENDENCIES=yes it'll take ~4s (on that box), and well re-apply the free.cocci rule to all the *.c files (and this is all with the caching mechanism in 5/5, real "spatch" will be much slower).

If you already have git compiled (or partially compiled) the "happy path" to avoiding work is almost definitely to re-compile that *.o because the *.c changed, at which point we'll be able to see if we even need to re-run any of the coccinelle rules. Usually we'll need to re-run a far smaller set than the full set we operate on now.

Thanks, will fix. I see I conflated SPATCH_BATCH_SIZE with spatch's --jobs there (although from experimentation that seems to have pretty limited parallelism).

1. https://lore.kernel.org/git/220825.86ilmg4mil.gmgdl@evledraar.gmail.com/

On Fri, Aug 26, 2022 at 12:18:28AM +0200, Ævar Arnfjörð Bjarmason wrote:

Abusing SPATCH_BATCH_SIZE to try to shoot yourself in the foot definitely doesn't qualify as "bug" in my book :)

It seems like a bad and undesired change to me. We have plenty of other jobs that do build Git. It's just a waste of time to do it in a job that doesn't actually need it.

You don't even want to look at Coccinelle output at all, if any of the other jobs were to fail for whatever reason.

It did not work:

  # Host:
  $ make
    SUBDIR git-gui
    SUBDIR gitk-git
    SUBDIR templates

  # Container:
  $ time make coccicheck -j4 SPATCH_FLAGS=--all-includes
  GIT_VERSION = 2.37.GIT
  make: curl-config: Command not found
      * new build flags
      CC chdir-notify.o
      CC imap-send.o
  /bin/sh: 1: gcc-10: not found
  /bin/sh: 1: gcc-10: not found
  make: *** [Makefile:2605: chdir-notify.o] Error 127
  make: *** Waiting for unfinished jobs....
  make: *** [Makefile:2605: imap-send.o] Error 127
      CC usage.o
  /bin/sh: 1: gcc-10: not found
  make: *** [Makefile:2605: usage.o] Error 127

  # Host again:
  $ make
  GIT_VERSION = 2.37.2.387.g804ba1c054
      * new build flags
      CC fuzz-commit-graph.o
  [ ... rebuild ... ]

OK, with these changes on top I can run 'make coccicheck' in the container after Git was built on the host.

And I noticed a few behavior changes:

  - 'make coccicheck' currently prints ~"Nr-of-semantic-patches"
    lines, but with this change it grows to "Nr-of-semantic-patches *
    Nr-of-source-files" lines.

    I didn't like this when I did something similar a few years ago in

      https://public-inbox.org/git/20180802115522.16107-1-szeder.dev@gmail.com/

    and I don't like it now, either, but oh well, there is always
    'make -s' (though Peff thought it's "eye candy" back then :).

  - Any transformation found during 'make coccicheck' is now
    considered an error.

    With 'SPATCH_FLAGS=--recursive-includes' I got this:

      [... gazillions of SPATCH .build/... lines ... ]
          SPATCH .build/contrib/coccinelle/hashmap.cocci.patch/config.c
          SPATCH MERGE contrib/coccinelle/array.cocci.patch
          SPATCH MERGE contrib/coccinelle/commit.cocci.patch
          SPATCH MERGE contrib/coccinelle/xcalloc.cocci.patch
          SPATCH MERGE contrib/coccinelle/preincr.cocci.patch
          SPATCH MERGE contrib/coccinelle/free.cocci.patch
          SPATCH MERGE contrib/coccinelle/equals-null.cocci.patch
          SPATCH MERGE contrib/coccinelle/xopen.cocci.patch
          SPATCH MERGE contrib/coccinelle/qsort.cocci.patch
          SPATCH MERGE contrib/coccinelle/xstrdup_or_null.cocci.patch
          SPATCH MERGE contrib/coccinelle/swap.cocci.patch
          SPATCH MERGE contrib/coccinelle/strbuf.cocci.patch
          SPATCH MERGE contrib/coccinelle/object_id.cocci.patch
          SPATCH MERGE contrib/coccinelle/flex_alloc.cocci.patch
          SPATCH RESULT contrib/coccinelle/array.cocci.patch
          SPATCH RESULT contrib/coccinelle/commit.cocci.patch
          SPATCH RESULT contrib/coccinelle/xcalloc.cocci.patch
              diff -u -p a/promisor-remote.c b/promisor-remote.c
              --- a/promisor-remote.c
              +++ b/promisor-remote.c
              @@ -146,7 +146,7 @@ static void promisor_remote_init(struct
               	if (r->promisor_remote_config)
               		return;
               	config = r->promisor_remote_config =
              -		xcalloc(sizeof(*r->promisor_remote_config), 1);
              +		xcalloc(1, sizeof(*r->promisor_remote_config));
               	config->promisors_tail = &config->promisors;
               
               	repo_config(r, promisor_remote_config, config);
      make: *** [Makefile:3218: .build/contrib/coccinelle/xcalloc.cocci.patch.ok] Error 1
      make: *** Waiting for unfinished jobs....

    So it seems that if more than one semantic patches find something
    to transform, then only the transformations from the first will
    be generated (and printed).

    Note, however, that if transformations are found while "manually"
    applying semantic patches, i.e. running 'make foo.cocci.patch',
    then it's not an error.

  - The resulting transformations from (only one of) the semantic
    patches are now printed at the end of 'make coccicheck'.  I doubt
    it's desirable, but even if it is, it's an independent change, and
    the static analysis CI job needs a corresponding update, because
    it already 'cat's all non-empty '.cocci.patch' files.

    Note, however, that if transformations are found while "manually"
    applying semantic patches, then neither the transformations are
    printed, nor anything that would indicate that there were any
    transformations.

  - As I remember before this series the SPATCH TESTs were always run
    first during 'make coccicheck'.  I don't know whether that was
    intentional and there were dependencies to enforce that order, or
    'make' just happened to schedule them in that order on my system.
    Or maybe I'm just misremembering, but it does make sense to me to
    run those tests first (or at least before applying the
    corresponding semantic patches to the whole tree).

    With this series the tests were run near the end:

      [...]
      SPATCH .build/contrib/coccinelle/unused.cocci.patch/git.c
      SPATCH .build/contrib/coccinelle/unused.cocci.patch/color.c
      MKDIR -p .build/contrib/coccinelle/tests
      SPATCH TEST contrib/coccinelle/tests/unused.res
      SPATCH TEST contrib/coccinelle/tests/free.res
      SPATCH .build/contrib/coccinelle/hashmap.cocci.patch/upload-pack.c
      SPATCH .build/contrib/coccinelle/hashmap.cocci.patch/merge-ort-wrappers.c
      [...]

    'hashmap.cocci' happened to be applied last.

Unfortunately:

  # After a successful 'make coccicheck' in the container:
  $ time make -W column.h contrib/coccinelle/free.cocci.patch COMPUTE_HEADER_DEPENDENCIES=yes
  make: curl-config: Command not found
      * new spatch flags
      * new build flags
      CC chdir-notify.o
  /bin/sh: 1: gcc-10: not found
      CC imap-send.o
  /bin/sh: 1: gcc-10: not found
  make: *** [Makefile:2605: chdir-notify.o] Error 127
  make: *** Waiting for unfinished jobs....
  make: *** [Makefile:2605: imap-send.o] Error 127
      CC usage.o
  /bin/sh: 1: gcc-10: not found
  make: *** [Makefile:2605: usage.o] Error 127

'spatch --jobs' works just fine; the problem is that we use 'spatch' wrong.

Add a rather trivial "spatchcache", with this running e.g.:

	make clean
	make contrib/coccinelle/free.cocci.patch

Is cut down from ~20s to ~3s on my system (and much of it is fixable shell overhead). This uses "redis" as a cache by default, but it's configurable. See the embedded documentation.

This is *not* like ccache in that we won't cache failed spatch invocations, or those where spatch suggests changes for us. Those cases are so rare that I didn't think it was worth the bother, by far the most common case is that it has no suggested changes.

Because we narrow the cache to that we don't need to save away stdout, stderr & the exit code. We simply cache the cases where we had no suggested changes.

Another benchmark is to compare this with the previous SPATCH_BATCH_SIZE=N, as noted in [1]. Before this (on my 8 core system) running:

	make clean; time make contrib/coccinelle/array.cocci.patch SPATCH_BATCH_SIZE=0

Would take 33s, but with the preceding changes running without this "spatchcache" is slightly slower, or around 35s:

	make clean; time make contrib/coccinelle/array.cocci.patch

Now doing the same with SPATCH=contrib/coccinelle/spatchcache will take around 6s, but we'll need to compile the *.o files first to take full advantage of it (which can be fast with "ccache"):

	make clean; make; time make contrib/coccinelle/array.cocci.patch SPATCH=contrib/coccinelle/spatchcache

1. https://lore.kernel.org/git/YwdRqP1CyUAzCEn2@coredump.intra.peff.net/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/coccinelle/spatchcache | 217 +++++++++++++++++++++++++++++++++
 1 file changed, 217 insertions(+)
 create mode 100755 contrib/coccinelle/spatchcache

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/

Changes since v1 (based on SZEDER's feedback, thanks!):

 * Corrected small bits of commit message, removed mention of a "bug"
   (which was really something in an earlier series of mine...)

 * Split up the previous "incremental" commit. Now 1/9 is an unrelated
   fix that was in it (but which was spotted by it).

 * Added some preceding patches to make tweaking spatch flags easier.

 * The part where we optionally piggy-back on the dependencies of *.o
   files is now in 8/9, the "pure" incremental coccicheck in 7/9 will
   never need a compiler.

 * The "make coccicheck" now acts as before, i.e. no "make -k" needed,
   it runs to completion. I think having it "make -k" friendly would
   optionally make sense, but that can be left for some other time.

 * The previous change dropped the cocci *.log, we now log the stderr
   we had as before.

 * The spatchcache contrib command in 9/9 has updated docs & behavior.

 * The spatchcache now requires SPATCH_FLAGS=--very-quiet, which was
   the new default in this series before. I.e. we never cache things
   that have any stderr output.

Ævar Arnfjörð Bjarmason (9):
  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
  spatchcache: add a ccache-alike for "spatch"

 .gitignore                                    |   1 +
 Makefile                                      | 113 +++++++--
 contrib/coccinelle/spatchcache                | 234 ++++++++++++++++++
 .../coccinelle/the_repository.pending.cocci   |   1 -
 shared.mak                                    |   3 +-
 5 files changed, 325 insertions(+), 27 deletions(-)
 create mode 100755 contrib/coccinelle/spatchcache

Range-diff against v1:
 -:  ----------- >  1:  72b6a8e4e0b cocci rules: remove unused "F" metavariable from pending rule
 1:  8604ac9bfbc =  2:  0998948b881 Makefile: add ability to TAB-complete cocci *.patch rules
 2:  db3b3427668 !  3:  63cf9f58d99 Makefile: have "coccicheck" re-run if flags change
    @@ Commit message
         since 63f0a758a06 (add coccicheck make target, 2016-09-15), unlike
         e.g. "make grep.o" we wouldn't re-run it when $(SPATCH) or
         $(SPATCH_FLAGS) changed. To test new flags we needed to first do a
    -    "make clean".
    +    "make cocciclean".
     
         This now uses the same (copy/pasted) pattern as other "DEFINES"
         rules. As a result we'll re-run properly. This can be demonstrated
 -:  ----------- >  4:  54d6bae3984 Makefile: split off SPATCH_BATCH_SIZE comment from "cocci" heading
 -:  ----------- >  5:  4f165bf6128 cocci: split off include-less "tests" from SPATCH_FLAGS
 -:  ----------- >  6:  c74d09f4825 cocci: split off "--all-includes" from SPATCH_FLAGS
 3:  6fa83695f1f !  7:  120607b5da6 cocci: make "coccicheck" rule incremental
    @@ Commit message
         cocci: make "coccicheck" rule incremental
     
         Optimize the very slow "coccicheck" target to take advantage of
    -    incremental rebuilding, and fix outstanding bugs and dependency
    -    problems with the existing rule.
    +    incremental rebuilding, and fix outstanding dependency problems with
    +    the existing rule.
     
         The rule is now faster both on the initial run as we can make better
         use of GNU make's parallelism than the old ad-hoc combination of
    -    make's parallelism combined with $(SPATCH_BATCH_SIZE) and "xargs -n".
    +    make's parallelism combined with $(SPATCH_BATCH_SIZE) and/or the
    +    "--jobs" argument to "spatch(1)".
     
         It also makes us *much* faster when incrementally building, it's now
         viable to "make coccicheck" as topic branches are merged down.
    @@ Commit message
     
             make -W grep.c coccicheck
     
    -    Would be the same as doing a full re-run. A change in a single file
    -    would force us to do a full re-run.
    +    Would do a full re-run, i.e. a a change in a single file would force
    +    us to do a full re-run.
     
         The reason for this (not the initial rationale, but my analysis) is:
     
         * Since we create a single "*.cocci.patch+" we don't know where to
    -      pick up where we left off. Note that (per [1]) we've had a race
    -      condition since 960154b9c17 (coccicheck: optionally batch spatch
    -      invocations, 2019-05-06) which might result in us producing corrupt
    -      patches to to the concurrent appending to "$@+" in the pre-image.
    -
    -      That bug is now fixed.
    +      pick up where we left off, or how to incrementally merge e.g. a
    +      "grep.c" change with an existing *.cocci.patch.
     
         * We've been carrying forward the dependency on the *.c files since
           63f0a758a06 (add coccicheck make target, 2016-09-15) the rule was
    @@ Commit message
            file (e.g. contrib/coccinelle/swap.cocci.patch) is now a simple "cat
            $^" of all of all of the <RULE>/<FILE> files for a given <RULE>.
     
    +       In the case discussed above of "grep.c" being changed we'll do the
    +       full "cat" every time, so they resulting *.cocci.patch will always
    +       be correct and up-to-date, even if it's "incrementally updated".
    +
            See 1cc0425a27c (Makefile: have "make pot" not "reset --hard",
            2022-05-26) for another recent rule that used that technique.
     
    -     * We depend on a .build/<RULE>.ok file, which in turn depends on our
    -       <RULE>. This emulates the previous "test -s" test, but in a more
    -       Makefile-native way (e.g. -k works as expected).
    -
    -    As noted above this change fixes existing dependency issues,
    -    e.g. changing strbuf.h will now trigger a re-build. Our default is to
    -    depend on $(FOUND_H_SOURCES). This makes incremental re-building
    -    possible, but it's still fairly slow, as modifying one header will
    -    cause a full re-build of everything.
    -
    -    Which is why we'll not depend on $(FOUND_H_SOURCES) but the *.o file
    -    corresponding to the *.c file, if it exists already. This means that
    -    we can do:
    -
    -        make all
    -        make coccicheck
    -        make -W column.h coccicheck
    -
    -    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").
    +    As before we'll:
     
    -     * If that *.o file doesn't exist we'll depend on an intermediate file
    -       of ours which in turn depends on $(FOUND_H_SOURCES).
    +     * End up generating a contrib/coccinelle/swap.cocci.patch, if we
    +       "fail" by creating a non-empty patch we'll still exit with a zero
    +       exit code.
     
    -       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.
    +       Arguably we should move to a more Makefile-native way of doing
    +       this, i.e. fail early, and if we want all of the "failed" changes
    +       we can use "make -k", but as the current
    +       "ci/run-static-analysis.sh" expects us to behave this way let's
    +       keep the existing behavior of exhaustively discovering all cocci
    +       changes, and only failing if spatch itself errors out.
     
         Further implementation details & notes:
     
    -     * We can take better advantage of parallelism, while making sure that
    -       we don't racily append to the contrib/coccinelle/swap.cocci.patch
    -       file from multiple workers.
    -
    -       Before this change running "make coccicheck" would by default end
    +     * Before this change running "make coccicheck" would by default end
            up pegging just one CPU at the very end for a while, usually as
            we'd finish whichever *.cocci rule was the most expensive.
     
    @@ Commit message
            setting that makes sense when doing a non-incremental run of "make
            coccicheck".
     
    -     * The change here to
    -       "contrib/coccinelle/the_repository.pending.cocci" is because we've
    -       been spewing out this warning into our $@.log since that rule was
    -       added:
    -
    -           warning: rule starting on line 21: metavariable F not used in the - or context code
    -
    -       It's being fixed here because we'll now notice on "make
    -       coccicheck-pending", since we're not redirecting all of STDERR to a
    -       log file, instead we use the "--very-quiet" option.
    -
         Outstanding issues & future work:
     
          * We could get rid of "--all-includes" in favor of manually
            specifying a list of includes to give to "spatch(1)".
     
    -       As noted upthread of [2] a naïve removal of "--all-includes" will
    +       As noted upthread of [1] a naïve removal of "--all-includes" will
            result in broken *.cocci patches, but if we know the exhaustive
            list of includes via COMPUTE_HEADER_DEPENDENCIES we don't need to
            re-scan for them, we could grab the headers to include from the
            .depend.d/<file>.o.d and supply them with the "--include" option to
            spatch(1).q
     
    -     * We can generalize COMPUTE_HEADER_DEPENDENCIES to spew out the list
    -       of *.h dependencies without making a *.o object, which is slightly
    -       faster, and would also benefit the "make sparse" target. We'd still
    -       need to run the compiler, it would just have less work to do.
    -
    -    1. https://lore.kernel.org/git/877dlwotjc.fsf@evledraar.gmail.com/
    -    2. https://lore.kernel.org/git/87ft18tcog.fsf@evledraar.gmail.com/
    +    1. https://lore.kernel.org/git/87ft18tcog.fsf@evledraar.gmail.com/
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Makefile ##
    -@@ Makefile: SP_EXTRA_FLAGS = -Wno-universal-initializer
    - SANITIZE_LEAK =
    - SANITIZE_ADDRESS =
    - 
    --# For the 'coccicheck' target; setting SPATCH_BATCH_SIZE higher will
    +@@ Makefile: SANITIZE_ADDRESS =
    + SPATCH_INCLUDE_FLAGS = --all-includes
    + SPATCH_FLAGS =
    + SPATCH_TEST_FLAGS =
    +-# Setting SPATCH_BATCH_SIZE higher will
     -# usually result in less CPU usage at the cost of higher peak memory.
     -# Setting it to 0 will feed all files in a single spatch invocation.
    - SPATCH_FLAGS = --all-includes
     -SPATCH_BATCH_SIZE = 1
      
      # Rebuild 'coccicheck' if $(SPATCH), its flags etc. change
      TRACK_SPATCH_DEFINES =
    - TRACK_SPATCH_DEFINES += $(SPATCH)
    +@@ Makefile: TRACK_SPATCH_DEFINES += $(SPATCH)
    + TRACK_SPATCH_DEFINES += $(SPATCH_INCLUDE_FLAGS)
      TRACK_SPATCH_DEFINES += $(SPATCH_FLAGS)
    + TRACK_SPATCH_DEFINES += $(SPATCH_TEST_FLAGS)
     -TRACK_SPATCH_DEFINES += $(SPATCH_BATCH_SIZE)
      GIT-SPATCH-DEFINES: FORCE
      	@FLAGS='$(TRACK_SPATCH_DEFINES)'; \
    @@ Makefile: check: $(GENERATED_H)
      COCCI = $(wildcard contrib/coccinelle/*.cocci)
     +COCCI_NAMES = $(COCCI:contrib/coccinelle/%.cocci=%)
      COCCI_PATCHES = $(addsuffix .patch,$(COCCI))
    -+COCCI_PATCHES_OK = $(COCCI_PATCHES:%=.build/%.ok)
      COCCICHECK_PENDING = $(filter %.pending.cocci.patch,$(COCCI_PATCHES))
     -COCCICHECK = $(filter-out $(COCCICHECK_PENDING),$(COCCI_PATCHES))
      
     +COCCICHECK = $(filter-out $(COCCICHECK_PENDING),$(COCCI_PATCHES))
    -+COCCICHECK_OK = $(COCCICHECK:%=.build/%.ok)
    -+COCCICHECK_PENDING_OK = $(COCCICHECK_PENDING:%=.build/%.ok)
      COCCI_TEST_RES = $(wildcard contrib/coccinelle/tests/*.res)
      
     -$(COCCI_PATCHES): GIT-SPATCH-DEFINES
    @@ Makefile: check: $(GENERATED_H)
     -	fi; \
     -	if ! echo $(COCCI_SOURCES) | xargs $$limit \
     -		$(SPATCH) $(SPATCH_FLAGS) \
    +-		$(SPATCH_INCLUDE_FLAGS) \
     -		--sp-file $< --patch . \
     -		>$@+ 2>$@.log; \
     +.build/contrib/coccinelle/FOUND_H_SOURCES: $(FOUND_H_SOURCES)
    @@ Makefile: check: $(GENERATED_H)
     +define cocci-rule
     +
     +## Rule for .build/$(1).patch/$(2); Params:
    -+# 1 = $(1)
    -+# 2 = $(2)
    ++# $(1) = e.g. "free.cocci"
    ++# $(2) = e.g. "grep.c"
     +COCCI_$(1:contrib/coccinelle/%.cocci=%) += .build/$(1).patch/$(2)
     +.build/$(1).patch/$(2): GIT-SPATCH-DEFINES
    -+.build/$(1).patch/$(2): $(if $(wildcard $(3)),$(3),.build/contrib/coccinelle/FOUND_H_SOURCES)
    ++.build/$(1).patch/$(2): .build/contrib/coccinelle/FOUND_H_SOURCES
     +.build/$(1).patch/$(2): $(1)
     +.build/$(1).patch/$(2): .build/$(1).patch/% : %
     +	$$(call mkdir_p_parent_template)
    -+	$$(QUIET_SPATCH)$$(SPATCH) $$(SPATCH_FLAGS) \
    -+		--very-quiet \
    -+		--sp-file $(1) --patch . \
    -+		$$(SPATCH_PATCH_FLAGS) $$< >$$@
    ++	$$(QUIET_SPATCH)if ! $$(SPATCH) $$(SPATCH_FLAGS) \
    ++		$$(SPATCH_INCLUDE_FLAGS) \
    ++		--sp-file $(1) --patch . $$< \
    ++		>$$@ 2>$$@.log; \
    + 	then \
    +-		cat $@.log; \
    ++		echo "ERROR when applying '$(1)' to '$$<'; '$$@.log' follows:"; \
    ++		cat $$@.log; \
    + 		exit 1; \
    +-	fi; \
    +-	mv $@+ $@; \
    +-	if test -s $@; \
    ++	fi
     +endef
     +
     +define cocci-matrix-2
     +
    -+$(foreach s,$(COCCI_SOURCES),$(call cocci-rule,$(c),$(s),$(s:%.c=%.o)))
    ++$(foreach s,$(COCCI_SOURCES),$(call cocci-rule,$(c),$(s)))
     +endef
     +define cocci-matrix-1
     +$(foreach c,$(COCCI),$(call cocci-matrix-2,$(c)))
    @@ Makefile: check: $(GENERATED_H)
     +define spatch-rule
     +
     +contrib/coccinelle/$(1).cocci.patch: $$(COCCI_$(1))
    -+	$(QUIET_SPATCH_M)cat $$^ >$$@
    -+endef
    -+
    -+$(COCCI_PATCHES_OK): .build/%.ok : %
    -+	$(QUIET_SPATCH_R)if test -s $<; \
    ++	$(QUIET_SPATCH_M)cat $$^ >$$@ && \
    ++	if test -s $$@; \
      	then \
    --		cat $@.log; \
    -+		cat $< | sed -e 's/^/	/'; \
    - 		exit 1; \
    --	fi; \
    --	mv $@+ $@; \
    --	if test -s $@; \
    --	then \
     -		echo '    ' SPATCH result: $@; \
    --	fi
    -+	fi && \
    -+	>$@
    ++		echo '    ' SPATCH result: $$@; \
    + 	fi
    ++endef
     +
     +ifneq ($(filter $(COCCI_RULES),$(MAKECMDGOALS)),)
     +$(eval $(foreach n,$(COCCI_NAMES),$(call spatch-rule,$(n))))
    @@ Makefile: check: $(GENERATED_H)
      
      COCCI_TEST_RES_GEN = $(addprefix .build/,$(COCCI_TEST_RES))
      $(COCCI_TEST_RES_GEN): GIT-SPATCH-DEFINES
    -@@ Makefile: $(COCCI_TEST_RES_GEN): .build/contrib/coccinelle/tests/%.res : contrib/coccinell
    - coccicheck-test: $(COCCI_TEST_RES_GEN)
    - 
    - coccicheck: coccicheck-test
    --coccicheck: $(COCCICHECK)
    -+coccicheck: $(COCCICHECK_OK)
    - 
    - # See contrib/coccinelle/README
    - coccicheck-pending: coccicheck-test
    --coccicheck-pending: $(COCCICHECK_PENDING)
    -+coccicheck-pending: $(COCCICHECK_PENDING_OK)
    - 
    - .PHONY: coccicheck coccicheck-pending
    - 
     @@ Makefile: profile-clean:
      
      cocciclean:
    @@ Makefile: profile-clean:
      clean: profile-clean coverage-clean cocciclean
      	$(RM) -r .build
     
    - ## contrib/coccinelle/the_repository.pending.cocci ##
    -@@ contrib/coccinelle/the_repository.pending.cocci: expression E;
    - 
    - @@
    - expression E;
    --expression F;
    - @@
    - - has_object_file_with_flags(
    - + repo_has_object_file_with_flags(the_repository,
    -
      ## shared.mak ##
     @@ shared.mak: ifndef V
      	QUIET_SP       = @echo '   ' SP $<;
    @@ shared.mak: ifndef V
     +	QUIET_SPATCH   = @echo '   ' SPATCH $@;
      	QUIET_SPATCH_T = @echo '   ' SPATCH TEST $(@:.build/%=%);
     +	QUIET_SPATCH_M = @echo '   ' SPATCH MERGE $$@;
    -+	QUIET_SPATCH_R = @echo '   ' SPATCH RESULT $<;
      
      ## Used in "Documentation/Makefile"
      	QUIET_ASCIIDOC	= @echo '   ' ASCIIDOC $@;
 4:  cda20790146 !  8:  536dce45eef cocci: make incremental compilation even faster
    @@ Metadata
     Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Commit message ##
    -    cocci: make incremental compilation even faster
    +    cocci: optimistically use COMPUTE_HEADER_DEPENDENCIES
     
    -    Make the incremental compilation of "make coccicheck" even faster, at
    -    the cost of losing dependency discovery for a small whitelist of *.c
    -    files which we can't create *.o files for.
    +    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 will break obscure edge cases since e.g.:
    +    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.
     
    -        time make -W block-sha1/sha1.h coccicheck
    +    This means that the common case of:
     
    -    Would correctly re-build before, as block-sha1/sha1.c uses
    -    block-sha1/sha1.h, but as it and these other
    -    $(USE_TINY_FOUND_H_SOURCES) files depended on the union of all such
    -    headers (and more, all of $(FOUND_H_SOURCES)) the above command would
    -    take ~10 seconds before.
    +            make
    +            make coccicheck
     
    -    As noted in the preceding commit these obscure cases were broken
    -    before the preceding commit fixed them. I think it makes sense to make
    -    a correctness v.s. performance trade-off here. By doing this it
    -    becomes viable to e.g. run "make coccicheck" in the same loop that
    -    merges various topic branches down (along with "ccache" sped-up
    -    compilation).
    +    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@szeder.dev/
    +
    +       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@gmail.com>
     
      ## Makefile ##
    -@@ Makefile: COCCICHECK_OK = $(COCCICHECK:%=.build/%.ok)
    - COCCICHECK_PENDING_OK = $(COCCICHECK_PENDING:%=.build/%.ok)
    - COCCI_TEST_RES = $(wildcard contrib/coccinelle/tests/*.res)
    - 
    -+USE_TINY_FOUND_H_SOURCES =
    -+USE_TINY_FOUND_H_SOURCES += compat/%
    -+USE_TINY_FOUND_H_SOURCES += block-sha1/%
    -+USE_TINY_FOUND_H_SOURCES += ppc/%
    -+
    -+TINY_FOUND_H_SOURCES =
    -+TINY_FOUND_H_SOURCES += cache.h
    -+TINY_FOUND_H_SOURCES += git-compat-util.h
    -+TINY_FOUND_H_SOURCES += strbuf.h
    -+
    - .build/contrib/coccinelle/FOUND_H_SOURCES: $(FOUND_H_SOURCES)
    +@@ Makefile: COCCI_TEST_RES = $(wildcard contrib/coccinelle/tests/*.res)
      	$(call mkdir_p_parent_template)
      	$(QUIET_GEN) >$@
    -@@ Makefile: define cocci-rule
    - # 2 = $(2)
    + 
    ++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:
    + # $(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): $(if $(wildcard $(3)),$(3),.build/contrib/coccinelle/FOUND_H_SOURCES)
    -+.build/$(1).patch/$(2): $(if $(wildcard $(3)),$(3),$(if $(filter $(USE_TINY_FOUND_H_SOURCES),$(3)),$(TINY_FOUND_H_SOURCES),.build/contrib/coccinelle/FOUND_H_SOURCES))
    +-.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)
    +@@ Makefile: endef
    + 
    + define cocci-matrix-2
    + 
    +-$(foreach s,$(COCCI_SOURCES),$(call cocci-rule,$(c),$(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)))
 5:  ce4734e5d79 !  9:  2b978676a56 spatchcache: add a ccache-alike for "spatch"
    @@ Commit message
     
         Add a rather trivial "spatchcache", with this running e.g.:
     
    -            make clean
    -            make contrib/coccinelle/free.cocci.patch
    +            make cocciclean
    +            make contrib/coccinelle/free.cocci.patch \
    +                    SPATCH=contrib/coccicheck/spatchcache \
    +                    SPATCH_FLAGS=--very-quiet
     
    -    Is cut down from ~20s to ~3s on my system (and much of it is fixable
    -    shell overhead). This uses "redis" as a cache by default, but it's
    -    configurable. See the embedded documentation.
    +    Is cut down from ~20s to ~5s on my system. Much of that is either
    +    fixable shell overhead, or the around 40 files we "CANTCACHE" (see the
    +    implementation).
    +
    +    This uses "redis" as a cache by default, but it's configurable. See
    +    the embedded documentation.
     
         This is *not* like ccache in that we won't cache failed spatch
         invocations, or those where spatch suggests changes for us. Those
         cases are so rare that I didn't think it was worth the bother, by far
    -    the most common case is that it has no suggested changes.
    +    the most common case is that it has no suggested changes. We'll also
    +    refuse to cache any "spatch" invocation that has output on stderr,
    +    which means that "--very-quiet" must be added to "SPATCH_FLAGS".
     
         Because we narrow the cache to that we don't need to save away stdout,
         stderr & the exit code. We simply cache the cases where we had no
    @@ contrib/coccinelle/spatchcache (new)
     +# driving "spatch" in git.git, in particular if we invoke:
     +#
     +#	make
    -+#	make coccicheck
    ++#	make coccicheck SPATCH_FLAGS=--very-quiet
     +#
     +# We can with COMPUTE_HEADER_DEPENDENCIES (auto-detected as true with
     +# "gcc" and "clang") write e.g. a .depend/grep.o.d for grep.c, when we
    @@ contrib/coccinelle/spatchcache (new)
     +# "contrib/coccinelle" makes it viable to (re-)run coccicheck
     +# e.g. when merging integration branches.
     +#
    ++# 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.
    ++#
     +# To use this, simply set SPATCH to
     +# contrib/coccinelle/spatchcache. Then optionally set:
     +#
     +#	[spatchCache]
    -+#		# path to a custom spatch
    ++#		# Optional: path to a custom spatch
     +#		spatch = ~/g/coccicheck/spatch.opt
     +#
     +# As well as this trace config (debug implies trace):
    @@ contrib/coccinelle/spatchcache (new)
     +#		debug = false
     +#
     +# Setting "trace" to "true" allows for seeing when we have a cache HIT
    -+# or MISS.
    ++# or MISS. To debug whether the cache is working do that, and run e.g.:
    ++#
    ++#	redis-cli FLUSHALL
    ++#	<make && make coccicheck, as above>
    ++#	grep -hore HIT -e MISS -e SET -e NOCACHE -e CANTCACHE .build/contrib/coccinelle | sort | uniq -c
    ++#	    600 CANTCACHE
    ++#	   7365 MISS
    ++#	   7365 SET
    ++#
    ++# A subsequent "make cocciclean && make coccicheck" should then have
    ++# all "HIT"'s and "CANTCACHE"'s.
     +
     +set -e
     +
    @@ contrib/coccinelle/spatchcache (new)
     +
     +if ! test -f "$dep"
     +then
    -+	trace_it "$0: have no '$dep' for '$arg_file', can't cache!"
    ++	trace_it "$0: CANTCACHE have no '$dep' for '$arg_file'!"
     +	exec "$spatch" "$@"
     +fi
     +
    @@ contrib/coccinelle/spatchcache (new)
     +		exit 128
     +	fi
     +else
    -+	trace_it "$0: NO CACHE (have changes): for '$arg_file' with '$arg_sp'"
    ++	trace_it "$0: NOCACHE (have changes): for '$arg_file' with '$arg_sp'"
     +	cat "$out"
     +	cat "$err" >&2
     +	rm -f "$out" "$err"

Declare the contrib/coccinelle/<rule>.cocci.patch rules in such a way as to allow TAB-completion, and slightly optimize the Makefile by cutting down on the number of $(wildcard) in favor of defining "coccicheck" and "coccicheck-pending" in terms of the same incrementally filtered list.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

Fix an issue with the "coccicheck" family of rules that's been here since 63f0a758a06 (add coccicheck make target, 2016-09-15), unlike e.g. "make grep.o" we wouldn't re-run it when $(SPATCH) or $(SPATCH_FLAGS) changed. To test new flags we needed to first do a "make cocciclean".

This now uses the same (copy/pasted) pattern as other "DEFINES" rules. As a result we'll re-run properly. This can be demonstrated e.g. on the issue noted in [1]:

	$ make contrib/coccinelle/xcalloc.cocci.patch COCCI_SOURCES=promisor-remote.c V=1
	[...]
	    SPATCH contrib/coccinelle/xcalloc.cocci
	$ make contrib/coccinelle/xcalloc.cocci.patch COCCI_SOURCES=promisor-remote.c SPATCH_FLAGS="--all-includes --recursive-includes"
	    * new spatch flags
	    SPATCH contrib/coccinelle/xcalloc.cocci
	     SPATCH result: contrib/coccinelle/xcalloc.cocci.patch
	$

1. https://lore.kernel.org/git/20220823095602.GC1735@szeder.dev/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .gitignore |  1 +
 Makefile   | 14 ++++++++++++++
 2 files changed, 15 insertions(+)

Fix an issue with a rule added in 9b45f499818 (object-store: prepare has_{sha1, object}_file to handle any repo, 2018-11-13). We've been spewing out this warning into our $@.log since that rule was added:

	warning: rule starting on line 21: metavariable F not used in the - or context code

We should do a better job of scouring our coccinelle log files for such issues, but for now let's fix this as a one-off.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/coccinelle/the_repository.pending.cocci | 1 -
 1 file changed, 1 deletion(-)

Split off the "; setting[...]" part of the comment added in In 960154b9c17 (coccicheck: optionally batch spatch invocations, 2019-05-06), and restore what we had before that, which was a comment indicating that variables for the "coccicheck" target were being set here.

When 960154b9c17 amended the heading to discuss SPATCH_BATCH_SIZE it left no natural place to add a new comment about other flags that preceded it. As subsequent commits will add such comments we need to split the existing comment up.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Amend the "coccicheck-test" rule added in f7ff6597a75 (cocci: add a "coccicheck-test" target and test *.cocci rules, 2022-07-05) to stop using "--all-includes". The flags we'll need for the tests are different than the ones we'll need for our main source code.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Per the rationale in 7b63ea57500 (Makefile: remove mandatory "spatch" arguments from SPATCH_FLAGS, 2022-07-05) we have certain flags that are truly mandatory, such as "--sp-file" and "--patch .". The "--all-includes" flag is also critical, but per [1] we might want to ad-hoc tweak it occasionally for testing or one-offs.

But being unable to set e.g. SPATCH_FLAGS="--verbose-parsing" without breaking how our "spatch" works isn't ideal, i.e. before this we'd need to know about the default include flags, and specify: SPATCH_FLAGS="--all-includes --verbose-parsing".

If we were then to change the default include flag (e.g. to "--recursive-includes") in the future any such one-off commands would need to be correspondingly updated.

Let's instead leave the SPATCH_FLAGS for the user, while creating a new SPATCH_INCLUDE_FLAGS to allow for ad-hoc testing of the include strategy itself.

1. https://lore.kernel.org/git/20220823095733.58685-1-szeder.dev@gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

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@szeder.dev/

   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@gmail.com>
---
 Makefile | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Optimize the very slow "coccicheck" target to take advantage of incremental rebuilding, and fix outstanding dependency problems with the existing rule.

The rule is now faster both on the initial run as we can make better use of GNU make's parallelism than the old ad-hoc combination of make's parallelism combined with $(SPATCH_BATCH_SIZE) and/or the "--jobs" argument to "spatch(1)".

It also makes us *much* faster when incrementally building, it's now viable to "make coccicheck" as topic branches are merged down.

The rule didn't use FORCE (or its equivalents) before, so a:

	make coccicheck
	make coccicheck

Would report nothing to do on the second iteration. But all of our patch output depended on all $(COCCI_SOURCES) files, therefore e.g.:

    make -W grep.c coccicheck

Would do a full re-run, i.e. a a change in a single file would force us to do a full re-run.

The reason for this (not the initial rationale, but my analysis) is:

* Since we create a single "*.cocci.patch+" we don't know where to
  pick up where we left off, or how to incrementally merge e.g. a
  "grep.c" change with an existing *.cocci.patch.

* We've been carrying forward the dependency on the *.c files since
  63f0a758a06 (add coccicheck make target, 2016-09-15) the rule was
  initially added as a sort of poor man's dependency discovery.

  As we don't include other *.c files depending on other *.c files
  has always been broken, as could be trivially demonstrated
  e.g. with:

       make coccicheck
       make -W strbuf.h coccicheck

  However, depending on the corresponding *.c files has been doing
  something, namely that *if* an API change modified both *.c and *.h
  files we'd catch the change to the *.h we care about via the *.c
  being changed.

  For API changes that happened only via *.h files we'd do the wrong
  thing before this change, but e.g. for function additions (not
  "static inline" ones) catch the *.h change by proxy.

Now we'll instead:

 * Create a <RULE>/<FILE> pair in the .build directory, E.g. for
   swap.cocci and grep.c we'll create
   .build/contrib/coccinelle/swap.cocci.patch/grep.c.

   That file is the diff we'll apply for that <RULE>-<FILE>
   combination, if there's no changes to me made (the common case)
   it'll be an empty file.

 * Our generated *.patch
   file (e.g. contrib/coccinelle/swap.cocci.patch) is now a simple "cat
   $^" of all of all of the <RULE>/<FILE> files for a given <RULE>.

   In the case discussed above of "grep.c" being changed we'll do the
   full "cat" every time, so they resulting *.cocci.patch will always
   be correct and up-to-date, even if it's "incrementally updated".

   See 1cc0425a27c (Makefile: have "make pot" not "reset --hard",
   2022-05-26) for another recent rule that used that technique.

As before we'll:

 * End up generating a contrib/coccinelle/swap.cocci.patch, if we
   "fail" by creating a non-empty patch we'll still exit with a zero
   exit code.

   Arguably we should move to a more Makefile-native way of doing
   this, i.e. fail early, and if we want all of the "failed" changes
   we can use "make -k", but as the current
   "ci/run-static-analysis.sh" expects us to behave this way let's
   keep the existing behavior of exhaustively discovering all cocci
   changes, and only failing if spatch itself errors out.

Further implementation details & notes:

 * Before this change running "make coccicheck" would by default end
   up pegging just one CPU at the very end for a while, usually as
   we'd finish whichever *.cocci rule was the most expensive.

   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.

   There will be cases where getting rid of "SPATCH_BATCH_SIZE" makes
   things worse, but a from-scratch "make coccicheck" with the default
   of SPATCH_BATCH_SIZE=1 (and tweaking it doesn't make a difference)
   is faster (~3m36s v.s. ~3m56s) with this approach, as we can feed
   the CPU more work in a less staggered way.

 * Getting rid of "SPATCH_BATCH_SIZE" particularly helps in cases
   where the default of 1 yields parallelism under "make coccicheck",
   but then running e.g.:

       make -W contrib/coccinelle/swap.cocci coccicheck

   I.e. before that would use only one CPU core, until the user
   remembered to adjust "SPATCH_BATCH_SIZE" differently than the
   setting that makes sense when doing a non-incremental run of "make
   coccicheck".

Outstanding issues & future work:

 * We could get rid of "--all-includes" in favor of manually
   specifying a list of includes to give to "spatch(1)".

   As noted upthread of [1] a naïve removal of "--all-includes" will
   result in broken *.cocci patches, but if we know the exhaustive
   list of includes via COMPUTE_HEADER_DEPENDENCIES we don't need to
   re-scan for them, we could grab the headers to include from the
   .depend.d/<file>.o.d and supply them with the "--include" option to
   spatch(1).q

1. https://lore.kernel.org/git/87ft18tcog.fsf@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile   | 83 +++++++++++++++++++++++++++++++++++++-----------------
 shared.mak |  3 +-
 2 files changed, 59 insertions(+), 27 deletions(-)

On Wed, Aug 31, 2022 at 10:57:54PM +0200, Ævar Arnfjörð Bjarmason wrote:

On my machine a "from scratch" 'make -j4 coccicheck' without the inacceptably slow 'unused.cocci' takes 9m28s, with SPATCH_BATCH_SIZE=32 it takes 6m39s. If we invoke 'spatch' like in the patch below and let one 'spatch' invocation process all source files one by one (i.e. unlike the batched invocations) and using its '--jobs' option then it takes 4m56s.

This patch series is slower than any of the above, as it takes 10m3s.

As pointed out previously, this is not the case.

  ---  >8  ---

On Thu, Sep 01 2022, SZEDER Gábor wrote:

But is such from-scratch building your primary use-case?

When I e.g. do a "make coccicheck" on master, then:

	git merge gitster/ac/bitmap-lookup-table

Or whatever, it takes much less time than before, ditto things like:

	time make -W grep.h contrib/coccinelle/free.cocci.patch

I.e. changing a file and wanting to re-run it for all of its reverse dependencies (this is with the *.o files).

Yes we discussed this in v1, I just missed this in the re-roll. Sorry! Will fix it in the next iteration.

Yes, I think it's unfortunate that there *are* faster ways to run it, but they rely on doing things in batches of N, which doesn't really jive well with make's incremental model of the world.

But I think we can either have that, or the ability to incrementally (re)build.

Do you see some easy way to have our cake & eat it too here? If you/others really feel strongly about this use case I could also keep the old method, and just hide this all behind a SPATCH_INCREMENTAL=Y flag or something. But I'm hoping we can turn this (mostly) into some win-win.

Add a rather trivial "spatchcache", with this running e.g.:

	make cocciclean
	make contrib/coccinelle/free.cocci.patch \
		SPATCH=contrib/coccicheck/spatchcache \
		SPATCH_FLAGS=--very-quiet

Is cut down from ~20s to ~5s on my system. Much of that is either fixable shell overhead, or the around 40 files we "CANTCACHE" (see the implementation).

This uses "redis" as a cache by default, but it's configurable. See the embedded documentation.

This is *not* like ccache in that we won't cache failed spatch invocations, or those where spatch suggests changes for us. Those cases are so rare that I didn't think it was worth the bother, by far the most common case is that it has no suggested changes. We'll also refuse to cache any "spatch" invocation that has output on stderr, which means that "--very-quiet" must be added to "SPATCH_FLAGS".

Because we narrow the cache to that we don't need to save away stdout, stderr & the exit code. We simply cache the cases where we had no suggested changes.

Another benchmark is to compare this with the previous SPATCH_BATCH_SIZE=N, as noted in [1]. Before this (on my 8 core system) running:

	make clean; time make contrib/coccinelle/array.cocci.patch SPATCH_BATCH_SIZE=0

Would take 33s, but with the preceding changes running without this "spatchcache" is slightly slower, or around 35s:

	make clean; time make contrib/coccinelle/array.cocci.patch

Now doing the same with SPATCH=contrib/coccinelle/spatchcache will take around 6s, but we'll need to compile the *.o files first to take full advantage of it (which can be fast with "ccache"):

	make clean; make; time make contrib/coccinelle/array.cocci.patch SPATCH=contrib/coccinelle/spatchcache

1. https://lore.kernel.org/git/YwdRqP1CyUAzCEn2@coredump.intra.peff.net/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/coccinelle/spatchcache | 234 +++++++++++++++++++++++++++++++++
 1 file changed, 234 insertions(+)
 create mode 100755 contrib/coccinelle/spatchcache

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"

In f7ff6597a75 (cocci: add a "coccicheck-test" target and test *.cocci rules, 2022-07-05) we abbreviated "_TEST" to "_T" to have it align with the rest of the "="'s above it.

Subsequent commits will add more QUIET_SPATCH_* variables, so let's stop abbreviating this, and indent it in preparation for adding more of these variables.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile   | 2 +-
 shared.mak | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

Fix an issue with a rule added in 9b45f499818 (object-store: prepare has_{sha1, object}_file to handle any repo, 2018-11-13). We've been spewing out this warning into our $@.log since that rule was added:

	warning: rule starting on line 21: metavariable F not used in the - or context code

We should do a better job of scouring our coccinelle log files for such issues, but for now let's fix this as a one-off.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/coccinelle/the_repository.pending.cocci | 1 -
 1 file changed, 1 deletion(-)

Split off the "; setting[...]" part of the comment added in In 960154b9c17 (coccicheck: optionally batch spatch invocations, 2019-05-06), and restore what we had before that, which was a comment indicating that variables for the "coccicheck" target were being set here.

When 960154b9c17 amended the heading to discuss SPATCH_BATCH_SIZE it left no natural place to add a new comment about other flags that preceded it. As subsequent commits will add such comments we need to split the existing comment up.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

On Fri, Oct 14, 2022 at 05:31:21PM +0200, Ævar Arnfjörð Bjarmason wrote:

I suppose it goes away once the coccicheck rule becomes incremental a few patches later, but I did find this rewrapping odd.

Thanks, Taylor

Declare the contrib/coccinelle/<rule>.cocci.patch rules in such a way as to allow TAB-completion, and slightly optimize the Makefile by cutting down on the number of $(wildcard) in favor of defining "coccicheck" and "coccicheck-pending" in terms of the same incrementally filtered list.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

Per the rationale in 7b63ea57500 (Makefile: remove mandatory "spatch" arguments from SPATCH_FLAGS, 2022-07-05) we have certain flags that are truly mandatory, such as "--sp-file" and "--patch .". The "--all-includes" flag is also critical, but per [1] we might want to ad-hoc tweak it occasionally for testing or one-offs.

But being unable to set e.g. SPATCH_FLAGS="--verbose-parsing" without breaking how our "spatch" works isn't ideal, i.e. before this we'd need to know about the default include flags, and specify: SPATCH_FLAGS="--all-includes --verbose-parsing".

If we were then to change the default include flag (e.g. to "--recursive-includes") in the future any such one-off commands would need to be correspondingly updated.

Let's instead leave the SPATCH_FLAGS for the user, while creating a new SPATCH_INCLUDE_FLAGS to allow for ad-hoc testing of the include strategy itself.

1. https://lore.kernel.org/git/20220823095733.58685-1-szeder.dev@gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Fix an issue with the "coccicheck" family of rules that's been here since 63f0a758a06 (add coccicheck make target, 2016-09-15), unlike e.g. "make grep.o" we wouldn't re-run it when $(SPATCH) or $(SPATCH_FLAGS) changed. To test new flags we needed to first do a "make cocciclean".

This now uses the same (copy/pasted) pattern as other "DEFINES" rules. As a result we'll re-run properly. This can be demonstrated e.g. on the issue noted in [1]:

	$ make contrib/coccinelle/xcalloc.cocci.patch COCCI_SOURCES=promisor-remote.c V=1
	[...]
	    SPATCH contrib/coccinelle/xcalloc.cocci
	$ make contrib/coccinelle/xcalloc.cocci.patch COCCI_SOURCES=promisor-remote.c SPATCH_FLAGS="--all-includes --recursive-includes"
	    * new spatch flags
	    SPATCH contrib/coccinelle/xcalloc.cocci
	     SPATCH result: contrib/coccinelle/xcalloc.cocci.patch
	$

1. https://lore.kernel.org/git/20220823095602.GC1735@szeder.dev/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .gitignore |  1 +
 Makefile   | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

Amend the "coccicheck-test" rule added in f7ff6597a75 (cocci: add a "coccicheck-test" target and test *.cocci rules, 2022-07-05) to stop using "--all-includes". The flags we'll need for the tests are different than the ones we'll need for our main source code.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Optimize the very slow "coccicheck" target to take advantage of incremental rebuilding, and fix outstanding dependency problems with the existing rule.

The rule is now faster both on the initial run as we can make better use of GNU make's parallelism than the old ad-hoc combination of make's parallelism combined with $(SPATCH_BATCH_SIZE) and/or the "--jobs" argument to "spatch(1)".

It also makes us *much* faster when incrementally building, it's now viable to "make coccicheck" as topic branches are merged down.

The rule didn't use FORCE (or its equivalents) before, so a:

	make coccicheck
	make coccicheck

Would report nothing to do on the second iteration. But all of our patch output depended on all $(COCCI_SOURCES) files, therefore e.g.:

    make -W grep.c coccicheck

Would do a full re-run, i.e. a a change in a single file would force us to do a full re-run.

The reason for this (not the initial rationale, but my analysis) is:

* Since we create a single "*.cocci.patch+" we don't know where to
  pick up where we left off, or how to incrementally merge e.g. a
  "grep.c" change with an existing *.cocci.patch.

* We've been carrying forward the dependency on the *.c files since
  63f0a758a06 (add coccicheck make target, 2016-09-15) the rule was
  initially added as a sort of poor man's dependency discovery.

  As we don't include other *.c files depending on other *.c files
  has always been broken, as could be trivially demonstrated
  e.g. with:

       make coccicheck
       make -W strbuf.h coccicheck

  However, depending on the corresponding *.c files has been doing
  something, namely that *if* an API change modified both *.c and *.h
  files we'd catch the change to the *.h we care about via the *.c
  being changed.

  For API changes that happened only via *.h files we'd do the wrong
  thing before this change, but e.g. for function additions (not
  "static inline" ones) catch the *.h change by proxy.

Now we'll instead:

 * Create a <RULE>/<FILE> pair in the .build directory, E.g. for
   swap.cocci and grep.c we'll create
   .build/contrib/coccinelle/swap.cocci.patch/grep.c.

   That file is the diff we'll apply for that <RULE>-<FILE>
   combination, if there's no changes to me made (the common case)
   it'll be an empty file.

 * Our generated *.patch
   file (e.g. contrib/coccinelle/swap.cocci.patch) is now a simple "cat
   $^" of all of all of the <RULE>/<FILE> files for a given <RULE>.

   In the case discussed above of "grep.c" being changed we'll do the
   full "cat" every time, so they resulting *.cocci.patch will always
   be correct and up-to-date, even if it's "incrementally updated".

   See 1cc0425a27c (Makefile: have "make pot" not "reset --hard",
   2022-05-26) for another recent rule that used that technique.

As before we'll:

 * End up generating a contrib/coccinelle/swap.cocci.patch, if we
   "fail" by creating a non-empty patch we'll still exit with a zero
   exit code.

   Arguably we should move to a more Makefile-native way of doing
   this, i.e. fail early, and if we want all of the "failed" changes
   we can use "make -k", but as the current
   "ci/run-static-analysis.sh" expects us to behave this way let's
   keep the existing behavior of exhaustively discovering all cocci
   changes, and only failing if spatch itself errors out.

Further implementation details & notes:

 * Before this change running "make coccicheck" would by default end
   up pegging just one CPU at the very end for a while, usually as
   we'd finish whichever *.cocci rule was the most expensive.

   This could be mitigated by combining "make -jN" with
   SPATCH_BATCH_SIZE, see 960154b9c17 (coccicheck: optionally batch
   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
   of SPATCH_BATCH_SIZE=1 (and tweaking it doesn't make a difference)
   is faster (~3m36s v.s. ~3m56s) with this approach, as we can feed
   the CPU more work in a less staggered way.

 * Getting rid of "SPATCH_BATCH_SIZE" particularly helps in cases
   where the default of 1 yields parallelism under "make coccicheck",
   but then running e.g.:

       make -W contrib/coccinelle/swap.cocci coccicheck

   I.e. before that would use only one CPU core, until the user
   remembered to adjust "SPATCH_BATCH_SIZE" differently than the
   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
   specifying a list of includes to give to "spatch(1)".

   As noted upthread of [1] a naïve removal of "--all-includes" will
   result in broken *.cocci patches, but if we know the exhaustive
   list of includes via COMPUTE_HEADER_DEPENDENCIES we don't need to
   re-scan for them, we could grab the headers to include from the
   .depend.d/<file>.o.d and supply them with the "--include" option to
   spatch(1).q

1. https://lore.kernel.org/git/87ft18tcog.fsf@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile   | 84 +++++++++++++++++++++++++++++++++++++-----------------
 shared.mak |  5 +++-
 2 files changed, 62 insertions(+), 27 deletions(-)

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@szeder.dev/

   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@gmail.com>
---
 Makefile | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Add a rather trivial "spatchcache", with this running e.g.:

	make cocciclean
	make contrib/coccinelle/free.cocci.patch \
		SPATCH=contrib/coccicheck/spatchcache \
		SPATCH_FLAGS=--very-quiet

Is cut down from ~20s to ~5s on my system. Much of that is either fixable shell overhead, or the around 40 files we "CANTCACHE" (see the implementation).

This uses "redis" as a cache by default, but it's configurable. See the embedded documentation.

This is *not* like ccache in that we won't cache failed spatch invocations, or those where spatch suggests changes for us. Those cases are so rare that I didn't think it was worth the bother, by far the most common case is that it has no suggested changes. We'll also refuse to cache any "spatch" invocation that has output on stderr, which means that "--very-quiet" must be added to "SPATCH_FLAGS".

Because we narrow the cache to that we don't need to save away stdout, stderr & the exit code. We simply cache the cases where we had no suggested changes.

Another benchmark is to compare this with the previous SPATCH_BATCH_SIZE=N, as noted in [1]. Before this (on my 8 core system) running:

	make clean; time make contrib/coccinelle/array.cocci.patch SPATCH_BATCH_SIZE=0

Would take 33s, but with the preceding changes running without this "spatchcache" is slightly slower, or around 35s:

	make clean; time make contrib/coccinelle/array.cocci.patch

Now doing the same with SPATCH=contrib/coccinelle/spatchcache will take around 6s, but we'll need to compile the *.o files first to take full advantage of it (which can be fast with "ccache"):

	make clean; make; time make contrib/coccinelle/array.cocci.patch SPATCH=contrib/coccinelle/spatchcache

1. https://lore.kernel.org/git/YwdRqP1CyUAzCEn2@coredump.intra.peff.net/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/coccinelle/spatchcache | 272 +++++++++++++++++++++++++++++++++
 1 file changed, 272 insertions(+)
 create mode 100755 contrib/coccinelle/spatchcache

The preceding commits to make the "coccicheck" target incremental made it slower in some cases. As an optimization let's not have the many=many mapping of <*.cocci>=<*.[ch]>, but instead concat the <*.cocci> into an ALL.cocci, and then run one-to-many ALL.cocci=<*.[ch]>.

A "make coccicheck" is now around 2x as fast as it was on "master", and around 1.5x as fast as the preceding change to make the run incremental:

	$ git hyperfine -L rev origin/master,HEAD~,HEAD -p 'make clean' 'make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' -r 3
	Benchmark 1: make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'origin/master
	  Time (mean ± σ):      4.258 s ±  0.015 s    [User: 27.432 s, System: 1.532 s]
	  Range (min … max):    4.241 s …  4.268 s    3 runs

	Benchmark 2: make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'HEAD~
	  Time (mean ± σ):      5.365 s ±  0.079 s    [User: 36.899 s, System: 1.810 s]
	  Range (min … max):    5.281 s …  5.436 s    3 runs

	Benchmark 3: make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'HEAD
	  Time (mean ± σ):      2.725 s ±  0.063 s    [User: 14.796 s, System: 0.233 s]
	  Range (min … max):    2.667 s …  2.792 s    3 runs

	Summary
	  'make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'HEAD' ran
	    1.56 ± 0.04 times faster than 'make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'origin/master'
	    1.97 ± 0.05 times faster than 'make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'HEAD~'

This can be turned off with SPATCH_CONCAT_COCCI, but as the beneficiaries of "SPATCH_CONCAT_COCCI=" would mainly be those developing the *.cocci rules themselves, let's leave this optimization on by default.

For more information see my "Optimizing *.cocci rules by concat'ing them" (<220901.8635dbjfko.gmgdl@evledraar.gmail.com>) on the cocci@inria.fr mailing list.

This potentially changes the results of our *.cocci rules, but as noted in that discussion it should be safe for our use. We don't name rules, or if we do their names don't conflict across our *.cocci files.

To the extent that we'd have any inter-dependencies between rules this doesn't make that worse, as we'd have them now if we ran "make coccicheck", applied the results, and would then have (due to hypothetical interdependencies) suggested changes on the subsequent "make coccicheck".

Our "coccicheck-test" target makes use of the ALL.cocci when running tests, e.g. when testing unused.{c,out} we test it against ALL.cocci, not unused.cocci. We thus assert (to the extent that we have test coverage) that this concatenation doesn't change the expected results of running these rules.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile                      | 40 ++++++++++++++++++++++++++++++++---
 contrib/coccinelle/.gitignore |  1 +
 shared.mak                    |  1 +
 3 files changed, 39 insertions(+), 3 deletions(-)

On Fri, Oct 14, 2022 at 05:31:16PM +0200, Ævar Arnfjörð Bjarmason wrote:

Is that safe? The last time it was brought up on this list (AFAIK) was in:

  https://lore.kernel.org/git/alpine.DEB.2.20.1808030755350.2446@hadrien/

where Julia said:

  I'm surprised that the above cat command would work.  Semantic patch rules
  have names, and Coccinelle will not be happy isf two rules have the same
  name.  Some may also have variables declared in initializers, although
  perhaps the ones in the kernel don't do this.  Causing these variables to
  be shared would not have a good effect.

  Putting everything together can also go counter to the optimizations that
  Coccinelle provides. [...]

Maybe we don't do any of the things that could trigger problems in our spatch rules. But it's not clear to me what we're risking. Do you have a link for further discussion?

-Peff

On Mon, Oct 17 2022, Jeff King wrote:

I think 10/11's commit message should answer your question: https://lore.kernel.org/git/patch-v3-10.11-52177ea2a68-20221014T152553Z-avarab@gmail.com/

The tl;dr is that it's not safe in the general case, as noted in the post you & the more recent one I linked to in 10/11.

So, with this series doing:

	perl -pi -e 's/swap/preincrement/g' contrib/coccinelle/swap.cocci

Will error it if you run it with "SPATCH_CONCAT_COCCI=Y", but not with "SPATCH_CONCAT_COCCI=", as the rule names conflict in the ALL.cocci.

But as 10/11 notes we can just avoid this by not picking conflicting names, which doesn't seem like an undue burden.

AFAICT we have 5 named rules, and seemingly only 1/4 actually needs its
name, the rest are apparently only using it for self-documentation, and
we could either remove the name, or accomplish the same with a comment:
	
	diff --git a/contrib/coccinelle/hashmap.cocci b/contrib/coccinelle/hashmap.cocci
	index d69e120ccff..c5dbb4557b5 100644
	--- a/contrib/coccinelle/hashmap.cocci
	+++ b/contrib/coccinelle/hashmap.cocci
	@@ -1,4 +1,4 @@
	-@ hashmap_entry_init_usage @
	+@@
	 expression E;
	 struct hashmap_entry HME;
	 @@
	diff --git a/contrib/coccinelle/preincr.cocci b/contrib/coccinelle/preincr.cocci
	index 7fe1e8d2d9a..ae42cb07302 100644
	--- a/contrib/coccinelle/preincr.cocci
	+++ b/contrib/coccinelle/preincr.cocci
	@@ -1,4 +1,4 @@
	-@ preincrement @
	+@@
	 identifier i;
	 @@
	 - ++i > 1
	diff --git a/contrib/coccinelle/strbuf.cocci b/contrib/coccinelle/strbuf.cocci
	index 0970d98ad72..5f06105df6d 100644
	--- a/contrib/coccinelle/strbuf.cocci
	+++ b/contrib/coccinelle/strbuf.cocci
	@@ -1,4 +1,4 @@
	-@ strbuf_addf_with_format_only @
	+@@
	 expression E;
	 constant fmt !~ "%";
	 @@
	diff --git a/contrib/coccinelle/swap.cocci b/contrib/coccinelle/swap.cocci
	index a0934d1fdaf..522177afb66 100644
	--- a/contrib/coccinelle/swap.cocci
	+++ b/contrib/coccinelle/swap.cocci
	@@ -1,4 +1,4 @@
	-@ swap_with_declaration @
	+@@
	 type T;
	 identifier tmp;
	 T a, b;
	

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

As long as it is clearly documented to make it well known to everybody who would attempt to touch or add .cocci files, I am OK with that direction.

Well, if they are for self-documentation, they'd better be unique, or they are not self-documenting enough, no ;-)?

On Mon, Oct 17, 2022 at 08:36:46PM +0200, Ævar Arnfjörð Bjarmason wrote:

Thanks. Holy cow, the coccinelle list is hard to use compared to lore/public-inbox. The direct link to the thread in question is:

  https://sympa.inria.fr/sympa/arc/cocci/2022-09/msg00001.html

(you'll need to click a javascript button to see it, though).

Yeah, that seems OK, then. In fact, I'd be fine with guidance in the README saying "don't bother with a name if you don't need it".

-Peff

A re-roll of the series to have "make coccicheck" run incrementally. For the v1 and general overview see: https://lore.kernel.org/git/cover-0.5-00000000000-20220825T141212Z-avarab@gmail.com/

For me a "make coccicheck" on master takes ~7m, with "SPATCH_BATCH_SIZE=0" (the usual way to speed it up on a beefy box) it's ~6m.

With this topic it takes 2m30s with no caching, and using the "spatchcache" tool in 12/12 10s. The ~6-7m to ~2m30s speed-up is mostly 11/12, i.e. we now concatenate the rules.

But being able to run this in ~10s once you set up "spatchcache" means that incremental runs become viable (e.g. sanity checks when merging topics in a loop).

These numbers are on a 8 core colo'd box, so YMMV. For GitHub CI the "static-analysis" (which has more than just "make coccicheck") now takes ~8m instead of ~15m.

Changes since v4:

 * update contrib/coccinelle/README as we go, see the range-diff
   below.

 * Added a 10/12 to remove named rules that didn't need their names,
   as we make an ALL.cocci these are more likely to collide.

 * I made the spatchcache tool faster, it spend ~40ms per run on a
   cache hit, mostly on running "git config", now it can be told via
   environment variables to use its default, and not ask the config,
   which makes it take ~10ms on a cache hit.

 * We now explain in 11/12's README addition that rule names are now
   global, as we concatenate all the *.cocci files.

Ævar Arnfjörð Bjarmason (12):
  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 rules: remove <id>'s from rules that don't need them
  cocci: run against a generated ALL.cocci
  spatchcache: add a ccache-alike for "spatch"

 .gitignore                                    |   1 +
 Makefile                                      | 159 +++++++--
 contrib/coccinelle/.gitignore                 |   1 +
 contrib/coccinelle/README                     |  49 +++
 contrib/coccinelle/hashmap.cocci              |   2 +-
 contrib/coccinelle/preincr.cocci              |   2 +-
 contrib/coccinelle/spatchcache                | 304 ++++++++++++++++++
 contrib/coccinelle/strbuf.cocci               |   2 +-
 contrib/coccinelle/swap.cocci                 |   2 +-
 .../coccinelle/the_repository.pending.cocci   |   1 -
 shared.mak                                    |  10 +-
 11 files changed, 501 insertions(+), 32 deletions(-)
 create mode 100755 contrib/coccinelle/spatchcache

Range-diff against v3:
 1:  4494c91df9a =  1:  c0306e3c397 Makefile + shared.mak: rename and indent $(QUIET_SPATCH_T)
 2:  8219b1b12f2 =  2:  14e757cc020 cocci rules: remove unused "F" metavariable from pending rule
 3:  6dbfafa08fd =  3:  210dbc6b101 Makefile: add ability to TAB-complete cocci *.patch rules
 4:  f779a2d22aa =  4:  b859dae7509 Makefile: have "coccicheck" re-run if flags change
 5:  ab25b586f38 !  5:  11b9227722b Makefile: split off SPATCH_BATCH_SIZE comment from "cocci" heading
    @@ Commit message
         preceded it. As subsequent commits will add such comments we need to
         split the existing comment up.
     
    +    The wrapping for the "SPATCH_BATCH_SIZE" is now a bit odd, but
    +    minimizes the diff size. As a subsequent commit will remove that
    +    feature altogether this is worth it.
    +
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Makefile ##
 6:  691be73b6fb =  6:  1c30f4b543b cocci: split off include-less "tests" from SPATCH_FLAGS
 7:  2ca5ea5beca =  7:  86827388627 cocci: split off "--all-includes" from SPATCH_FLAGS
 8:  2072a508064 =  8:  340ce523436 cocci: make "coccicheck" rule incremental
 9:  739652eada9 !  9:  134b3743a60 cocci: optimistically use COMPUTE_HEADER_DEPENDENCIES
    @@ Makefile: endef
      endef
      
      ifdef COCCI_GOALS
    +
    + ## contrib/coccinelle/README ##
    +@@ contrib/coccinelle/README: 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.
 -:  ----------- > 10:  89f778ac055 cocci rules: remove <id>'s from rules that don't need them
10:  52177ea2a68 ! 11:  a848d09527f cocci: run against a generated ALL.cocci
    @@ contrib/coccinelle/.gitignore
     +/ALL.cocci
      *.patch*
     
    + ## contrib/coccinelle/README ##
    +@@ contrib/coccinelle/README: Git-specific tips & things to know about how we run "spatch":
    + 
    +    To disable this behavior use the "SPATCH_USE_O_DEPENDENCIES=NoThanks"
    +    flag.
    ++
    ++ * To speed up our rules the "make coccicheck" target will by default
    ++   concatenate all of the *.cocci files here into an "ALL.cocci", and
    ++   apply it to each source file.
    ++
    ++   This makes the run faster, as we don't need to run each rule
    ++   against each source file. See the Makefile for further discussion,
    ++   this behavior can be disabled with "SPATCH_CONCAT_COCCI=".
    ++
    ++   But since they're concatenated any <id> in the <rulname> (e.g. "@
    ++   my_name", v.s. anonymous "@@") needs to be unique across all our
    ++   *.cocci files. You should only need to name rules if other rules
    ++   depend on them (currently only one rule is named).
    +
      ## shared.mak ##
     @@ shared.mak: ifndef V
      ## Used in "Makefile": SPATCH
11:  f7ca3f9c9af ! 12:  a728aa301a6 spatchcache: add a ccache-alike for "spatch"
    @@ Commit message
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
    + ## contrib/coccinelle/README ##
    +@@ contrib/coccinelle/README: Git-specific tips & things to know about how we run "spatch":
    +    my_name", v.s. anonymous "@@") needs to be unique across all our
    +    *.cocci files. You should only need to name rules if other rules
    +    depend on them (currently only one rule is named).
    ++
    ++ * To speed up incremental runs even more use the "spatchcache" tool
    ++   in this directory as your "SPATCH". It aimns to be a "ccache" for
    ++   coccinelle, and piggy-backs on "COMPUTE_HEADER_DEPENDENCIES".
    ++
    ++   It caches in Redis by default, see it source for a how-to.
    ++
    ++   In one setup with a primed cache "make coccicheck" followed by a
    ++   "make clean && make" takes around 10s to run, but 2m30s with the
    ++   default of "SPATCH_CONCAT_COCCI=Y".
    ++
    ++   With "SPATCH_CONCAT_COCCI=" the total runtime is around ~6m, sped
    ++   up to ~1m with "spatchcache".
    ++
    ++   Most of the 10s (or ~1m) being spent on re-running "spatch" on
    ++   files we couldn't cache, as we didn't compile them (in contrib/*
    ++   and compat/* mostly).
    ++
    ++   The absolute times will differ for you, but the relative speedup
    ++   from caching should be on that order.
    +
      ## contrib/coccinelle/spatchcache (new) ##
     @@
     +#!/bin/sh
    @@ contrib/coccinelle/spatchcache (new)
     +# driving "spatch" in git.git, in particular if we invoke:
     +#
     +#	make
    ++#	# See "spatchCache.cacheWhenStderr" for why "--very-quiet" is
    ++#	# used
     +#	make coccicheck SPATCH_FLAGS=--very-quiet
     +#
     +# We can with COMPUTE_HEADER_DEPENDENCIES (auto-detected as true with
    @@ contrib/coccinelle/spatchcache (new)
     +# 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.
    ++#
    ++# Reading the config can make the command much slower, to work around
    ++# this the config can be set in the environment, with environment
    ++# variable name corresponding to the config key. "default" can be used
    ++# to use whatever's the script default, e.g. setting
    ++# spatchCache.cacheWhenStderr=true and deferring to the defaults for
    ++# the rest is:
    ++#
    ++#	export GIT_CONTRIB_SPATCHCACHE_DEBUG=default
    ++#	export GIT_CONTRIB_SPATCHCACHE_TRACE=default
    ++#	export GIT_CONTRIB_SPATCHCACHE_CACHEWHENSTDERR=true
    ++#	export GIT_CONTRIB_SPATCHCACHE_SPATCH=default
    ++#	export GIT_CONTRIB_SPATCHCACHE_DEPENDFORMAT=default
    ++#	export GIT_CONTRIB_SPATCHCACHE_SETCMD=default
    ++#	export GIT_CONTRIB_SPATCHCACHE_GETCMD=default
     +
     +set -e
     +
    ++env_or_config () {
    ++	env="$1"
    ++	shift
    ++	if test "$env" = "default"
    ++	then
    ++		# Avoid expensive "git config" invocation
    ++		return
    ++	elif test -n "$env"
    ++	then
    ++		echo "$env"
    ++	else
    ++		git config $@ || :
    ++	fi
    ++}
    ++
     +## Our own configuration & options
    -+debug=$(git config --bool "spatchCache.debug")
    ++debug=$(env_or_config "$GIT_CONTRIB_SPATCHCACHE_DEBUG" --bool "spatchCache.debug")
     +if test "$debug" != "true"
     +then
     +	debug=
    @@ contrib/coccinelle/spatchcache (new)
     +	set -x
     +fi
     +
    -+trace=$(git config --bool "spatchCache.trace")
    ++trace=$(env_or_config "$GIT_CONTRIB_SPATCHCACHE_TRACE" --bool "spatchCache.trace")
     +if test "$trace" != "true"
     +then
     +	trace=
    @@ contrib/coccinelle/spatchcache (new)
     +	trace=true
     +fi
     +
    -+cacheWhenStderr=$(git config --bool "spatchCache.cacheWhenStderr")
    ++cacheWhenStderr=$(env_or_config "$GIT_CONTRIB_SPATCHCACHE_CACHEWHENSTDERR" --bool "spatchCache.cacheWhenStderr")
     +if test "$cacheWhenStderr" != "true"
     +then
     +	cacheWhenStderr=
    @@ contrib/coccinelle/spatchcache (new)
     +	echo "$@" >&2
     +}
     +
    -+spatch=$(git config --path "spatchCache.spatch" || :)
    ++spatch=$(env_or_config "$GIT_CONTRIB_SPATCHCACHE_SPATCH" --path "spatchCache.spatch")
     +if test -n "$spatch"
     +then
     +	if test -n "$debug"
    @@ contrib/coccinelle/spatchcache (new)
     +fi
     +
     +dependFormat='$dirname/.depend/${basename%.c}.o.d'
    -+dependFormatCfg=$(git config "spatchCache.dependFormat" || :)
    ++dependFormatCfg=$(env_or_config "$GIT_CONTRIB_SPATCHCACHE_DEPENDFORMAT" "spatchCache.dependFormat")
     +if test -n "$dependFormatCfg"
     +then
     +	dependFormat="$dependFormatCfg"
     +fi
     +
    -+set=$(git config spatchCache.setCmd || :)
    -+get=$(git config spatchCache.getCmd || :)
    ++set=$(env_or_config "$GIT_CONTRIB_SPATCHCACHE_SETCMD" "spatchCache.setCmd")
    ++get=$(env_or_config "$GIT_CONTRIB_SPATCHCACHE_GETCMD" "spatchCache.getCmd")
     +
     +## Parse spatch()-like command-line for caching info
     +arg_sp=

In f7ff6597a75 (cocci: add a "coccicheck-test" target and test *.cocci rules, 2022-07-05) we abbreviated "_TEST" to "_T" to have it align with the rest of the "="'s above it.

Subsequent commits will add more QUIET_SPATCH_* variables, so let's stop abbreviating this, and indent it in preparation for adding more of these variables.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile   | 2 +-
 shared.mak | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

Fix an issue with a rule added in 9b45f499818 (object-store: prepare has_{sha1, object}_file to handle any repo, 2018-11-13). We've been spewing out this warning into our $@.log since that rule was added:

	warning: rule starting on line 21: metavariable F not used in the - or context code

We should do a better job of scouring our coccinelle log files for such issues, but for now let's fix this as a one-off.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/coccinelle/the_repository.pending.cocci | 1 -
 1 file changed, 1 deletion(-)

Declare the contrib/coccinelle/<rule>.cocci.patch rules in such a way as to allow TAB-completion, and slightly optimize the Makefile by cutting down on the number of $(wildcard) in favor of defining "coccicheck" and "coccicheck-pending" in terms of the same incrementally filtered list.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

Fix an issue with the "coccicheck" family of rules that's been here since 63f0a758a06 (add coccicheck make target, 2016-09-15), unlike e.g. "make grep.o" we wouldn't re-run it when $(SPATCH) or $(SPATCH_FLAGS) changed. To test new flags we needed to first do a "make cocciclean".

This now uses the same (copy/pasted) pattern as other "DEFINES" rules. As a result we'll re-run properly. This can be demonstrated e.g. on the issue noted in [1]:

	$ make contrib/coccinelle/xcalloc.cocci.patch COCCI_SOURCES=promisor-remote.c V=1
	[...]
	    SPATCH contrib/coccinelle/xcalloc.cocci
	$ make contrib/coccinelle/xcalloc.cocci.patch COCCI_SOURCES=promisor-remote.c SPATCH_FLAGS="--all-includes --recursive-includes"
	    * new spatch flags
	    SPATCH contrib/coccinelle/xcalloc.cocci
	     SPATCH result: contrib/coccinelle/xcalloc.cocci.patch
	$

1. https://lore.kernel.org/git/20220823095602.GC1735@szeder.dev/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .gitignore |  1 +
 Makefile   | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

Split off the "; setting[...]" part of the comment added in In 960154b9c17 (coccicheck: optionally batch spatch invocations, 2019-05-06), and restore what we had before that, which was a comment indicating that variables for the "coccicheck" target were being set here.

When 960154b9c17 amended the heading to discuss SPATCH_BATCH_SIZE it left no natural place to add a new comment about other flags that preceded it. As subsequent commits will add such comments we need to split the existing comment up.

The wrapping for the "SPATCH_BATCH_SIZE" is now a bit odd, but minimizes the diff size. As a subsequent commit will remove that feature altogether this is worth it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Per the rationale in 7b63ea57500 (Makefile: remove mandatory "spatch" arguments from SPATCH_FLAGS, 2022-07-05) we have certain flags that are truly mandatory, such as "--sp-file" and "--patch .". The "--all-includes" flag is also critical, but per [1] we might want to ad-hoc tweak it occasionally for testing or one-offs.

But being unable to set e.g. SPATCH_FLAGS="--verbose-parsing" without breaking how our "spatch" works isn't ideal, i.e. before this we'd need to know about the default include flags, and specify: SPATCH_FLAGS="--all-includes --verbose-parsing".

If we were then to change the default include flag (e.g. to "--recursive-includes") in the future any such one-off commands would need to be correspondingly updated.

Let's instead leave the SPATCH_FLAGS for the user, while creating a new SPATCH_INCLUDE_FLAGS to allow for ad-hoc testing of the include strategy itself.

1. https://lore.kernel.org/git/20220823095733.58685-1-szeder.dev@gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Amend the "coccicheck-test" rule added in f7ff6597a75 (cocci: add a "coccicheck-test" target and test *.cocci rules, 2022-07-05) to stop using "--all-includes". The flags we'll need for the tests are different than the ones we'll need for our main source code.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Optimize the very slow "coccicheck" target to take advantage of incremental rebuilding, and fix outstanding dependency problems with the existing rule.

The rule is now faster both on the initial run as we can make better use of GNU make's parallelism than the old ad-hoc combination of make's parallelism combined with $(SPATCH_BATCH_SIZE) and/or the "--jobs" argument to "spatch(1)".

It also makes us *much* faster when incrementally building, it's now viable to "make coccicheck" as topic branches are merged down.

The rule didn't use FORCE (or its equivalents) before, so a:

	make coccicheck
	make coccicheck

Would report nothing to do on the second iteration. But all of our patch output depended on all $(COCCI_SOURCES) files, therefore e.g.:

    make -W grep.c coccicheck

Would do a full re-run, i.e. a a change in a single file would force us to do a full re-run.

The reason for this (not the initial rationale, but my analysis) is:

* Since we create a single "*.cocci.patch+" we don't know where to
  pick up where we left off, or how to incrementally merge e.g. a
  "grep.c" change with an existing *.cocci.patch.

* We've been carrying forward the dependency on the *.c files since
  63f0a758a06 (add coccicheck make target, 2016-09-15) the rule was
  initially added as a sort of poor man's dependency discovery.

  As we don't include other *.c files depending on other *.c files
  has always been broken, as could be trivially demonstrated
  e.g. with:

       make coccicheck
       make -W strbuf.h coccicheck

  However, depending on the corresponding *.c files has been doing
  something, namely that *if* an API change modified both *.c and *.h
  files we'd catch the change to the *.h we care about via the *.c
  being changed.

  For API changes that happened only via *.h files we'd do the wrong
  thing before this change, but e.g. for function additions (not
  "static inline" ones) catch the *.h change by proxy.

Now we'll instead:

 * Create a <RULE>/<FILE> pair in the .build directory, E.g. for
   swap.cocci and grep.c we'll create
   .build/contrib/coccinelle/swap.cocci.patch/grep.c.

   That file is the diff we'll apply for that <RULE>-<FILE>
   combination, if there's no changes to me made (the common case)
   it'll be an empty file.

 * Our generated *.patch
   file (e.g. contrib/coccinelle/swap.cocci.patch) is now a simple "cat
   $^" of all of all of the <RULE>/<FILE> files for a given <RULE>.

   In the case discussed above of "grep.c" being changed we'll do the
   full "cat" every time, so they resulting *.cocci.patch will always
   be correct and up-to-date, even if it's "incrementally updated".

   See 1cc0425a27c (Makefile: have "make pot" not "reset --hard",
   2022-05-26) for another recent rule that used that technique.

As before we'll:

 * End up generating a contrib/coccinelle/swap.cocci.patch, if we
   "fail" by creating a non-empty patch we'll still exit with a zero
   exit code.

   Arguably we should move to a more Makefile-native way of doing
   this, i.e. fail early, and if we want all of the "failed" changes
   we can use "make -k", but as the current
   "ci/run-static-analysis.sh" expects us to behave this way let's
   keep the existing behavior of exhaustively discovering all cocci
   changes, and only failing if spatch itself errors out.

Further implementation details & notes:

 * Before this change running "make coccicheck" would by default end
   up pegging just one CPU at the very end for a while, usually as
   we'd finish whichever *.cocci rule was the most expensive.

   This could be mitigated by combining "make -jN" with
   SPATCH_BATCH_SIZE, see 960154b9c17 (coccicheck: optionally batch
   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
   of SPATCH_BATCH_SIZE=1 (and tweaking it doesn't make a difference)
   is faster (~3m36s v.s. ~3m56s) with this approach, as we can feed
   the CPU more work in a less staggered way.

 * Getting rid of "SPATCH_BATCH_SIZE" particularly helps in cases
   where the default of 1 yields parallelism under "make coccicheck",
   but then running e.g.:

       make -W contrib/coccinelle/swap.cocci coccicheck

   I.e. before that would use only one CPU core, until the user
   remembered to adjust "SPATCH_BATCH_SIZE" differently than the
   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
   specifying a list of includes to give to "spatch(1)".

   As noted upthread of [1] a naïve removal of "--all-includes" will
   result in broken *.cocci patches, but if we know the exhaustive
   list of includes via COMPUTE_HEADER_DEPENDENCIES we don't need to
   re-scan for them, we could grab the headers to include from the
   .depend.d/<file>.o.d and supply them with the "--include" option to
   spatch(1).q

1. https://lore.kernel.org/git/87ft18tcog.fsf@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile   | 84 +++++++++++++++++++++++++++++++++++++-----------------
 shared.mak |  5 +++-
 2 files changed, 62 insertions(+), 27 deletions(-)

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@szeder.dev/

   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@gmail.com>
---
 Makefile                  | 15 +++++++++++++--
 contrib/coccinelle/README | 16 ++++++++++++++++
 2 files changed, 29 insertions(+), 2 deletions(-)

The <id> in the <rulename> part of the coccinelle syntax[1] is for our purposes there to declares if we have inter-dependencies between different rules.

But such <id>'s must be unique within a given semantic patch file. As we'll be processing a concatenated version of our rules in the subsequent commit let's remove these names. They weren't being used for the semantic patches themselves, and equated to a short comment about the rule.

Both the filename and context of the rules makes it clear what they're doing, so we're not gaining anything from keeping these. Retaining them goes against recommendations that "contrib/coccinelle/README" will be making in the subsequent commit.

This leaves only one named rule in our sources, where it's needed for a "<id> <-> <extends> <id>" relationship:

	$ git -P grep '^@ ' -- contrib/coccinelle/
	contrib/coccinelle/swap.cocci:@ swap @
	contrib/coccinelle/swap.cocci:@ extends swap @

1. https://coccinelle.gitlabpages.inria.fr/website/docs/main_grammar.html

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/coccinelle/hashmap.cocci | 2 +-
 contrib/coccinelle/preincr.cocci | 2 +-
 contrib/coccinelle/strbuf.cocci  | 2 +-
 contrib/coccinelle/swap.cocci    | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

The preceding commits to make the "coccicheck" target incremental made it slower in some cases. As an optimization let's not have the many=many mapping of <*.cocci>=<*.[ch]>, but instead concat the <*.cocci> into an ALL.cocci, and then run one-to-many ALL.cocci=<*.[ch]>.

A "make coccicheck" is now around 2x as fast as it was on "master", and around 1.5x as fast as the preceding change to make the run incremental:

	$ git hyperfine -L rev origin/master,HEAD~,HEAD -p 'make clean' 'make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' -r 3
	Benchmark 1: make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'origin/master
	  Time (mean ± σ):      4.258 s ±  0.015 s    [User: 27.432 s, System: 1.532 s]
	  Range (min … max):    4.241 s …  4.268 s    3 runs

	Benchmark 2: make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'HEAD~
	  Time (mean ± σ):      5.365 s ±  0.079 s    [User: 36.899 s, System: 1.810 s]
	  Range (min … max):    5.281 s …  5.436 s    3 runs

	Benchmark 3: make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'HEAD
	  Time (mean ± σ):      2.725 s ±  0.063 s    [User: 14.796 s, System: 0.233 s]
	  Range (min … max):    2.667 s …  2.792 s    3 runs

	Summary
	  'make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'HEAD' ran
	    1.56 ± 0.04 times faster than 'make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'origin/master'
	    1.97 ± 0.05 times faster than 'make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'HEAD~'

This can be turned off with SPATCH_CONCAT_COCCI, but as the beneficiaries of "SPATCH_CONCAT_COCCI=" would mainly be those developing the *.cocci rules themselves, let's leave this optimization on by default.

For more information see my "Optimizing *.cocci rules by concat'ing them" (<220901.8635dbjfko.gmgdl@evledraar.gmail.com>) on the cocci@inria.fr mailing list.

This potentially changes the results of our *.cocci rules, but as noted in that discussion it should be safe for our use. We don't name rules, or if we do their names don't conflict across our *.cocci files.

To the extent that we'd have any inter-dependencies between rules this doesn't make that worse, as we'd have them now if we ran "make coccicheck", applied the results, and would then have (due to hypothetical interdependencies) suggested changes on the subsequent "make coccicheck".

Our "coccicheck-test" target makes use of the ALL.cocci when running tests, e.g. when testing unused.{c,out} we test it against ALL.cocci, not unused.cocci. We thus assert (to the extent that we have test coverage) that this concatenation doesn't change the expected results of running these rules.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile                      | 40 ++++++++++++++++++++++++++++++++---
 contrib/coccinelle/.gitignore |  1 +
 contrib/coccinelle/README     | 13 ++++++++++++
 shared.mak                    |  1 +
 4 files changed, 52 insertions(+), 3 deletions(-)

On Wed, Oct 26, 2022 at 04:20:38PM +0200, Ævar Arnfjörð Bjarmason wrote:

This should also remove 'contrib/coccinelle/ALL.cocci.patch'.

Add a rather trivial "spatchcache", with this running e.g.:

	make cocciclean
	make contrib/coccinelle/free.cocci.patch \
		SPATCH=contrib/coccicheck/spatchcache \
		SPATCH_FLAGS=--very-quiet

Is cut down from ~20s to ~5s on my system. Much of that is either fixable shell overhead, or the around 40 files we "CANTCACHE" (see the implementation).

This uses "redis" as a cache by default, but it's configurable. See the embedded documentation.

This is *not* like ccache in that we won't cache failed spatch invocations, or those where spatch suggests changes for us. Those cases are so rare that I didn't think it was worth the bother, by far the most common case is that it has no suggested changes. We'll also refuse to cache any "spatch" invocation that has output on stderr, which means that "--very-quiet" must be added to "SPATCH_FLAGS".

Because we narrow the cache to that we don't need to save away stdout, stderr & the exit code. We simply cache the cases where we had no suggested changes.

Another benchmark is to compare this with the previous SPATCH_BATCH_SIZE=N, as noted in [1]. Before this (on my 8 core system) running:

	make clean; time make contrib/coccinelle/array.cocci.patch SPATCH_BATCH_SIZE=0

Would take 33s, but with the preceding changes running without this "spatchcache" is slightly slower, or around 35s:

	make clean; time make contrib/coccinelle/array.cocci.patch

Now doing the same with SPATCH=contrib/coccinelle/spatchcache will take around 6s, but we'll need to compile the *.o files first to take full advantage of it (which can be fast with "ccache"):

	make clean; make; time make contrib/coccinelle/array.cocci.patch SPATCH=contrib/coccinelle/spatchcache

1. https://lore.kernel.org/git/YwdRqP1CyUAzCEn2@coredump.intra.peff.net/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/coccinelle/README      |  20 +++
 contrib/coccinelle/spatchcache | 304 +++++++++++++++++++++++++++++++++
 2 files changed, 324 insertions(+)
 create mode 100755 contrib/coccinelle/spatchcache

A re-roll of the series to have "make coccicheck" run incrementally. For the v1 and general overview see: https://lore.kernel.org/git/cover-0.5-00000000000-20220825T141212Z-avarab@gmail.com/

For a performance overview see the v4 CL: http://lore.kernel.org/git/cover-v4-00.12-00000000000-20221026T141005Z-avarab@gmail.com

Updates since v4:

 * SZEDER pointed out I needed to remove the ALL.cocci. I did that by
   having that ALL.cocci generated in .build/contrib/coccinelle,
   instead among the user-maintained *.cocci files in
   contrib/coccinelle.

   I think it leaves things in a beter place, but there's some churn
   from 8/13 onwards in service of that.
 * To make that easier the *.cocci files are copied to .build before
   being executed, which makes making ALL.cocci and the
   "non-generated" *.cocci easier.
 * Fixed typo/breakage in "make coccicheck-pending" Updated .gitignore
 * rule along with 8/13 (no more *.patch+ files.FixeFixeFixeFixe
 * A few other misc changes (e.g. commit mesages, docs), see
   range-diff below.

Passing CI for this: https://github.com/avar/git/actions/runs/3363433825/jobs/5586038736; It finished static-analysis in ~7m now, instead of ~17m (master result at: https://github.com/git/git/actions/runs/3358829617/jobs/5566255602)

Ævar Arnfjörð Bjarmason (13):
  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
  Makefile: copy contrib/coccinelle/*.cocci to build/
  cocci rules: remove <id>'s from rules that don't need them
  cocci: run against a generated ALL.cocci
  spatchcache: add a ccache-alike for "spatch"

 .gitignore                                    |   1 +
 Makefile                                      | 175 ++++++++--
 contrib/coccinelle/.gitignore                 |   2 +-
 contrib/coccinelle/README                     |  49 +++
 contrib/coccinelle/hashmap.cocci              |   2 +-
 contrib/coccinelle/preincr.cocci              |   2 +-
 contrib/coccinelle/spatchcache                | 304 ++++++++++++++++++
 contrib/coccinelle/strbuf.cocci               |   2 +-
 contrib/coccinelle/swap.cocci                 |   2 +-
 .../coccinelle/the_repository.pending.cocci   |   1 -
 shared.mak                                    |   8 +-
 11 files changed, 515 insertions(+), 33 deletions(-)
 create mode 100755 contrib/coccinelle/spatchcache

Range-diff against v4:
 1:  c0306e3c397 =  1:  1c77706db26 Makefile + shared.mak: rename and indent $(QUIET_SPATCH_T)
 2:  14e757cc020 =  2:  d8556f94cb4 cocci rules: remove unused "F" metavariable from pending rule
 3:  210dbc6b101 =  3:  6bf908044eb Makefile: add ability to TAB-complete cocci *.patch rules
 4:  b859dae7509 =  4:  460025dbbf6 Makefile: have "coccicheck" re-run if flags change
 5:  11b9227722b =  5:  b00ceccf1d1 Makefile: split off SPATCH_BATCH_SIZE comment from "cocci" heading
 6:  1c30f4b543b =  6:  04d268e246b cocci: split off include-less "tests" from SPATCH_FLAGS
 7:  86827388627 =  7:  cae46461ebf cocci: split off "--all-includes" from SPATCH_FLAGS
 8:  340ce523436 !  8:  ee9ad433ab5 cocci: make "coccicheck" rule incremental
    @@ Makefile: check: $(GENERATED_H)
     +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)
    -+
    +@@ Makefile: COCCICHECK = $(filter-out $(COCCICHECK_PENDING),$(COCCI_RULES))
    + COCCICHECK_PATCHES = $(COCCICHECK:%=%.patch)
    + COCCICHECK_PATCHES_PENDING = $(COCCICHECK_PENDING:%=%.patch)
    + 
     +# It's expensive to compute the many=many rules below, only eval them
     +# on $(MAKECMDGOALS) that match these $(COCCI_RULES)
     +COCCI_RULES_GLOB =
    @@ Makefile: check: $(GENERATED_H)
     +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)
    - 
    ++
      COCCI_TEST_RES = $(wildcard contrib/coccinelle/tests/*.res)
      
     -COCCI_PATCHES = $(COCCI_RULES:%=%.patch)
    @@ Makefile: check: $(GENERATED_H)
     +define spatch-rule
     +
     +contrib/coccinelle/$(1).cocci.patch: $$(COCCI_$(1))
    -+	$(QUIET_SPATCH_CAT_TMPL)cat $$^ >$$@ && \
    ++	$$(QUIET_SPATCH_CAT)cat $$^ >$$@ && \
     +	if test -s $$@; \
      	then \
     -		echo '    ' SPATCH result: $@; \
    @@ Makefile: profile-clean:
      	$(RM) GIT-SPATCH-DEFINES
      	$(RM) -r .build/contrib/coccinelle
     -	$(RM) contrib/coccinelle/*.cocci.patch*
    -+	$(RM) $(COCCICHECK_PATCHES)
    -+	$(RM) $(COCCICHECK_PATCHES_PENDING)
    ++	$(RM) contrib/coccinelle/*.cocci.patch
      
      clean: profile-clean coverage-clean cocciclean
      	$(RM) -r .build
     
    + ## contrib/coccinelle/.gitignore ##
    +@@
    +-*.patch*
    ++*.patch
    +
      ## shared.mak ##
     @@ shared.mak: ifndef V
      	QUIET_RC       = @echo '   ' RC $@;
    @@ shared.mak: ifndef V
     -	QUIET_SPATCH			= @echo '   ' SPATCH $<;
     +	QUIET_SPATCH			= @echo '   ' SPATCH $@;
      	QUIET_SPATCH_TEST		= @echo '   ' SPATCH TEST $(@:.build/%=%);
    ++	QUIET_SPATCH_CAT		= @echo '   ' SPATCH CAT $$^ \>$@;
      
    -+## 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 $@;
 9:  134b3743a60 =  9:  332fb6c8640 cocci: optimistically use COMPUTE_HEADER_DEPENDENCIES
 -:  ----------- > 10:  56ca8f5720a Makefile: copy contrib/coccinelle/*.cocci to build/
10:  89f778ac055 = 11:  73bb897c965 cocci rules: remove <id>'s from rules that don't need them
11:  a848d09527f ! 12:  55d2f638359 cocci: run against a generated ALL.cocci
    @@ Makefile: SPATCH_TEST_FLAGS =
     +#
     +# - Makes error and performance analysis harder, as rules will be
     +#   applied from a monolithic ALL.cocci, rather than
    -+#   e.g. strbuf.cocci.
    ++#   e.g. strbuf.cocci. To work around this either undefine this, or
    ++#   generate a specific patch, e.g. this will always use strbuf.cocci,
    ++#   not ALL.cocci:
    ++#
    ++#	make contrib/coccinelle/strbuf.cocci.patch
     +SPATCH_CONCAT_COCCI = YesPlease
     +
      # Rebuild 'coccicheck' if $(SPATCH), its flags etc. change
    @@ Makefile: check: $(GENERATED_H)
      		exit 1; \
      	fi
      
    -+COCCI_GEN_ALL = contrib/coccinelle/ALL.cocci
    ++COCCI_GEN_ALL = .build/contrib/coccinelle/ALL.cocci
      COCCI_GLOB = $(wildcard contrib/coccinelle/*.cocci)
    --COCCI_RULES = $(COCCI_GLOB)
    --COCCI_NAMES = $(COCCI_RULES:contrib/coccinelle/%.cocci=%)
    -+COCCI_NAMES = $(sort ALL $(COCCI_GLOB:contrib/coccinelle/%.cocci=%))
    -+COCCI_RULES = $(filter-out $(COCCI_GEN_ALL),$(COCCI_GLOB))
    - 
    - COCCICHECK_PENDING = $(filter %.pending.cocci,$(COCCI_RULES))
    - COCCICHECK_PATCHES_PENDING = $(COCCICHECK_PENDING:%=%.patch)
    -@@ Makefile: COCCI_RULES_GLOB += cocci%
    - COCCI_RULES_GLOB += .build/contrib/coccinelle/%
    - COCCI_RULES_GLOB += $(COCCICHECK_PATCHES)
    - COCCI_RULES_GLOB += $(COCCICHEC_PATCHES_PENDING)
    -+COCCI_RULES_GLOB += $(COCCI_GEN_ALL)
    - COCCI_GOALS = $(filter $(COCCI_RULES_GLOB),$(MAKECMDGOALS))
    - 
    - COCCICHECK_PATCHES = $(COCCICHECK:%=%.patch)
    -@@ Makefile: COCCI_TEST_RES = $(wildcard contrib/coccinelle/tests/*.res)
    + COCCI_RULES_TRACKED = $(COCCI_GLOB:%=.build/%)
    ++COCCI_RULES_TRACKED_NO_PENDING = $(filter-out %.pending.cocci,$(COCCI_RULES_TRACKED))
    + COCCI_RULES =
    ++COCCI_RULES += $(COCCI_GEN_ALL)
    + COCCI_RULES += $(COCCI_RULES_TRACKED)
    + COCCI_NAMES =
    + COCCI_NAMES += $(COCCI_RULES:.build/contrib/coccinelle/%.cocci=%)
    +@@ Makefile: $(COCCI_RULES_TRACKED): .build/% : %
      	$(call mkdir_p_parent_template)
      	$(QUIET_GEN) >$@
      
    -+$(COCCI_GEN_ALL): $(COCCICHECK)
    ++$(COCCI_GEN_ALL): $(COCCI_RULES_TRACKED_NO_PENDING)
     +	$(call mkdir_p_parent_template)
     +	$(QUIET_SPATCH_CAT)cat $^ >$@
     +
      ifeq ($(COMPUTE_HEADER_DEPENDENCIES),no)
      SPATCH_USE_O_DEPENDENCIES =
      endif
    -@@ Makefile: $(foreach s,$(COCCI_SOURCES),$(call cocci-rule,$(c),$(s),$(s:%.c=%.o)))
    - endef
    - 
    - ifdef COCCI_GOALS
    --$(eval $(foreach c,$(COCCI_RULES),$(call cocci-matrix,$(c))))
    -+$(eval $(foreach c,$(COCCI_RULES) $(COCCI_GEN_ALL),$(call cocci-matrix,$(c))))
    - endif
    - 
    - define spatch-rule
     @@ Makefile: COCCI_TEST_RES_GEN = $(addprefix .build/,$(COCCI_TEST_RES))
      $(COCCI_TEST_RES_GEN): GIT-SPATCH-DEFINES
      $(COCCI_TEST_RES_GEN): .build/%.res : %.c
    @@ Makefile: $(COCCI_TEST_RES_GEN): .build/contrib/coccinelle/tests/%.res : contrib
     +ifdef SPATCH_CONCAT_COCCI
     +coccicheck: contrib/coccinelle/ALL.cocci.patch
     +else
    - coccicheck: $(COCCICHECK_PATCHES)
    + coccicheck: $(COCCICHECK_PATCHES_INTREE)
     +endif
      
      # See contrib/coccinelle/README
      coccicheck-pending: coccicheck-test
    -@@ Makefile: cocciclean:
    - 	$(RM) -r .build/contrib/coccinelle
    - 	$(RM) $(COCCICHECK_PATCHES)
    - 	$(RM) $(COCCICHECK_PATCHES_PENDING)
    -+	$(RM) $(COCCI_GEN_ALL)
    - 
    - clean: profile-clean coverage-clean cocciclean
    - 	$(RM) -r .build
    -
    - ## contrib/coccinelle/.gitignore ##
    -@@
    -+/ALL.cocci
    - *.patch*
     
      ## contrib/coccinelle/README ##
     @@ contrib/coccinelle/README: Git-specific tips & things to know about how we run "spatch":
    @@ contrib/coccinelle/README: Git-specific tips & things to know about how we run "
     +   my_name", v.s. anonymous "@@") needs to be unique across all our
     +   *.cocci files. You should only need to name rules if other rules
     +   depend on them (currently only one rule is named).
    -
    - ## shared.mak ##
    -@@ shared.mak: ifndef V
    - ## Used in "Makefile": SPATCH
    - 	QUIET_SPATCH			= @echo '   ' SPATCH $@;
    - 	QUIET_SPATCH_TEST		= @echo '   ' SPATCH TEST $(@:.build/%=%);
    -+	QUIET_SPATCH_CAT		= @echo '   ' SPATCH CAT $$^ \>$@;
    - 
    - ## Used in "Makefile": SPATCH_*TMPL (quoted for use in "define"'s)
    - 	QUIET_SPATCH_CAT_TMPL		= @echo '   ' SPATCH CAT $$$$^ \>$$@;
12:  a728aa301a6 = 13:  ef578c230ea spatchcache: add a ccache-alike for "spatch"

In f7ff6597a75 (cocci: add a "coccicheck-test" target and test *.cocci rules, 2022-07-05) we abbreviated "_TEST" to "_T" to have it align with the rest of the "="'s above it.

Subsequent commits will add more QUIET_SPATCH_* variables, so let's stop abbreviating this, and indent it in preparation for adding more of these variables.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile   | 2 +-
 shared.mak | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

Fix an issue with a rule added in 9b45f499818 (object-store: prepare has_{sha1, object}_file to handle any repo, 2018-11-13). We've been spewing out this warning into our $@.log since that rule was added:

	warning: rule starting on line 21: metavariable F not used in the - or context code

We should do a better job of scouring our coccinelle log files for such issues, but for now let's fix this as a one-off.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/coccinelle/the_repository.pending.cocci | 1 -
 1 file changed, 1 deletion(-)

Declare the contrib/coccinelle/<rule>.cocci.patch rules in such a way as to allow TAB-completion, and slightly optimize the Makefile by cutting down on the number of $(wildcard) in favor of defining "coccicheck" and "coccicheck-pending" in terms of the same incrementally filtered list.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

Fix an issue with the "coccicheck" family of rules that's been here since 63f0a758a06 (add coccicheck make target, 2016-09-15), unlike e.g. "make grep.o" we wouldn't re-run it when $(SPATCH) or $(SPATCH_FLAGS) changed. To test new flags we needed to first do a "make cocciclean".

This now uses the same (copy/pasted) pattern as other "DEFINES" rules. As a result we'll re-run properly. This can be demonstrated e.g. on the issue noted in [1]:

	$ make contrib/coccinelle/xcalloc.cocci.patch COCCI_SOURCES=promisor-remote.c V=1
	[...]
	    SPATCH contrib/coccinelle/xcalloc.cocci
	$ make contrib/coccinelle/xcalloc.cocci.patch COCCI_SOURCES=promisor-remote.c SPATCH_FLAGS="--all-includes --recursive-includes"
	    * new spatch flags
	    SPATCH contrib/coccinelle/xcalloc.cocci
	     SPATCH result: contrib/coccinelle/xcalloc.cocci.patch
	$

1. https://lore.kernel.org/git/20220823095602.GC1735@szeder.dev/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .gitignore |  1 +
 Makefile   | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

Amend the "coccicheck-test" rule added in f7ff6597a75 (cocci: add a "coccicheck-test" target and test *.cocci rules, 2022-07-05) to stop using "--all-includes". The flags we'll need for the tests are different than the ones we'll need for our main source code.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Split off the "; setting[...]" part of the comment added in In 960154b9c17 (coccicheck: optionally batch spatch invocations, 2019-05-06), and restore what we had before that, which was a comment indicating that variables for the "coccicheck" target were being set here.

When 960154b9c17 amended the heading to discuss SPATCH_BATCH_SIZE it left no natural place to add a new comment about other flags that preceded it. As subsequent commits will add such comments we need to split the existing comment up.

The wrapping for the "SPATCH_BATCH_SIZE" is now a bit odd, but minimizes the diff size. As a subsequent commit will remove that feature altogether this is worth it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Per the rationale in 7b63ea57500 (Makefile: remove mandatory "spatch" arguments from SPATCH_FLAGS, 2022-07-05) we have certain flags that are truly mandatory, such as "--sp-file" and "--patch .". The "--all-includes" flag is also critical, but per [1] we might want to ad-hoc tweak it occasionally for testing or one-offs.

But being unable to set e.g. SPATCH_FLAGS="--verbose-parsing" without breaking how our "spatch" works isn't ideal, i.e. before this we'd need to know about the default include flags, and specify: SPATCH_FLAGS="--all-includes --verbose-parsing".

If we were then to change the default include flag (e.g. to "--recursive-includes") in the future any such one-off commands would need to be correspondingly updated.

Let's instead leave the SPATCH_FLAGS for the user, while creating a new SPATCH_INCLUDE_FLAGS to allow for ad-hoc testing of the include strategy itself.

1. https://lore.kernel.org/git/20220823095733.58685-1-szeder.dev@gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Optimize the very slow "coccicheck" target to take advantage of incremental rebuilding, and fix outstanding dependency problems with the existing rule.

The rule is now faster both on the initial run as we can make better use of GNU make's parallelism than the old ad-hoc combination of make's parallelism combined with $(SPATCH_BATCH_SIZE) and/or the "--jobs" argument to "spatch(1)".

It also makes us *much* faster when incrementally building, it's now viable to "make coccicheck" as topic branches are merged down.

The rule didn't use FORCE (or its equivalents) before, so a:

	make coccicheck
	make coccicheck

Would report nothing to do on the second iteration. But all of our patch output depended on all $(COCCI_SOURCES) files, therefore e.g.:

    make -W grep.c coccicheck

Would do a full re-run, i.e. a a change in a single file would force us to do a full re-run.

The reason for this (not the initial rationale, but my analysis) is:

* Since we create a single "*.cocci.patch+" we don't know where to
  pick up where we left off, or how to incrementally merge e.g. a
  "grep.c" change with an existing *.cocci.patch.

* We've been carrying forward the dependency on the *.c files since
  63f0a758a06 (add coccicheck make target, 2016-09-15) the rule was
  initially added as a sort of poor man's dependency discovery.

  As we don't include other *.c files depending on other *.c files
  has always been broken, as could be trivially demonstrated
  e.g. with:

       make coccicheck
       make -W strbuf.h coccicheck

  However, depending on the corresponding *.c files has been doing
  something, namely that *if* an API change modified both *.c and *.h
  files we'd catch the change to the *.h we care about via the *.c
  being changed.

  For API changes that happened only via *.h files we'd do the wrong
  thing before this change, but e.g. for function additions (not
  "static inline" ones) catch the *.h change by proxy.

Now we'll instead:

 * Create a <RULE>/<FILE> pair in the .build directory, E.g. for
   swap.cocci and grep.c we'll create
   .build/contrib/coccinelle/swap.cocci.patch/grep.c.

   That file is the diff we'll apply for that <RULE>-<FILE>
   combination, if there's no changes to me made (the common case)
   it'll be an empty file.

 * Our generated *.patch
   file (e.g. contrib/coccinelle/swap.cocci.patch) is now a simple "cat
   $^" of all of all of the <RULE>/<FILE> files for a given <RULE>.

   In the case discussed above of "grep.c" being changed we'll do the
   full "cat" every time, so they resulting *.cocci.patch will always
   be correct and up-to-date, even if it's "incrementally updated".

   See 1cc0425a27c (Makefile: have "make pot" not "reset --hard",
   2022-05-26) for another recent rule that used that technique.

As before we'll:

 * End up generating a contrib/coccinelle/swap.cocci.patch, if we
   "fail" by creating a non-empty patch we'll still exit with a zero
   exit code.

   Arguably we should move to a more Makefile-native way of doing
   this, i.e. fail early, and if we want all of the "failed" changes
   we can use "make -k", but as the current
   "ci/run-static-analysis.sh" expects us to behave this way let's
   keep the existing behavior of exhaustively discovering all cocci
   changes, and only failing if spatch itself errors out.

Further implementation details & notes:

 * Before this change running "make coccicheck" would by default end
   up pegging just one CPU at the very end for a while, usually as
   we'd finish whichever *.cocci rule was the most expensive.

   This could be mitigated by combining "make -jN" with
   SPATCH_BATCH_SIZE, see 960154b9c17 (coccicheck: optionally batch
   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
   of SPATCH_BATCH_SIZE=1 (and tweaking it doesn't make a difference)
   is faster (~3m36s v.s. ~3m56s) with this approach, as we can feed
   the CPU more work in a less staggered way.

 * Getting rid of "SPATCH_BATCH_SIZE" particularly helps in cases
   where the default of 1 yields parallelism under "make coccicheck",
   but then running e.g.:

       make -W contrib/coccinelle/swap.cocci coccicheck

   I.e. before that would use only one CPU core, until the user
   remembered to adjust "SPATCH_BATCH_SIZE" differently than the
   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
   specifying a list of includes to give to "spatch(1)".

   As noted upthread of [1] a naïve removal of "--all-includes" will
   result in broken *.cocci patches, but if we know the exhaustive
   list of includes via COMPUTE_HEADER_DEPENDENCIES we don't need to
   re-scan for them, we could grab the headers to include from the
   .depend.d/<file>.o.d and supply them with the "--include" option to
   spatch(1).q

1. https://lore.kernel.org/git/87ft18tcog.fsf@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile                      | 81 ++++++++++++++++++++++++-----------
 contrib/coccinelle/.gitignore |  2 +-
 shared.mak                    |  3 +-
 3 files changed, 58 insertions(+), 28 deletions(-)

On Tue, Nov 01, 2022 at 11:35:50PM +0100, Ævar Arnfjörð Bjarmason wrote:

Indeed, and the output of 'make coccicheck' with this patch looks like this:

    MKDIR -p .build/contrib/coccinelle/hashmap.cocci.patch
    SPATCH .build/contrib/coccinelle/hashmap.cocci.patch/upload-pack.c
    SPATCH .build/contrib/coccinelle/hashmap.cocci.patch/merge-ort-wrappers.c
    SPATCH .build/contrib/coccinelle/hashmap.cocci.patch/unpack-trees.c
    SPATCH .build/contrib/coccinelle/hashmap.cocci.patch/gpg-interface.c
    SPATCH .build/contrib/coccinelle/hashmap.cocci.patch/linear-assignment.c

This is misleading, because it appears as 'spatch' would generate C source files, but that's not the case, of course.

Change the "coccinelle" rule so that we first copy the *.cocci source in e.g. "contrib/coccinelle/strbuf.cocci" to ".build/contrib/coccinelle/strbuf.cocci" before operating on it.

For now this serves as a rather pointless indirection, but prepares us for the subsequent commit where we'll be able to inject generated *.cocci files. Having the entire dependency tree live inside .build/* simplifies both the globbing we'd need to do, and any "clean" rules.

It will also help for future targets which will want to act on the generated patches or the logs, e.g. targets to alert if we can't parse certain files (or, less so than usual) with "spatch", and e.g. a replacement for "ci/run-static-analysis.sh". Such a replacement won't care about placing the patches in the in-tree, only whether they're "OK" (and about the diff).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile   | 27 +++++++++++++++++++++------
 shared.mak |  1 +
 2 files changed, 22 insertions(+), 6 deletions(-)

On Tue, Nov 01, 2022 at 11:35:52PM +0100, Ævar Arnfjörð Bjarmason wrote:

After this patch the output of 'make coccicheck' looks like this:

    CP contrib/coccinelle/hashmap.cocci .build/contrib/coccinelle/hashmap.cocci
    MKDIR -p .build/.build/contrib/coccinelle/hashmap.cocci.patch
    SPATCH .build/.build/contrib/coccinelle/hashmap.cocci.patch/upload-pack.c
    SPATCH .build/.build/contrib/coccinelle/hashmap.cocci.patch/merge-ort-wrappers.c
    SPATCH .build/.build/contrib/coccinelle/hashmap.cocci.patch/unpack-trees.c
    SPATCH .build/.build/contrib/coccinelle/hashmap.cocci.patch/gpg-interface.c
    SPATCH .build/.build/contrib/coccinelle/hashmap.cocci.patch/linear-assignment.c

Notice how there is not one but two leading '.build' path components. Surely one would be enough :)

This also breaks 'make cocciclean':

  $ make cocciclean
  rm -f GIT-SPATCH-DEFINES
  rm -f -r .build/contrib/coccinelle
  rm -f contrib/coccinelle/*.cocci.patch
  $ find .build/
  .build/
  .build/contrib
  .build/.build
  .build/.build/contrib
  .build/.build/contrib/coccinelle
  .build/.build/contrib/coccinelle/hashmap.cocci.patch
  .build/.build/contrib/coccinelle/hashmap.cocci.patch/upload-pack.c
  .build/.build/contrib/coccinelle/hashmap.cocci.patch/merge-ort-wrappers.c
  .build/.build/contrib/coccinelle/hashmap.cocci.patch/unpack-trees.c
  .build/.build/contrib/coccinelle/hashmap.cocci.patch/gpg-interface.c
  .build/.build/contrib/coccinelle/hashmap.cocci.patch/linear-assignment.c

On Wed, Nov 09 2022, SZEDER Gábor wrote:

Oops, well spotted, I'll submit a patch on top soon to fix that...

...and that, i.e. the nested .build is clearly unintended...

Aside: Now that "coccicheck" is a well-behaved (well, mostly, sans the
above) target that knows its deps etc. I wonder if it makes sense to
have it clean this at all, and just leave it for "make clean". I.e. it
should clean the worktree litter, but we could just leave the
".build/contrib/coccinelle".

Anyway, that's just a thought, and something to leave for some later "what should we clean in .build" topic, if any. I'll make sure it rm -rf's the right .build/ dir, and that we put stuff in it...

Fix a couple of issues in the recently merged 0f3c55d4c2b (Merge branch 'ab/coccicheck-incremental' into next, 2022-11-08):

In copying over the "contrib/coccinelle/" rules to ".build/contrib/coccinelle/" we inadvertently ended up with a ".build/.build/contrib/coccinelle/" as well. We'd generate the per-file patches in the former, and keep the rule and overall result in the latter. E.g. running:

	make contrib/coccinelle/free.cocci.patch COCCI_SOURCES="attr.c grep.c"

Would, per "tree -a .build" yield the following result:

	.build
	├── .build
	│   └── contrib
	│       └── coccinelle
	│           └── free.cocci.patch
	│               ├── attr.c
	│               ├── attr.c.log
	│               ├── grep.c
	│               └── grep.c.log
	└── contrib
	    └── coccinelle
	        ├── FOUND_H_SOURCES
	        ├── free.cocci
	        └── free.cocci.patch

Now we'll instead generate all of our files in ".build/contrib/coccinelle/". Fixing this required renaming the directory where we keep our per-file patches, as we'd otherwise conflict with the result.

Now the per-file patch directory is named e.g. "free.cocci.d". And the end result will now be:

	.build
	└── contrib
	    └── coccinelle
	        ├── FOUND_H_SOURCES
	        ├── free.cocci
	        ├── free.cocci.d
	        │   ├── attr.c.patch
	        │   ├── attr.c.patch.log
	        │   ├── grep.c.patch
	        │   └── grep.c.patch.log
	        └── free.cocci.patch

The per-file patches now have a ".patch" file suffix, which fixes another issue reported against 0f3c55d4c2b: The summary output was confusing. Before for the "make" command above we'd emit:

	[...]
	MKDIR -p .build/contrib/coccinelle
	CP contrib/coccinelle/free.cocci .build/contrib/coccinelle/free.cocci
	GEN .build/contrib/coccinelle/FOUND_H_SOURCES
	MKDIR -p .build/.build/contrib/coccinelle/free.cocci.patch
	SPATCH .build/.build/contrib/coccinelle/free.cocci.patch/grep.c
	SPATCH .build/.build/contrib/coccinelle/free.cocci.patch/attr.c
	SPATCH CAT $^ >.build/contrib/coccinelle/free.cocci.patch
	CP .build/contrib/coccinelle/free.cocci.patch contrib/coccinelle/free.cocci.patch

But now we'll instead emit (identical output at the start omitted):

	[...]
	MKDIR -p .build/contrib/coccinelle/free.cocci.d
	SPATCH grep.c >.build/contrib/coccinelle/free.cocci.d/grep.c.patch
	SPATCH attr.c >.build/contrib/coccinelle/free.cocci.d/attr.c.patch
	SPATCH CAT .build/contrib/coccinelle/free.cocci.d/**.patch >.build/contrib/coccinelle/free.cocci.patch
	CP .build/contrib/coccinelle/free.cocci.patch contrib/coccinelle/free.cocci.patch

I.e. we have an "SPATCH" line that makes it clear that we're running against the "{attr,grep}.c" file. The "SPATCH CAT" is then altered to correspond to it, showing that we're concatenating the "free.cocci.d/**.patch" files into one generated "free.cocci.patch" at the end.

Reported-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

This fixes the issues SZEDER reported against the ab/coccicheck-incremental topic (now in "next") in https://lore.kernel.org/git/20221109150556.GE1731@szeder.dev/ and https://lore.kernel.org/git/20221109145701.GD1731@szeder.dev/

 Makefile   | 12 ++++++------
 shared.mak |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

On Thu, Nov 10, 2022 at 05:14:18PM +0100, Ævar Arnfjörð Bjarmason wrote:

Thanks for coming back for these. Will queue this on top of ab/coccicheck-incremental and re-merge back into 'next'.

Thanks, Taylor

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@szeder.dev/

   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@gmail.com>
---
 Makefile                  | 15 +++++++++++++--
 contrib/coccinelle/README | 16 ++++++++++++++++
 2 files changed, 29 insertions(+), 2 deletions(-)

The <id> in the <rulename> part of the coccinelle syntax[1] is for our purposes there to declares if we have inter-dependencies between different rules.

But such <id>'s must be unique within a given semantic patch file. As we'll be processing a concatenated version of our rules in the subsequent commit let's remove these names. They weren't being used for the semantic patches themselves, and equated to a short comment about the rule.

Both the filename and context of the rules makes it clear what they're doing, so we're not gaining anything from keeping these. Retaining them goes against recommendations that "contrib/coccinelle/README" will be making in the subsequent commit.

This leaves only one named rule in our sources, where it's needed for a "<id> <-> <extends> <id>" relationship:

	$ git -P grep '^@ ' -- contrib/coccinelle/
	contrib/coccinelle/swap.cocci:@ swap @
	contrib/coccinelle/swap.cocci:@ extends swap @

1. https://coccinelle.gitlabpages.inria.fr/website/docs/main_grammar.html

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/coccinelle/hashmap.cocci | 2 +-
 contrib/coccinelle/preincr.cocci | 2 +-
 contrib/coccinelle/strbuf.cocci  | 2 +-
 contrib/coccinelle/swap.cocci    | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

The preceding commits to make the "coccicheck" target incremental made it slower in some cases. As an optimization let's not have the many=many mapping of <*.cocci>=<*.[ch]>, but instead concat the <*.cocci> into an ALL.cocci, and then run one-to-many ALL.cocci=<*.[ch]>.

A "make coccicheck" is now around 2x as fast as it was on "master", and around 1.5x as fast as the preceding change to make the run incremental:

	$ git hyperfine -L rev origin/master,HEAD~,HEAD -p 'make clean' 'make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' -r 3
	Benchmark 1: make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'origin/master
	  Time (mean ± σ):      4.258 s ±  0.015 s    [User: 27.432 s, System: 1.532 s]
	  Range (min … max):    4.241 s …  4.268 s    3 runs

	Benchmark 2: make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'HEAD~
	  Time (mean ± σ):      5.365 s ±  0.079 s    [User: 36.899 s, System: 1.810 s]
	  Range (min … max):    5.281 s …  5.436 s    3 runs

	Benchmark 3: make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'HEAD
	  Time (mean ± σ):      2.725 s ±  0.063 s    [User: 14.796 s, System: 0.233 s]
	  Range (min … max):    2.667 s …  2.792 s    3 runs

	Summary
	  'make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'HEAD' ran
	    1.56 ± 0.04 times faster than 'make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'origin/master'
	    1.97 ± 0.05 times faster than 'make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'HEAD~'

This can be turned off with SPATCH_CONCAT_COCCI, but as the beneficiaries of "SPATCH_CONCAT_COCCI=" would mainly be those developing the *.cocci rules themselves, let's leave this optimization on by default.

For more information see my "Optimizing *.cocci rules by concat'ing them" (<220901.8635dbjfko.gmgdl@evledraar.gmail.com>) on the cocci@inria.fr mailing list.

This potentially changes the results of our *.cocci rules, but as noted in that discussion it should be safe for our use. We don't name rules, or if we do their names don't conflict across our *.cocci files.

To the extent that we'd have any inter-dependencies between rules this doesn't make that worse, as we'd have them now if we ran "make coccicheck", applied the results, and would then have (due to hypothetical interdependencies) suggested changes on the subsequent "make coccicheck".

Our "coccicheck-test" target makes use of the ALL.cocci when running tests, e.g. when testing unused.{c,out} we test it against ALL.cocci, not unused.cocci. We thus assert (to the extent that we have test coverage) that this concatenation doesn't change the expected results of running these rules.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile                  | 38 ++++++++++++++++++++++++++++++++++++++
 contrib/coccinelle/README | 13 +++++++++++++
 2 files changed, 51 insertions(+)

Add a rather trivial "spatchcache", with this running e.g.:

	make cocciclean
	make contrib/coccinelle/free.cocci.patch \
		SPATCH=contrib/coccicheck/spatchcache \
		SPATCH_FLAGS=--very-quiet

Is cut down from ~20s to ~5s on my system. Much of that is either fixable shell overhead, or the around 40 files we "CANTCACHE" (see the implementation).

This uses "redis" as a cache by default, but it's configurable. See the embedded documentation.

This is *not* like ccache in that we won't cache failed spatch invocations, or those where spatch suggests changes for us. Those cases are so rare that I didn't think it was worth the bother, by far the most common case is that it has no suggested changes. We'll also refuse to cache any "spatch" invocation that has output on stderr, which means that "--very-quiet" must be added to "SPATCH_FLAGS".

Because we narrow the cache to that we don't need to save away stdout, stderr & the exit code. We simply cache the cases where we had no suggested changes.

Another benchmark is to compare this with the previous SPATCH_BATCH_SIZE=N, as noted in [1]. Before this (on my 8 core system) running:

	make clean; time make contrib/coccinelle/array.cocci.patch SPATCH_BATCH_SIZE=0

Would take 33s, but with the preceding changes running without this "spatchcache" is slightly slower, or around 35s:

	make clean; time make contrib/coccinelle/array.cocci.patch

Now doing the same with SPATCH=contrib/coccinelle/spatchcache will take around 6s, but we'll need to compile the *.o files first to take full advantage of it (which can be fast with "ccache"):

	make clean; make; time make contrib/coccinelle/array.cocci.patch SPATCH=contrib/coccinelle/spatchcache

1. https://lore.kernel.org/git/YwdRqP1CyUAzCEn2@coredump.intra.peff.net/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/coccinelle/README      |  20 +++
 contrib/coccinelle/spatchcache | 304 +++++++++++++++++++++++++++++++++
 2 files changed, 324 insertions(+)
 create mode 100755 contrib/coccinelle/spatchcache