git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Subject: [PATCH 4/4] builtin/merge.c: reduce parents early
Date: Tue, 17 Apr 2012 13:34:46 -0700	[thread overview]
Message-ID: <1334694886-27756-5-git-send-email-gitster@pobox.com> (raw)
In-Reply-To: <1334694886-27756-1-git-send-email-gitster@pobox.com>

Instead of waiting until we record the parents of resulting merge, reduce
redundant parents (including our HEAD) immediately after reading them.

The change to t7602 illustrates the essence of the effect of this change.
The octopus merge strategy used to be fed with redundant commits only to
discard them as "up-to-date", but we no longer feed such redundant commits
to it and the affected test degenerates to a regular two-head merge.

And obviously the known-to-be-broken test in t7602 is now fixed.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c               |   65 +++++++++++++++++++++++++----------------
 t/t6028-merge-up-to-date.sh   |    2 +-
 t/t7602-merge-octopus-many.sh |   10 +++----
 3 files changed, 45 insertions(+), 32 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 2cef2f6..20aeca0 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -938,31 +938,22 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
 }
 
 static int finish_automerge(struct commit *head,
+			    int head_subsumed,
 			    struct commit_list *common,
 			    struct commit_list *remoteheads,
 			    unsigned char *result_tree,
 			    const char *wt_strategy)
 {
-	struct commit_list *parents = NULL, *j;
+	struct commit_list *parents = NULL;
 	struct strbuf buf = STRBUF_INIT;
 	unsigned char result_commit[20];
 
 	free_commit_list(common);
-	if (allow_fast_forward) {
-		parents = remoteheads;
+	parents = remoteheads;
+	if (!head_subsumed || !allow_fast_forward)
 		commit_list_insert(head, &parents);
-		parents = reduce_heads(parents);
-	} else {
-		struct commit_list **pptr = &parents;
-
-		pptr = &commit_list_insert(head,
-				pptr)->next;
-		for (j = remoteheads; j; j = j->next)
-			pptr = &commit_list_insert(j->item, pptr)->next;
-	}
 	strbuf_addch(&merge_msg, '\n');
 	prepare_to_commit(remoteheads);
-	free_commit_list(remoteheads);
 	if (commit_tree(&merge_msg, result_tree, parents, result_commit,
 			NULL, sign_commit))
 		die(_("failed to write commit object"));
@@ -1137,12 +1128,16 @@ static int default_edit_option(void)
 		st_stdin.st_mode == st_stdout.st_mode);
 }
 
-static struct commit_list *collect_parents(int argc, const char **argv)
+static struct commit_list *collect_parents(struct commit *head_commit,
+					   int *head_subsumed,
+					   int argc, const char **argv)
 {
 	int i;
-	struct commit_list *remoteheads = NULL;
+	struct commit_list *remoteheads = NULL, *parents, *next;
 	struct commit_list **remotes = &remoteheads;
 
+	if (head_commit)
+		remotes = &commit_list_insert(head_commit, remotes)->next;
 	for (i = 0; i < argc; i++) {
 		struct commit *commit = get_merge_parent(argv[i]);
 		if (!commit)
@@ -1150,6 +1145,20 @@ static struct commit_list *collect_parents(int argc, const char **argv)
 		remotes = &commit_list_insert(commit, remotes)->next;
 	}
 	*remotes = NULL;
+
+	parents = reduce_heads(remoteheads);
+
+	*head_subsumed = 1; /* we will flip this to 0 when we find it */
+	for (remoteheads = NULL, remotes = &remoteheads;
+	     parents;
+	     parents = next) {
+		struct commit *commit = parents->item;
+		next = parents->next;
+		if (commit == head_commit)
+			*head_subsumed = 0;
+		else
+			remotes = &commit_list_insert(commit, remotes)->next;
+	}
 	return remoteheads;
 }
 
@@ -1161,7 +1170,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	struct commit *head_commit;
 	struct strbuf buf = STRBUF_INIT;
 	const char *head_arg;
-	int flag, i, ret = 0;
+	int flag, i, ret = 0, head_subsumed;
 	int best_cnt = -1, merge_was_ok = 0, automerge_was_ok = 0;
 	struct commit_list *common = NULL;
 	const char *best_strategy = NULL, *wt_strategy = NULL;
@@ -1270,7 +1279,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		head_arg = argv[1];
 		argv += 2;
 		argc -= 2;
