[PATCH v3 00/22] pickaxe: test and refactoring for future PCRE backend

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

This much-delayed re-roll of v2[2] is a test and refactoring change to
diffcore-pickaxe.c to allow an eventual removal of the kwset.[ch]
files and to add a PCRE backend.

This series is now based on my "test-lib.sh: new test_commit args,
simplification & fixes" series. The trivial dependency between the two
is using the new test_commit --printf option.

I'll not summarize the range-diff too much, but since v2 I addressed
all outstanding feedback from Junio. There's 2x new patches at the end
of the series to change the existing early return feature now using
"discard_hunk_line" to use a flag.

In v2 I had what I realized was a WIP migration of that to using
return values in the callback instead, but unlike the
xdiff_emit_line_fn I don't think that makes sense in
xdiff_emit_hunk_fn.

1. https://lore.kernel.org/git/20210216115801.4773-1-avarab@gmail.com/
2. https://lore.kernel.org/git/cover-00.16-00000000000-20210412T110456Z-avarab@gmail.com/

Ævar Arnfjörð Bjarmason (22):
  grep/pcre2 tests: reword comments referring to kwset
  pickaxe tests: refactor to use test_commit --append --printf
  pickaxe tests: add test for diffgrep_consume() internals
  pickaxe tests: add test for "log -S" not being a regex
  pickaxe tests: test for -G, -S and --find-object incompatibility
  pickaxe tests: add missing test for --no-pickaxe-regex being an error
  pickaxe: die when -G and --pickaxe-regex are combined
  pickaxe: die when --find-object and --pickaxe-all are combined
  diff.h: move pickaxe fields together again
  pickaxe/style: consolidate declarations and assignments
  perf: add performance test for pickaxe
  pickaxe: refactor function selection in diffcore-pickaxe()
  pickaxe: assert that we must have a needle under -G or -S
  pickaxe -S: support content with NULs under --pickaxe-regex
  pickaxe: rename variables in has_changes() for brevity
  pickaxe -S: slightly optimize contains()
  xdiff-interface: prepare for allowing early return
  xdiff-interface: allow early return from xdiff_emit_line_fn
  pickaxe -G: terminate early on matching lines
  pickaxe -G: don't special-case create/delete
  xdiff users: use designated initializers for out_line
  xdiff-interface: replace discard_hunk_line() with a flag

 builtin/merge-tree.c           |   5 +-
 builtin/rerere.c               |   4 +-
 combine-diff.c                 |   5 +-
 diff.c                         |  39 +++++++----
 diff.h                         |   7 +-
 diffcore-pickaxe.c             | 106 ++++++++++++++++++------------
 range-diff.c                   |   3 +-
 t/perf/p4209-pickaxe.sh        |  70 ++++++++++++++++++++
 t/t4209-log-pickaxe.sh         | 114 ++++++++++++++++++++++++++++-----
 t/t7816-grep-binary-pattern.sh |   4 +-
 xdiff-interface.c              |  27 ++++----
 xdiff-interface.h              |  31 ++++++---
 xdiff/xdiff.h                  |   1 +
 xdiff/xemit.c                  |   3 +-
 14 files changed, 312 insertions(+), 107 deletions(-)
 create mode 100755 t/perf/p4209-pickaxe.sh

