[PATCH v3 0/3] Output fixes for --remerge-diff

["Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>]

Philippe Blain found and reported a couple issues with the output of
--remerge-diff[1]. After digging in, I think one of them actually counts as
two separate issues, so here's a series with three patches to fix these
issues. Each includes testcases to keep us from regressing.

Changes since v2:

 * Added a comment describing the rationale for the new
   diff_filepair_is_phoney() function.

Changes since v1:

 * Added a new diff_filepair_is_phoney() function to make the code more
   self-documenting, as suggested by Junio.
 * Note: Patch 2, as called out in its commit message, disables
   TEST_PASSES_SANITIZE_LEAK for t4069. This is not because I introduce any
   memory leaks, but because I add new testcases invoking additional parts
   of the code (pickaxe stuff) which already had pre-existing leaks. This is
   not a change since v1, but this somehow accidentally got munged out of
   Junio's application of my v1.

Note: The issue fixed by the third commit for --remerge-diff is also an
issue exhibited by 'git log --cc $FILTER_RULES $COMMIT' (or by -c instead of
--cc). However, as far as I can tell the causes are different and come from
separate codepaths; this series focuses on --remerge-diff and hence makes no
attempt to fix independent (even if similar) --cc or -c issues.

[1]
https://lore.kernel.org/git/43cf2a1d-058a-fd79-befe-7d9bc62581ed@gmail.com/

Elijah Newren (3):
  diff: have submodule_format logic avoid additional diff headers
  diff: fix filtering of additional headers under --remerge-diff
  diff: fix filtering of merge commits under --remerge-diff

 diff.c                  | 32 ++++++++++++++++++++++++++------
 t/t4069-remerge-diff.sh | 30 +++++++++++++++++++++++++++++-
 2 files changed, 55 insertions(+), 7 deletions(-)


base-commit: 6c8e4ee870332d11e4bba84901654b355a9ff016
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1342%2Fnewren%2Fremerge-diff-output-fixes-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1342/newren/remerge-diff-output-fixes-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1342

Range-diff vs v2:

 1:  aea0bbc1c6a ! 1:  ed28acaed97 diff: have submodule_format logic avoid additional diff headers
     @@ Commit message
      
          The submodule_format handling is another codepath with the same issue;
          it would operate on these additional headers and attempt to display them
     -    as submodule changes.  Prevent that by explicitly checking for both
     -    modes being 0.
     +    as submodule changes.  Prevent that by explicitly checking for "phoney"
     +    filepairs (i.e. filepairs with both modes being 0).
      
          Reported-by: Philippe Blain <levraiphilippeblain@gmail.com>
          Signed-off-by: Elijah Newren <newren@gmail.com>
     @@ diff.c: static void add_formatted_headers(struct strbuf *msg,
      +static int diff_filepair_is_phoney(struct diff_filespec *one,
      +				   struct diff_filespec *two)
      +{
     ++	/*
     ++	 * This function specifically looks for pairs injected by
     ++	 * create_filepairs_for_header_only_notifications().  Such
     ++	 * pairs are "phoney" in that they do not represent any
     ++	 * content or even mode difference, but were inserted because
     ++	 * diff_queued_diff previously had no pair associated with
     ++	 * that path but we needed some pair to avoid losing the
     ++	 * "remerge CONFLICT" header associated with the path.
     ++	 */
      +	return !DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two);
      +}
      +
     @@ diff.c: static void builtin_diff(const char *name_a,
      -	if (!DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two)) {
      +	if (diff_filepair_is_phoney(one, two)) {
       		/*
     - 		 * We should only reach this point for pairs from
     +-		 * We should only reach this point for pairs from
     ++		 * We should only reach this point for pairs generated from
       		 * create_filepairs_for_header_only_notifications().  For
     +-		 * these, we should avoid the "/dev/null" special casing
     +-		 * above, meaning we avoid showing such pairs as either
     ++		 * these, we want to avoid the "/dev/null" special casing
     ++		 * above, because we do not want such pairs shown as either
     + 		 * "new file" or "deleted file" below.
     + 		 */
     + 		lbl[0] = a_one;
      
       ## t/t4069-remerge-diff.sh ##
      @@ t/t4069-remerge-diff.sh: test_expect_success 'remerge-diff w/ diff-filter=U: all conflict headers, no dif
 2:  3bd622d5b45 = 2:  f91bea2bbc3 diff: fix filtering of additional headers under --remerge-diff
 3:  7903f8380dc = 3:  084a037756d diff: fix filtering of merge commits under --remerge-diff

-- 
gitgitgadget