-		remoteheads = collect_parents(argc, argv);
+		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
 	} else if (!head_commit) {
 		struct commit *remote_head;
 		/*
@@ -1286,7 +1295,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		if (!allow_fast_forward)
 			die(_("Non-fast-forward commit does not make sense into "
 			    "an empty head"));
-		remoteheads = collect_parents(argc, argv);
+		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
 		remote_head = remoteheads->item;
 		if (!remote_head)
 			die(_("%s - not something we can merge"), argv[0]);
@@ -1305,7 +1314,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		 * the standard merge summary message to be appended
 		 * to the given message.
 		 */
-		remoteheads = collect_parents(argc, argv);
+		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
 		for (p = remoteheads; p; p = p->next)
 			merge_name(merge_remote_util(p->item)->name, &merge_names);
 
@@ -1351,7 +1360,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		option_edit = 0;
 
 	if (!use_strategies) {
-		if (!remoteheads->next)
+		if (!remoteheads)
+			; /* already up-to-date */
+		else if (!remoteheads->next)
 			add_strategies(pull_twohead, DEFAULT_TWOHEAD);
 		else
 			add_strategies(pull_octopus, DEFAULT_OCTOPUS);
@@ -1364,7 +1375,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			allow_trivial = 0;
 	}
 
-	if (!remoteheads->next)
+	if (!remoteheads)
+		; /* already up-to-date */
+	else if (!remoteheads->next)
 		common = get_merge_bases(head_commit, remoteheads->item, 1);
 	else {
 		struct commit_list *list = remoteheads;
@@ -1376,10 +1389,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	update_ref("updating ORIG_HEAD", "ORIG_HEAD", head_commit->object.sha1,
 		   NULL, 0, DIE_ON_ERR);
 
-	if (!common)
+	if (remoteheads && !common)
 		; /* No common ancestors found. We need a real merge. */
-	else if (!remoteheads->next && !common->next &&
-			common->item == remoteheads->item) {
+	else if (!remoteheads ||
+		 (!remoteheads->next && !common->next &&
+		  common->item == remoteheads->item)) {
 		/*
 		 * If head can reach all the merge then we are up to date.
 		 * but first the most common case of merging one remote.
@@ -1553,7 +1567,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	 * auto resolved the merge cleanly.
 	 */
 	if (automerge_was_ok) {
-		ret = finish_automerge(head_commit, common, remoteheads,
+		ret = finish_automerge(head_commit, head_subsumed,
+				       common, remoteheads,
 				       result_tree, wt_strategy);
 		goto done;
 	}
diff --git a/t/t6028-merge-up-to-date.sh b/t/t6028-merge-up-to-date.sh
index 824fca5..c518e9c 100755
--- a/t/t6028-merge-up-to-date.sh
+++ b/t/t6028-merge-up-to-date.sh
@@ -79,7 +79,7 @@ test_expect_success 'merge -s subtree up-to-date' '
 
 '
 
-test_expect_failure 'merge fast-forward octopus' '
+test_expect_success 'merge fast-forward octopus' '
 
 	git reset --hard c0 &&
 	test_tick &&
diff --git a/t/t7602-merge-octopus-many.sh b/t/t7602-merge-octopus-many.sh
index 5783ebf..7117b57 100755
--- a/t/t7602-merge-octopus-many.sh
+++ b/t/t7602-merge-octopus-many.sh
@@ -70,17 +70,15 @@ test_expect_success 'merge output uses pretty names' '
 '
 
 cat >expected <<\EOF
-Already up-to-date with c4
-Trying simple merge with c5
-Merge made by the 'octopus' strategy.
+Merge made by the 'recursive' strategy.
  c5.c |    1 +
  1 file changed, 1 insertion(+)
  create mode 100644 c5.c
 EOF
 
-test_expect_success 'merge up-to-date output uses pretty names' '
-	git merge c4 c5 >actual &&
-	test_cmp actual expected
+test_expect_success 'merge reduces irrelevant remote heads' '
+	GIT_MERGE_VERBOSITY=0 git merge c4 c5 >actual &&
+	test_cmp expected actual
 '
 
 cat >expected <<\EOF
-- 
1.7.10.332.g1863c

  parent reply	other threads:[~2012-04-17 20:35 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-16  6:26 What's cooking in git.git (Apr 2012, #05; Thu, 12) Michal Kiedrowicz
2012-04-16 14:57 ` Linus Torvalds
2012-04-16 17:29   ` Junio C Hamano
2012-04-16 17:50     ` Linus Torvalds
2012-04-16 22:03       ` Junio C Hamano
2012-04-17 20:34       ` [PATCH 0/4] merge: reduce set of parents consistently Junio C Hamano
2012-04-17 20:34         ` [PATCH 1/4] git-merge: test octopus with redundant parents Junio C Hamano
2012-04-17 20:34         ` [PATCH 2/4] builtin/merge.c: remove "remoteheads" global variable Junio C Hamano
2012-04-17 20:34         ` [PATCH 3/4] builtin/merge.c: collect other parents early Junio C Hamano
2012-04-17 20:34         ` Junio C Hamano [this message]
2012-04-16 17:36   ` What's cooking in git.git (Apr 2012, #05; Thu, 12) Junio C Hamano
2012-04-16 18:02     ` Linus Torvalds
2012-04-16 18:33       ` Linus Torvalds
2012-04-16 21:32         ` Michał Kiedrowicz
2012-04-17  1:22           ` Linus Torvalds
2012-04-17 18:25             ` [PATCH] git-merge: Reduce heads before trying to merge them Michał Kiedrowicz
2012-04-17 18:52               ` Junio C Hamano
2012-04-17 20:09                 ` Linus Torvalds
2012-04-17 20:48                   ` Junio C Hamano
2012-04-18 18:14                     ` Michał Kiedrowicz
2012-04-18 20:20                       ` Junio C Hamano
2012-04-19  5:19                         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1334694886-27756-5-git-send-email-gitster@pobox.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).