diff: have submodule_format logic avoid additional diff headers

Commit 95433eeed9 ("diff: add ability to insert additional headers for paths", 2022-02-02) introduced the possibility of additional headers, created in create_filepairs_for_header_only_notifications(). These are represented by inserting additional pairs in diff_queued_diff which always have a mode of 0 and a null_oid. When these were added, one code path was noted to assume that at least one of the diff_filespecs in the pair were valid, and that codepath was corrected. 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 "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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Sep 2, 2022 at 03:53 UTC 9b08091cb72bdd1d8b37446bcf0a6d73d319bf10
2 files changed +31 -6
diff.c
+23 -6
@@ -3398,6 +3398,21 @@ static void add_formatted_headers(struct strbuf *msg,
3398 line_prefix, meta, reset);
3399 }
3400
3401 +static int diff_filepair_is_phoney(struct diff_filespec *one,
3402 + struct diff_filespec *two)
3403 +{
3404 + /*
3405 + * This function specifically looks for pairs injected by
3406 + * create_filepairs_for_header_only_notifications(). Such
3407 + * pairs are "phoney" in that they do not represent any
3408 + * content or even mode difference, but were inserted because
3409 + * diff_queued_diff previously had no pair associated with
3410 + * that path but we needed some pair to avoid losing the
3411 + * "remerge CONFLICT" header associated with the path.
3412 + */
3413 + return !DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two);
3414 +}
3415 +
3416 static void builtin_diff(const char *name_a,
3417 const char *name_b,
3418 struct diff_filespec *one,
@@ -3429,14 +3444,16 @@ static void builtin_diff(const char *name_a,
3444
3445 if (o->submodule_format == DIFF_SUBMODULE_LOG &&
3446 (!one->mode || S_ISGITLINK(one->mode)) &&
3432 - (!two->mode || S_ISGITLINK(two->mode))) {
3447 + (!two->mode || S_ISGITLINK(two->mode)) &&
3448 + (!diff_filepair_is_phoney(one, two))) {
3449 show_submodule_diff_summary(o, one->path ? one->path : two->path,
3450 &one->oid, &two->oid,
3451 two->dirty_submodule);
3452 return;
3453 } else if (o->submodule_format == DIFF_SUBMODULE_INLINE_DIFF &&
3454 (!one->mode || S_ISGITLINK(one->mode)) &&
3439 - (!two->mode || S_ISGITLINK(two->mode))) {
3455 + (!two->mode || S_ISGITLINK(two->mode)) &&
3456 + (!diff_filepair_is_phoney(one, two))) {
3457 show_submodule_inline_diff(o, one->path ? one->path : two->path,
3458 &one->oid, &two->oid,
3459 two->dirty_submodule);
@@ -3456,12 +3473,12 @@ static void builtin_diff(const char *name_a,
3473 b_two = quote_two(b_prefix, name_b + (*name_b == '/'));
3474 lbl[0] = DIFF_FILE_VALID(one) ? a_one : "/dev/null";
3475 lbl[1] = DIFF_FILE_VALID(two) ? b_two : "/dev/null";
3459 - if (!DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two)) {
3476 + if (diff_filepair_is_phoney(one, two)) {
3477 /*
3461 - * We should only reach this point for pairs from
3478 + * We should only reach this point for pairs generated from
3479 * create_filepairs_for_header_only_notifications(). For
3463 - * these, we should avoid the "/dev/null" special casing
3464 - * above, meaning we avoid showing such pairs as either
3480 + * these, we want to avoid the "/dev/null" special casing
3481 + * above, because we do not want such pairs shown as either
3482 * "new file" or "deleted file" below.
3483 */
3484 lbl[0] = a_one;
t/t4069-remerge-diff.sh
+8
@@ -185,6 +185,14 @@ test_expect_success 'remerge-diff w/ diff-filter=U: all conflict headers, no dif
185 test_cmp expect actual
186 '
187
188 +test_expect_success 'submodule formatting ignores additional headers' '
189 + # Reuses "expect" from last testcase
190 +
191 + git show --oneline --remerge-diff --diff-filter=U --submodule=log >tmp &&
192 + sed -e "s/[0-9a-f]\{7,\}/HASH/g" tmp >actual &&
193 + test_cmp expect actual
194 +'
195 +
196 test_expect_success 'remerge-diff w/ diff-filter=R: relevant file + conflict header' '
197 git log -1 --oneline resolution >tmp &&
198 cat <<-EOF >>tmp &&