git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 3/3] diff: support ^! for merges

From
René Scharfe <l.s.r@web.de>
Date
Oct 1, 2022, 10:28 UTC
Message-ID
<0cf042fa-aa6f-4e3e-5982-3354c1cab677@web.de>
In-Reply-To
<16c49d20-cafc-4b48-3c6b-e11c74c29abb@web.de>

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>
---
 Documentation/git-diff.txt |  8 ++++----
 builtin/diff.c             | 23 ++++++++++++++++++-----
 t/t4038-diff-combined.sh   | 10 ++++++++++
 3 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
index 85ae6d6d08..52b679256c 100644
--- a/Documentation/git-diff.txt
+++ b/Documentation/git-diff.txt
@@ -79,10 +79,10 @@ If --merge-base is given, use the merge base of the two commits for the

 	This form is to view the results of a merge commit.  The first
 	listed <commit> must be the merge itself; the remaining two or
-	more commits should be its parents.  A convenient way to produce
-	the desired set of revisions is to use the `^@` suffix.
-	For instance, if `master` names a merge commit, `git diff master
-	master^@` gives the same combined diff as `git show master`.
+	more commits should be its parents.  Convenient ways to produce
+	the desired set of revisions are to use the suffixes `^@` and
+	`^!`.  If A is a merge commit, then `git diff A A^@`,
+	`git diff A^!` and `git show A` all give the same combined diff.

 'git diff' [<options>] <commit>..<commit> [--] [<path>...]::

diff --git a/builtin/diff.c b/builtin/diff.c
index 54bb3de964..0e49919735 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -209,7 +209,7 @@ static int builtin_diff_tree(struct rev_info *revs,
 static int builtin_diff_combined(struct rev_info *revs,
 				 int argc, const char **argv,
 				 struct object_array_entry *ent,
-				 int ents)
+				 int ents, int first_non_parent)
 {
 	struct oid_array parents = OID_ARRAY_INIT;
 	int i;
@@ -217,11 +217,18 @@ static int builtin_diff_combined(struct rev_info *revs,
 	if (argc > 1)
 		usage(builtin_diff_usage);

+	if (first_non_parent < 0)
+		die(_("no merge given, only parents."));
+	if (first_non_parent >= ents)
+		BUG("first_non_parent out of range: %d", first_non_parent);
+
 	diff_merges_set_dense_combined_if_unset(revs);

-	for (i = 1; i < ents; i++)
-		oid_array_append(&parents, &ent[i].item->oid);
-	diff_tree_combined(&ent[0].item->oid, &parents, revs);
+	for (i = 0; i < ents; i++) {
+		if (i != first_non_parent)
+			oid_array_append(&parents, &ent[i].item->oid);
+	}
+	diff_tree_combined(&ent[first_non_parent].item->oid, &parents, revs);
 	oid_array_clear(&parents);
 	return 0;
 }
@@ -385,6 +392,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	int i;
 	struct rev_info rev;
 	struct object_array ent = OBJECT_ARRAY_INIT;
+	int first_non_parent = -1;
 	int blobs = 0, paths = 0;
 	struct object_array_entry *blob[2];
 	int nongit = 0, no_index = 0;
@@ -543,6 +551,10 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 				continue;
 			obj->flags |= flags;
 			add_object_array(obj, name, &ent);
+			if (first_non_parent < 0 &&
+			    (i >= rev.cmdline.nr || /* HEAD by hand. */
+			     rev.cmdline.rev[i].whence != REV_CMD_PARENTS_ONLY))
+				first_non_parent = ent.nr - 1;
 		} else if (obj->type == OBJ_BLOB) {
 			if (2 <= blobs)
 				die(_("more than two blobs given: '%s'"), name);
@@ -590,7 +602,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 					   &ent.objects[0], &ent.objects[1]);
 	} else
 		result = builtin_diff_combined(&rev, argc, argv,
-					       ent.objects, ent.nr);
+					       ent.objects, ent.nr,
+					       first_non_parent);
 	result = diff_result_code(&rev.diffopt, result);
 	if (1 < rev.diffopt.skip_stat_unmatch)
 		refresh_index_quietly();
diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh
index 9a292bac70..2ce26e585c 100755
--- a/t/t4038-diff-combined.sh
+++ b/t/t4038-diff-combined.sh
@@ -80,11 +80,21 @@ test_expect_success 'check combined output (1)' '
 	verify_helper sidewithone
 '

+test_expect_success 'check combined output (1) with git diff <rev>^!' '
+	git diff sidewithone^! -- >sidewithone &&
+	verify_helper sidewithone
+'
+
 test_expect_success 'check combined output (2)' '
 	git show sidesansone -- >sidesansone &&
 	verify_helper sidesansone
 '

+test_expect_success 'check combined output (2) with git diff <rev>^!' '
+	git diff sidesansone^! -- >sidesansone &&
+	verify_helper sidesansone
+'
+
 test_expect_success 'diagnose truncated file' '
 	>file &&
 	git add file &&
--
2.37.3
Previous: René Scharfe
Message 16 of 16 in “revision: fix order of revs for ^!”
  1. 0/6 revision: fix order of revs for ^!René Scharfe, Sep 15, 2022
  2. 1/6 revision: use strtol_i() for exclude_parentRené Scharfe, Sep 15, 2022
  3. Chris TorekSep 17, 2022
  4. 2/6 revision: factor out get_commit()René Scharfe, Sep 15, 2022
  5. 3/6 revision: factor out add_parent()René Scharfe, Sep 15, 2022
  6. 4/6 revision: factor out add_parents()René Scharfe, Sep 15, 2022
  7. 5/6 revision: rename add_parents_only() to add_nth_parent()René Scharfe, Sep 15, 2022
  8. 6/6 revision: add parents after child for ^!René Scharfe, Sep 15, 2022
  9. Junio C HamanoSep 15, 2022
  10. René ScharfeSep 16, 2022
  11. Junio C HamanoSep 16, 2022
  12. René ScharfeSep 18, 2022
  13. 0/3 diff: support ^! for mergesRené Scharfe, Oct 1, 2022
  14. 1/3 revision: use strtol_i() for exclude_parentRené Scharfe, Oct 1, 2022
  15. 2/3 revisions.txt: unspecify order of resolved parts of ^!René Scharfe, Oct 1, 2022
  16. 3/3 diff: support ^! for mergesRené Scharfe, Oct 1, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.