Range-diff against v2:
 1:  75bfc8eba13 =  1:  cfe934d6081 grep/pcre2 tests: reword comments referring to kwset
 2:  cddb1f75f6c <  -:  ----------- test-lib-functions: document and test test_commit --no-tag
 3:  44540f6039e <  -:  ----------- test-lib-functions: reword "test_commit --append" docs
 4:  0e1f133476f <  -:  ----------- test-lib functions: add --printf option to test_commit
 5:  2a814e8d53a !  2:  413a330d3d6 pickaxe tests: refactor to use test_commit --append --printf
    @@ Metadata
      ## Commit message ##
         pickaxe tests: refactor to use test_commit --append --printf
     
    -    Refactor existing tests added in e0e7cb8080c (log -G: ignore binary
    -    files, 2018-12-14) to use the --append option I added in
    +    Refactor the existing tests added in e0e7cb8080c (log -G: ignore
    +    binary files, 2018-12-14) to use the --append option I added in
         3373518cc8b (test-lib functions: add an --append option to
    -    test_commit, 2021-01-12) and the --printf option added in a preceding
    -    commit.
    +    test_commit, 2021-01-12) and the --printf option added as part of an
    +    in-flight topic of mine this commit depends on.
    +
    +    While I'm at it change some of the setup of the test to use a more
    +    sensible pattern, e.g. setting up a temporary repo instead of creating
    +    an orphan branch.
    +
    +    Since the -G and -S options will behave the same way with truncated
    +    and removed content also change the "git rm" to emptying data.bin,
    +    that's just catering to how test_commit works. The resulting test is
    +    shorter.
     
         See also f5d79bf7dd6 (tests: refactor a few tests to use "test_commit
         --append", 2021-01-12) for prior similar refactoring.
    @@ t/t4209-log-pickaxe.sh: test_expect_success 'log -S --no-textconv (missing textc
     -	git commit -m "delete binary file" data.bin &&
     -	git log >full-log
     +	test_create_repo GS-bin-txt &&
    -+	test_commit -C GS-bin-txt --append --printf A data.bin "a\na\0a\n" &&
    ++	test_commit -C GS-bin-txt --printf A data.bin "a\na\0a\n" &&
     +	test_commit -C GS-bin-txt --append --printf B data.bin "a\na\0a\n" &&
     +	test_commit -C GS-bin-txt C data.bin "" &&
     +	git -C GS-bin-txt log >full-log
 6:  f49eb6c95e5 !  3:  ddd2224836b pickaxe tests: add test for diffgrep_consume() internals
    @@ t/t4209-log-pickaxe.sh: test_expect_success 'log -S --no-textconv (missing textc
     +
      test_expect_success 'setup log -[GS] binary & --text' '
      	test_create_repo GS-bin-txt &&
    - 	test_commit -C GS-bin-txt --append --printf A data.bin "a\na\0a\n" &&
    + 	test_commit -C GS-bin-txt --printf A data.bin "a\na\0a\n" &&
 7:  80c62fb0448 =  4:  ca6340c1fa7 pickaxe tests: add test for "log -S" not being a regex
 8:  6d329e0c3b1 !  5:  0c4657189a8 pickaxe tests: test for -G, -S and --find-object incompatibility
    @@ t/t4209-log-pickaxe.sh: test_expect_success setup '
      
     +test_expect_success 'usage' '
     +	test_expect_code 128 git log -Gregex -Sstring 2>err &&
    -+	test_i18ngrep "mutually exclusive" err &&
    ++	grep "mutually exclusive" err &&
     +
     +	test_expect_code 128 git log -Gregex --find-object=HEAD 2>err &&
    -+	test_i18ngrep "mutually exclusive" err &&
    ++	grep "mutually exclusive" err &&
     +
    -+	test_expect_code 128 git log -Gstring --find-object=HEAD 2>err &&
    -+	test_i18ngrep "mutually exclusive" err
    ++	test_expect_code 128 git log -Sstring --find-object=HEAD 2>err &&
    ++	grep "mutually exclusive" err
     +'
     +
      test_log	expect_initial	--grep initial
 -:  ----------- >  6:  1696076bb09 pickaxe tests: add missing test for --no-pickaxe-regex being an error
 9:  bd0c3b7e3b0 !  7:  83e7b4793b6 pickaxe: die when -G and --pickaxe-regex are combined
    @@ Commit message
         matching regular expressions, 2006-03-29) only the -S option
         existed. Then when -G was added in f506b8e8b5 (git log/diff: add
         -G<regexp> that greps in the patch text, 2010-08-23) neither the
    -    documentation for --pickaxe-regex was updater accordingly, nor was
    +    documentation for --pickaxe-regex was updated accordingly, nor was
         something like this assertion added.
     
         Since 5bc3f0b567 (diffcore-pickaxe doc: document -S and -G properly,
         2013-05-31) we've claimed that --pickaxe-regex should only be used
    -    with -S, but have silently toileted combining it with -G, let's die
    +    with -S, but have silently tolerated combining it with -G, let's die
         instead.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
    @@ diff.h: int git_config_rename(const char *var, const char *value);
     
      ## t/t4209-log-pickaxe.sh ##
     @@ t/t4209-log-pickaxe.sh: test_expect_success 'usage' '
    - 	test_expect_code 128 git log -Gregex -Sstring 2>err &&
    - 	test_i18ngrep "mutually exclusive" err &&
    + 	grep "mutually exclusive" err
    + '
      
    ++test_expect_success 'usage: --pickaxe-regex' '
     +	test_expect_code 128 git log -Gregex --pickaxe-regex 2>err &&
    -+	test_i18ngrep "mutually exclusive" err &&
    ++	grep "mutually exclusive" err
    ++'
     +
    - 	test_expect_code 128 git log -Gregex --find-object=HEAD 2>err &&
    - 	test_i18ngrep "mutually exclusive" err &&
    - 
    + test_expect_success 'usage: --no-pickaxe-regex' '
    + 	cat >expect <<-\EOF &&
    + 	fatal: unrecognized argument: --no-pickaxe-regex
