git/contrib
Ævar Arnfjörð Bjarmason f1c903debd cocci: make "coccicheck" rule incremental
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
  63f0a758a0 (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 1cc0425a27 (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 960154b9c1 (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>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-02 21:22:16 -04:00
..
buildsystems cmake: support local installations of git 2022-07-27 08:57:33 -07:00
coccinelle cocci: make "coccicheck" rule incremental 2022-11-02 21:22:16 -04:00
completion git-prompt: show presence of unresolved conflicts at command prompt 2022-08-19 10:58:40 -07:00
contacts git-contacts: also recognise "Reported-by:" 2017-07-27 09:42:55 -07:00
credential Merge branch 'ab/leak-check' 2022-08-12 13:19:08 -07:00
diff-highlight diff-highlight: correctly match blank lines for flush 2020-09-21 22:33:28 -07:00
emacs git{,-blame}.el: remove old bitrotting Emacs code 2018-04-16 17:25:49 +09:00
examples Merge branch 'bw/c-plus-plus' into ds/lazy-load-trees 2018-04-11 10:46:32 +09:00
fast-import import-tars: ignore the global PAX header 2020-03-24 14:39:47 -07:00
git-jump git-jump: pass "merge" arguments to ls-files 2021-11-09 11:15:21 -08:00
git-shell-commands
hg-to-git hg-to-git: make it compatible with both python3 and python2 2019-09-18 12:03:05 -07:00
hooks multimail: stop shipping a copy 2021-06-11 13:35:19 +09:00
long-running-filter docs: warn about possible '=' in clean/smudge filter process values 2016-12-06 11:29:52 -08:00
mw-to-git t6000-t9999: detect and signal failure within loop 2021-12-13 10:29:48 -08:00
persistent-https docs/config: mention protocol implications of url.insteadOf 2017-06-01 10:07:10 +09:00
remote-helpers contrib: git-remote-{bzr,hg} placeholders don't need Python 2017-03-03 11:09:34 -08:00
scalar Merge branch 'vd/scalar-enables-fsmonitor' 2022-08-29 14:55:12 -07:00
stats
subtree t/Makefile: don't remove test-results in "clean-except-prove-cache" 2022-07-27 16:35:40 -07:00
thunderbird-patch-inline contrib/thunderbird-patch-inline/appp.sh: use the $( ... ) construct for command substitution 2015-12-27 15:33:13 -08:00
update-unicode unicode_width.h: rename to use dash in file name 2018-04-11 18:11:00 +09:00
vscode vscode: improve tab size and wrapping 2022-06-27 15:37:44 -07:00
workdir git-new-workdir: mark script as LF-only 2017-05-10 13:32:50 +09:00
coverage-diff.sh contrib: add coverage-diff script 2018-10-10 10:11:35 +09:00
git-resurrect.sh contrib/git-resurrect.sh: use hash-agnostic OID pattern 2020-10-08 11:48:56 -07:00
README
remotes2config.sh
rerere-train.sh contrib/rerere-train: avoid useless gpg sign in training 2022-07-19 11:24:08 -07:00

Contributed Software

Although these pieces are available as part of the official git
source tree, they are in somewhat different status.  The
intention is to keep interesting tools around git here, maybe
even experimental ones, to give users an easier access to them,
and to give tools wider exposure, so that they can be improved
faster.

I am not expecting to touch these myself that much.  As far as
my day-to-day operation is concerned, these subdirectories are
owned by their respective primary authors.  I am willing to help
if users of these components and the contrib/ subtree "owners"
have technical/design issues to resolve, but the initiative to
fix and/or enhance things _must_ be on the side of the subtree
owners.  IOW, I won't be actively looking for bugs and rooms for
enhancements in them as the git maintainer -- I may only do so
just as one of the users when I want to scratch my own itch.  If
you have patches to things in contrib/ area, the patch should be
first sent to the primary author, and then the primary author
should ack and forward it to me (git pull request is nicer).
This is the same way as how I have been treating gitk, and to a
lesser degree various foreign SCM interfaces, so you know the
drill.

I expect that things that start their life in the contrib/ area
to graduate out of contrib/ once they mature, either by becoming
projects on their own, or moving to the toplevel directory.  On
the other hand, I expect I'll be proposing removal of disused
and inactive ones from time to time.

If you have new things to add to this area, please first propose
it on the git mailing list, and after a list discussion proves
there are some general interests (it does not have to be a
list-wide consensus for a tool targeted to a relatively narrow
audience -- for example I do not work with projects whose
upstream is svn, so I have no use for git-svn myself, but it is
of general interest for people who need to interoperate with SVN
repositories in a way git-svn works better than git-svnimport),
submit a patch to create a subdirectory of contrib/ and put your
stuff there.

-jc