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@xxxxxxxxx/ 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@xxxxxxxxx> Signed-off-by: Elijah Newren <newren@xxxxxxxxx> @@ 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