10:  72075874f5c !  8:  749c3ca3f98 pickaxe: die when --find-object and --pickaxe-all are combined
    @@ diff.h: int git_config_rename(const char *var, const char *value);
     
      ## t/t4209-log-pickaxe.sh ##
     @@ t/t4209-log-pickaxe.sh: test_expect_success 'usage' '
    - 	test_i18ngrep "mutually exclusive" err &&
    + 	grep "mutually exclusive" err &&
      
    - 	test_expect_code 128 git log -Gstring --find-object=HEAD 2>err &&
    -+	test_i18ngrep "mutually exclusive" err &&
    + 	test_expect_code 128 git log -Sstring --find-object=HEAD 2>err &&
    ++	grep "mutually exclusive" err &&
     +
     +	test_expect_code 128 git log --pickaxe-all --find-object=HEAD 2>err &&
    - 	test_i18ngrep "mutually exclusive" err
    + 	grep "mutually exclusive" err
      '
      
11:  f8116a2b814 =  9:  fe4e75c39d2 diff.h: move pickaxe fields together again
12:  4778357cfc7 = 10:  afe70b163a2 pickaxe/style: consolidate declarations and assignments
13:  7449a59b104 = 11:  97616d741c7 perf: add performance test for pickaxe
14:  160f7d8b0f2 ! 12:  c29deb428b1 pickaxe: refactor function selection in diffcore-pickaxe()
    @@ diffcore-pickaxe.c: void diffcore_pickaxe(struct diff_options *o)
      		regcomp_or_die(&regex, needle, cflags);
      		regexp = &regex;
     +
    -+		/* diff.c errors on -G and --pickaxe-regex for us */
     +		if (opts & DIFF_PICKAXE_KIND_G)
     +			fn = diff_grep;
     +		else if (opts & DIFF_PICKAXE_REGEX)
     +			fn = has_changes;
     +		else
    ++			/*
    ++			 * We don't need to check the combination of
    ++			 * -G and --pickaxe-regex, by the time we get
    ++			 * here diff.c has already died if they're
    ++			 * combined. See the usage tests in
    ++			 * t4209-log-pickaxe.sh.
    ++			 */
     +			BUG("unreachable");
      	} else if (opts & DIFF_PICKAXE_KIND_S) {
      		if (o->pickaxe_opts & DIFF_PICKAXE_IGNORE_CASE &&
15:  05fc57e54e1 ! 13:  115a369d067 pickaxe: assert that we must have a needle under -G or -S
    @@ diffcore-pickaxe.c: void diffcore_pickaxe(struct diff_options *o)
      	kwset_t kws = NULL;
      	pickaxe_fn fn;
      
    -+	if (opts & ~DIFF_PICKAXE_KIND_OBJFIND && !needle)
    ++	if (opts & ~DIFF_PICKAXE_KIND_OBJFIND &&
    ++	    (!needle || !*needle))
     +		BUG("should have needle under -G or -S");
      	if (opts & (DIFF_PICKAXE_REGEX | DIFF_PICKAXE_KIND_G)) {
      		int cflags = REG_EXTENDED | REG_NEWLINE;
    @@ t/t4209-log-pickaxe.sh: test_expect_success setup '
     +	test_i18ngrep "switch.*requires a value" err &&
     +
      	test_expect_code 128 git log -Gregex -Sstring 2>err &&
    - 	test_i18ngrep "mutually exclusive" err &&
    + 	grep "mutually exclusive" err &&
      
16:  550620ec13b ! 14:  a86032792b6 pickaxe -S: support content with NULs under --pickaxe-regex
    @@ Commit message
         for pickaxe matching regular expressions, 2006-03-29).
     
         It wasn't needed anymore, and as the now-passing test shows, actively
    -    getting in our way.
    +    getting in our way. Since we always require REG_STARTEND support we do
    +    not need to stop at NULs. If we are dealing with a haystack with NUL
    +    in it. The needle may be behind that NUL.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
    @@ t/t4209-log-pickaxe.sh: test_expect_success 'log -S looks into binary files' '
     +	git -C GS-bin-txt log --pickaxe-regex -Sa >log &&
     +	test_cmp log full-log &&
     +
    -+	git -C GS-bin-txt log --pickaxe-regex -S[a] >log &&
    ++	git -C GS-bin-txt log --pickaxe-regex -S"[a]" >log &&
     +	test_cmp log full-log
     +'
     +
17:  985e077d561 = 15:  10f85edcff7 pickaxe: rename variables in has_changes() for brevity
18:  648e6e5f11b ! 16:  ed83c3add89 pickaxe -S: slightly optimize contains()
    @@ Commit message
         pre-image and post-image of a change. As soon as we know we had e.g. 1
         before and 2 now we can stop, we don't need to keep counting past 2.
     
    +    With this change a diff between A and B may have different performance
    +    characteristics than between B and A. That's OK in this case, since
    +    we'll emit the same output, and the effect is to make one of them
    +    better.
    +
    +    I'm picking a check of "one" first on the assumption that it's a more
    +    common case to have files grow over time than not.
    +
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## diffcore-pickaxe.c ##
19:  b991660fbf5 ! 17:  62c306275c7 xdiff-interface: allow early return from xdiff_emit_{line,hunk}_fn
    @@ Metadata
     Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Commit message ##
    -    xdiff-interface: allow early return from xdiff_emit_{line,hunk}_fn
    +    xdiff-interface: prepare for allowing early return
     
    -    Change the function prototype of xdiff_emit_{line,hunk}_fn to return
    -    an int instead of void. This will allow for returning early from hunk
    -    & diff consumers that want some of the data, but not all of it.
    +    Change the function prototype of xdiff_emit_line_fn to return an "int"
    +    instead of "void". Change all of those functions to "return 0",
    +    nothing checks those return values yet, and no behavior is being
    +    changed.
     
    -    No behavior is being changed here, just replacing the equivalent of
    -    "return" with "return 0", nothing acts on the changed return values
    -    yet.
    -
    -    There was some work in this area of xdiff-interface.[ch] recently with
    -    3b40a090fd4 (diff: avoid generating unused hunk header lines,
    -    2018-11-02) and 7c61e25fbf1 (diff: use hunk callback for word-diff,
    -    2018-11-02).
    -
    -    In combination those two changes allow us to not do any work on the
    -    hunks and diff at all, but didn't change the status quo with regards
    -    to consumers that e.g. want the diff lines, but might want to abort
    -    early.
    -
    -    Whereas soon we can abort e.g. on the first "-line" of a 1000 line
    -    diff if that's all we needed.
    -
    -    This interface is rather scary as noted in the comment to
    -    xdiff-interface.h being added here, but it will be useful for
    -    diffcore-pickaxe.c in a subsequent commit. A future change could
    -    e.g. add more exit codes, and hack xdl_emit_diff() and friends to
    -    ignore or skip things more selectively as a result.
    +    In subsequent commits the interface will be changed to allow early
    +    return via this new return value.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## combine-diff.c ##
    -@@ combine-diff.c: struct combine_diff_state {
    - 	struct sline *lost_bucket;
    - };
    - 
    --static void consume_hunk(void *state_,
    -+static int consume_hunk(void *state_,
    - 			 long ob, long on,
    - 			 long nb, long nn,
    - 			 const char *funcline, long funclen)
     @@ combine-diff.c: static void consume_hunk(void *state_,
    - 		state->sline[state->nb-1].p_lno =
    - 			xcalloc(state->num_parent, sizeof(unsigned long));
      	state->sline[state->nb-1].p_lno[state->n] = state->ob;
    -+
    -+	return 0;
      }
      
     -static void consume_line(void *state_, char *line, unsigned long len)
    @@ combine-diff.c: static void consume_line(void *state_, char *line, unsigned long
      static void combine_diff(struct repository *r,
     
      ## diff.c ##
    -@@ diff.c: static int color_words_output_graph_prefix(struct diff_words_data *diff_words)
    - 	}
    - }
    - 
    --static void fn_out_diff_words_aux(void *priv,
    --				  long minus_first, long minus_len,
    --				  long plus_first, long plus_len,
    --				  const char *func, long funclen)
    -+static int fn_out_diff_words_aux(void *priv,
    -+				 long minus_first, long minus_len,
    -+				 long plus_first, long plus_len,
    -+				 const char *func, long funclen)
    - {
    - 	struct diff_words_data *diff_words = priv;
    - 	struct diff_words_style *style = diff_words->style;
    -@@ diff.c: static void fn_out_diff_words_aux(void *priv,
    - 
    - 	diff_words->current_plus = plus_end;
    - 	diff_words->last_minus = minus_first;
    -+
    -+	return 0;
    - }
    - 
    - /* This function starts looking at *begin, and returns 0 iff a word was found. */
     @@ diff.c: static void find_lno(const char *line, struct emit_callback *ecbdata)
      	ecbdata->lno_in_postimage = strtol(p + 1, NULL, 10);
      }
    @@ diff.c: static void diffstat_consume(void *priv, char *line, unsigned long len)
      }
      
      const char mime_boundary_leader[] = "------------";
    -@@ diff.c: static int is_conflict_marker(const char *line, int marker_size, unsigned long l
    - 	return 1;
    - }
    - 
    --static void checkdiff_consume_hunk(void *priv,
    -+static int checkdiff_consume_hunk(void *priv,
    - 				   long ob, long on, long nb, long nn,
    - 				   const char *func, long funclen)
    - 
    - {
    - 	struct checkdiff_t *data = priv;
    +@@ diff.c: static void checkdiff_consume_hunk(void *priv,
      	data->lineno = nb - 1;
    -+	return 0;
      }
      
     -static void checkdiff_consume(void *priv, char *line, unsigned long len)
    @@ range-diff.c: static void find_exact_matches(struct string_list *a, struct strin
     +	return 0;
      }
      
    --static void diffsize_hunk(void *data, long ob, long on, long nb, long nn,
    --			  const char *funcline, long funclen)
    -+static int diffsize_hunk(void *data, long ob, long on, long nb, long nn,
    -+			 const char *funcline, long funclen)
    - {
    - 	diffsize_consume(data, NULL, 0);
    -+	return 0;
    - }
    - 
    - static int diffsize(const char *a, const char *b)
    + static void diffsize_hunk(void *data, long ob, long on, long nb, long nn,
     
      ## xdiff-interface.c ##
     @@ xdiff-interface.c: static int xdiff_out_hunk(void *priv_,
    @@ xdiff-interface.c: static void consume_one(void *priv_, char *s, unsigned long s
      }
      
      static int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf)
    -@@ xdiff-interface.c: int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t co
    - 	return xdl_diff(&a, &b, xpp, xecfg, xecb);
    - }
    - 
    --void discard_hunk_line(void *priv,
    --		       long ob, long on, long nb, long nn,
    --		       const char *func, long funclen)
    -+int discard_hunk_line(void *priv,
    -+		      long ob, long on, long nb, long nn,
    -+		      const char *func, long funclen)
    - {
    -+	return 0;
    - }
    - 
    - int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2,
     
      ## xdiff-interface.h ##
     @@
    @@ xdiff-interface.h
      #define MAX_XDIFF_SIZE (1024UL * 1024 * 1023)
      
     -typedef void (*xdiff_emit_line_fn)(void *, char *, unsigned long);
    --typedef void (*xdiff_emit_hunk_fn)(void *data,
    --				   long old_begin, long old_nr,
    --				   long new_begin, long new_nr,
    --				   const char *func, long funclen);
    -+/*
    -+ * The xdiff_emit_{line,hunk}_fn consumers can return -1 to abort
    -+ * early, or 0 to continue processing. Note that doing so is an
    -+ * all-or-nothing affair, as returning -1 will return all the way to
    -+ * the top-level, e.g. the xdi_diff_outf() call to generate the diff.
    -+ *
    -+ * Thus returning -1 from a hunk header callback means you won't be
    -+ * getting any more hunks, or diffs, and likewise returning from a
    -+ * line callback means you won't be getting anymore lines.
    -+ *
    -+ * We may extend the interface in the future to understand other more
    -+ * granular return values, but for now use it carefully, or consider
    -+ * e.g. using discard_hunk_line() if you say just don't care about
    -+ * hunk headers.
    -+ */
     +typedef int (*xdiff_emit_line_fn)(void *, char *, unsigned long);
    -+typedef int (*xdiff_emit_hunk_fn)(void *data,
    -+				  long old_begin, long old_nr,
    -+				  long new_begin, long new_nr,
    -+				  const char *func, long funclen);
    - 
    - int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t const *xecfg, xdemitcb_t *ecb);
    - int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2,
    -@@ xdiff-interface.h: extern int git_xmerge_style;
    -  * Can be used as a no-op hunk_fn for xdi_diff_outf(), since a NULL
    -  * one just sends the hunk line to the line_fn callback).
    -  */
    --void discard_hunk_line(void *priv,
    --		       long ob, long on, long nb, long nn,
    --		       const char *func, long funclen);
    -+int discard_hunk_line(void *priv,
    -+		      long ob, long on, long nb, long nn,
    -+		      const char *func, long funclen);
    - 
    - /*
    -  * Compare the strings l1 with l2 which are of size s1 and s2 respectively.
    + typedef void (*xdiff_emit_hunk_fn)(void *data,
    + 				   long old_begin, long old_nr,
    + 				   long new_begin, long new_nr,
20:  69b061832b3 ! 18:  76d667f152f xdiff-interface: support early exit in xdiff_outf()
    @@ Metadata
     Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Commit message ##
    -    xdiff-interface: support early exit in xdiff_outf()
    +    xdiff-interface: allow early return from xdiff_emit_line_fn
     
    -    Bridge the gap between the preceding "xdiff-interface: allow early
    -    return from xdiff_emit_{line,hunk}_fn" change and the public
    -    interface.
    +    Finish the change started in the preceding commit and allow an early
    +    return from "xdiff_emit_line_fn" callbacks, this will allows
    +    diffcore-pickaxe.c to save itself redundant work.
     
    -    This change was split off from the preceding commit as it wasn't a
    -    purely mechanical addition of "return 0".
    +    Our xdiff interface also had the limitation of not being able to abort
    +    early since the beginning, see d9ea73e0564 (combine-diff: refactor
    +    built-in xdiff interface., 2006-04-05). Although at that time
    +    "xdiff_emit_line_fn" was called "xdiff_emit_consume_fn", and
    +    "xdiff_emit_hunk_fn" didn't exist yet.
     
    -    Here we want to be able to abort early, but do so in a way that
    -    doesn't skip the appropriate strbuf_reset() invocations.
    +    There was some work in this area of xdiff-interface.[ch] recently with
    +    3b40a090fd4 (diff: avoid generating unused hunk header lines,
    +    2018-11-02) and 7c61e25fbf1 (diff: use hunk callback for word-diff,
    +    2018-11-02).
    +
    +    In combination those two changes allow us to not do any work on the
    +    hunks and diff at all, but didn't change the status quo with regards
    +    to consumers that e.g. want the diff lines, but might want to abort
    +    early.
    +
    +    Whereas now we can abort e.g. on the first "-line" of a 1000 line diff
    +    if that's all we needed.
    +
    +    This interface is rather scary as noted in the comment to
    +    xdiff-interface.h being added here, as noted there a future change
    +    could add more exit codes, and hack xdl_emit_diff() and friends to
    +    ignore or skip things more selectively as a result.
    +
    +    I did not see an inherent reason for why xdl_emit_{diffrec,record}()
    +    could not be changed to ferry the "xdiff_emit_line_fn" error code
    +    upwards instead of returning -1 on all "ret < 0".
     
    -    The use of -1 as a return value is consistent with the rest of the
    -    xdiff codebase, where doing so signals an abort or error that'll
    -    propagate up the stack.
    +    But doing so would require corresponding changes in xdl_emit_diff(),
    +    xdl_diff(). I didn't see any issue with narrowly doing that to
    +    accomplish what I needed here, but it would leave xdiff's own return
    +    values in an inconsistent state.
    +
    +    Instead I've left it at returning a more conventional (for git's own
    +    codebase) 1 for an early return, and translating it (or rather, all
    +    non-zero) to -1 for xdiff's consumption.
    +
    +    The reason for most of the "stop" complexity in xdiff_outf() is
    +    because we want to be able to abort early, but do so in a way that
    +    doesn't skip the appropriate strbuf_reset() invocations.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## xdiff-interface.c ##
     @@ xdiff-interface.c: static int consume_one(void *priv_, char *s, unsigned long size)
    + 	char *ep;
    + 	while (size) {
      		unsigned long this_size;
    ++		int ret;
      		ep = memchr(s, '\n', size);
      		this_size = (ep == NULL) ? size : (ep - s + 1);
     -		priv->line_fn(priv->consume_callback_data, s, this_size);
    -+		if (priv->line_fn(priv->consume_callback_data, s, this_size))
    -+			return -1;
    ++		ret = priv->line_fn(priv->consume_callback_data, s, this_size);
    ++		if (ret)
    ++			return ret;
      		size -= this_size;
      		s += this_size;
      	}
    @@ xdiff-interface.c: static int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf)
      
      	for (i = 0; i < nbuf; i++) {
     +		if (stop)
    -+			return -1;
    ++			return 1;
      		if (mb[i].ptr[mb[i].size-1] != '\n') {
      			/* Incomplete line */
      			strbuf_add(&priv->remainder, mb[i].ptr, mb[i].size);
    @@ xdiff-interface.c: static int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf)
      	return 0;
      }
      
    +
    + ## xdiff-interface.h ##
    +@@
    +  */
    + #define MAX_XDIFF_SIZE (1024UL * 1024 * 1023)
    + 
    ++/**
    ++ * The `xdiff_emit_line_fn` function can return 1 to abort early, or 0
    ++ * to continue processing. Note that doing so is an all-or-nothing
    ++ * affair, as returning 1 will return all the way to the top-level,
    ++ * e.g. the xdi_diff_outf() call to generate the diff.
    ++ *
    ++ * Thus returning 1 means you won't be getting any more diff lines. If
    ++ * you need something in-between those two options you'll to use
    ++ * `xdl_emit_hunk_consume_func_t` and implement your own version of
    ++ * xdl_emit_diff().
    ++ *
    ++ * We may extend the interface in the future to understand other more
    ++ * granular return values. While you should return 1 to exit early,
    ++ * doing so will currently make your early return indistinguishable
    ++ * from an error internal to xdiff, xdiff itself will see that
    ++ * non-zero return and translate it to -1.
    ++ */
    + typedef int (*xdiff_emit_line_fn)(void *, char *, unsigned long);
    + typedef void (*xdiff_emit_hunk_fn)(void *data,
    + 				   long old_begin, long old_nr,
21:  fc0aa61d093 ! 19:  53e9405f849 pickaxe -G: terminate early on matching lines
    @@ Commit message
     
         Solve a long-standing item for "git log -Grx" of us e.g. finding "+
         str" in the diff context and noting that we had a "hit", but xdiff
    -    diligently continuing to generate and spew the rest of the diff at us.
    +    diligently continuing to generate and spew the rest of the diff at
    +    us. This makes use of a new "early return" xdiff interface added by
    +    preceding commits.
     
    -    The TODO item has been there since "git log -G" was implemented. See
    -    f506b8e8b5f (git log/diff: add -G<regexp> that greps in the patch
    -    text, 2010-08-23).
    +    The TODO item (or, the NEEDSWORK comment) has been there since "git
    +    log -G" was implemented. See f506b8e8b5f (git log/diff: add -G<regexp>
    +    that greps in the patch text, 2010-08-23).
     
    -    Our xdiff interface also had the limitation of not being able to abort
    -    early since the beginning, see d9ea73e0564 (combine-diff: refactor
    -    built-in xdiff interface., 2006-04-05). Although at that time
    -    "xdiff_emit_line_fn" was called "xdiff_emit_consume_fn", and
    -    "xdiff_emit_hunk_fn" didn't exist yet.
    -
    -    But now with the support added in the preceding ""xdiff-interface:
    -    allow early return from xdiff_emit_{line,hunk}_fn" commit we can
    -    return early, and furthermore test the functionality of the new
    -    early-exit xdiff-interface by having a BUG() call here to die if it
    -    ever starts handing us needless work again.
    +    But now with the support added in the preceding changes to the
    +    xdiff-interface we can return early. Let's assert the behavior of that
    +    new early-return xdiff-interface by having a BUG() call here to die if
    +    it ever starts handing us needless work again.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
    @@ diffcore-pickaxe.c: static int diffgrep_consume(void *priv, char *line, unsigned
     +	if (!regexec_buf(data->regexp, line + 1, len - 1, 1,
     +			 &regmatch, 0)) {
     +		data->hit = 1;
    -+		return -1;
    ++		return 1;
     +	}
      	return 0;
      }
    @@ diffcore-pickaxe.c: static int diff_grep(mmfile_t *one, mmfile_t *two,
     +
     +	/*
     +	 * An xdiff error might be our "data->hit" from above. See the
    -+	 * comment for xdiff_emit_{line,hunk}_fn in xdiff-interface.h
    -+	 * for why.
    ++	 * comment for xdiff_emit_line_fn in xdiff-interface.h
     +	 */
     +	ret = xdi_diff_outf(one, two, discard_hunk_line, diffgrep_consume,
     +			    &ecbdata, &xpp, &xecfg);
    @@ diffcore-pickaxe.c: static int diff_grep(mmfile_t *one, mmfile_t *two,
     
      ## xdiff-interface.h ##
     @@
    -  * granular return values, but for now use it carefully, or consider
    -  * e.g. using discard_hunk_line() if you say just don't care about
    -  * hunk headers.
    +  * doing so will currently make your early return indistinguishable
    +  * from an error internal to xdiff, xdiff itself will see that
    +  * non-zero return and translate it to -1.
     + *
    -+ * Note that just returning -1 will make your early return
    -+ * indistinguishable from an error internal to xdiff. See "diff_grep"
    -+ * in diffcore-pickaxe.c for a trick to work around this, i.e. using
    -+ * the "consume_callback_data" to note the desired early return.
    ++ * See "diff_grep" in diffcore-pickaxe.c for a trick to work around
    ++ * this, i.e. using the "consume_callback_data" to note the desired
    ++ * early return.
       */
      typedef int (*xdiff_emit_line_fn)(void *, char *, unsigned long);
    - typedef int (*xdiff_emit_hunk_fn)(void *data,
    + typedef void (*xdiff_emit_hunk_fn)(void *data,
22:  c81b18ca4c7 = 20:  76de6ebc8b8 pickaxe -G: don't special-case create/delete
 -:  ----------- > 21:  9bb7ac910f3 xdiff users: use designated initializers for out_line
 -:  ----------- > 22:  1178956fb3d xdiff-interface: replace discard_hunk_line() with a flag
-- 
2.31.1.639.g3d04783866f