diff: support ^! for merges

revision.c::handle_revision_arg_1() resolves <rev>^! by first adding the negated parents and then <rev> itself. builtin_diff_combined() expects the first tree to be the merge and the remaining ones to be the parents, though. This mismatch results in bogus diff output. Remember the first tree that doesn't belong to a parent and use it instead of blindly picking the first one. This makes "git diff <rev>^!" consistent with "git show <rev>^!". Reported-by: Tim Jaacks <tim.jaacks@garz-fricke.com> Suggested-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

René Scharfe committed Oct 1, 2022 at 12:28 UTC a79c6b60817c74534815bf132f0b26aa8e325874
3 files changed +32 -9
Documentation/git-diff.txt
+4 -4
@@ -79,10 +79,10 @@ If --merge-base is given, use the merge base of the two commits for the
79
80 This form is to view the results of a merge commit. The first
81 listed <commit> must be the merge itself; the remaining two or
82 - more commits should be its parents. A convenient way to produce
83 - the desired set of revisions is to use the `^@` suffix.
84 - For instance, if `master` names a merge commit, `git diff master
85 - master^@` gives the same combined diff as `git show master`.
82 + more commits should be its parents. Convenient ways to produce
83 + the desired set of revisions are to use the suffixes `^@` and
84 + `^!`. If A is a merge commit, then `git diff A A^@`,
85 + `git diff A^!` and `git show A` all give the same combined diff.
86
87 'git diff' [<options>] <commit>..<commit> [--] [<path>...]::
88
builtin/diff.c
+18 -5
@@ -209,7 +209,7 @@ static int builtin_diff_tree(struct rev_info *revs,
209 static int builtin_diff_combined(struct rev_info *revs,
210 int argc, const char **argv,
211 struct object_array_entry *ent,
212 - int ents)
212 + int ents, int first_non_parent)
213 {
214 struct oid_array parents = OID_ARRAY_INIT;
215 int i;
@@ -217,11 +217,18 @@ static int builtin_diff_combined(struct rev_info *revs,
217 if (argc > 1)
218 usage(builtin_diff_usage);
219
220 + if (first_non_parent < 0)
221 + die(_("no merge given, only parents."));
222 + if (first_non_parent >= ents)
223 + BUG("first_non_parent out of range: %d", first_non_parent);
224 +
225 diff_merges_set_dense_combined_if_unset(revs);
226
222 - for (i = 1; i < ents; i++)
223 - oid_array_append(&parents, &ent[i].item->oid);
224 - diff_tree_combined(&ent[0].item->oid, &parents, revs);
227 + for (i = 0; i < ents; i++) {
228 + if (i != first_non_parent)
229 + oid_array_append(&parents, &ent[i].item->oid);
230 + }
231 + diff_tree_combined(&ent[first_non_parent].item->oid, &parents, revs);
232 oid_array_clear(&parents);
233 return 0;
234 }
@@ -385,6 +392,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
392 int i;
393 struct rev_info rev;
394 struct object_array ent = OBJECT_ARRAY_INIT;
395 + int first_non_parent = -1;
396 int blobs = 0, paths = 0;
397 struct object_array_entry *blob[2];
398 int nongit = 0, no_index = 0;
@@ -543,6 +551,10 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
551 continue;
552 obj->flags |= flags;
553 add_object_array(obj, name, &ent);
554 + if (first_non_parent < 0 &&
555 + (i >= rev.cmdline.nr || /* HEAD by hand. */
556 + rev.cmdline.rev[i].whence != REV_CMD_PARENTS_ONLY))
557 + first_non_parent = ent.nr - 1;
558 } else if (obj->type == OBJ_BLOB) {
559 if (2 <= blobs)
560 die(_("more than two blobs given: '%s'"), name);
@@ -590,7 +602,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
602 &ent.objects[0], &ent.objects[1]);
603 } else
604 result = builtin_diff_combined(&rev, argc, argv,
593 - ent.objects, ent.nr);
605 + ent.objects, ent.nr,
606 + first_non_parent);
607 result = diff_result_code(&rev.diffopt, result);
608 if (1 < rev.diffopt.skip_stat_unmatch)
609 refresh_index_quietly();
t/t4038-diff-combined.sh
+10
@@ -80,11 +80,21 @@ test_expect_success 'check combined output (1)' '
80 verify_helper sidewithone
81 '
82
83 +test_expect_success 'check combined output (1) with git diff <rev>^!' '
84 + git diff sidewithone^! -- >sidewithone &&
85 + verify_helper sidewithone
86 +'
87 +
88 test_expect_success 'check combined output (2)' '
89 git show sidesansone -- >sidesansone &&
90 verify_helper sidesansone
91 '
92
93 +test_expect_success 'check combined output (2) with git diff <rev>^!' '
94 + git diff sidesansone^! -- >sidesansone &&
95 + verify_helper sidesansone
96 +'
97 +
98 test_expect_success 'diagnose truncated file' '
99 >file &&
100 git add file &&