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

[PATCH] builtin-merge: avoid run_command_v_opt() for recursive and subtree

From
Miklos Vajna <vmiklos@frugalware.org>
Date
Aug 28, 2008, 13:43 UTC
Message-ID
<1219930980-29114-1-git-send-email-vmiklos@frugalware.org>
In-Reply-To
<7vtzdbuqb6.fsf@gitster.siamese.dyndns.org>

The try_merge_strategy() function always ran the strategy in a separate process, though this is not always necessary. The recursive and subtree strategy can be called without a fork(). This patch adds a check, and calls recursive in the same process without wasting resources.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Sat, Aug 23, 2008 at 06:58:37PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
Show 13 quoted lines
> By the way, in the medium term, if we are serious about making an
> internal call to merge_recursive() from cmd_merge(), I think we may be
> better off making it the responsibility for try_merge_strategy() to
> leave an committable state in the in-core index (aka "the_index") when
> they return with 0 (success) status.  After calling external ones via
> the run_command interface, it should do a read_cache() (after calling
> discard_cache() if needed); if it calls merge_recursive(), hopefully
> you already have the committable state in the in-core index.
>
> That way, when automerge succeeds, write_tree_trivial() can write that
> in-core index out and create the tree object to be committed. The
> callchain to use merge_recursive() can avoid having to write to the
> on-disk index, read it again and write out the tree from it.

This is mostly the same patch I submitted earlier, but this is now on top of mv/merge-recursive, and now it does what you suggested above.

 builtin-merge.c |   92 +++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 63 insertions(+), 29 deletions(-)
diff --git a/builtin-merge.c b/builtin-merge.c
index 0bff26e..3a38089 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -22,6 +22,7 @@
 #include "log-tree.h"
 #include "color.h"
 #include "rerere.h"
+#include "merge-recursive.h"
 
 #define DEFAULT_TWOHEAD (1<<0)
 #define DEFAULT_OCTOPUS (1<<1)
@@ -511,28 +512,64 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 	struct commit_list *j;
 	struct strbuf buf;
 
-	args = xmalloc((4 + commit_list_count(common) +
-			commit_list_count(remoteheads)) * sizeof(char *));
-	strbuf_init(&buf, 0);
-	strbuf_addf(&buf, "merge-%s", strategy);
-	args[i++] = buf.buf;
-	for (j = common; j; j = j->next)
-		args[i++] = xstrdup(sha1_to_hex(j->item->object.sha1));
-	args[i++] = "--";
-	args[i++] = head_arg;
-	for (j = remoteheads; j; j = j->next)
-		args[i++] = xstrdup(sha1_to_hex(j->item->object.sha1));
-	args[i] = NULL;
-	ret = run_command_v_opt(args, RUN_GIT_CMD);
-	strbuf_release(&buf);
-	i = 1;
-	for (j = common; j; j = j->next)
-		free((void *)args[i++]);
-	i += 2;
-	for (j = remoteheads; j; j = j->next)
-		free((void *)args[i++]);
-	free(args);
-	return -ret;
+	if (!strcmp(strategy, "recursive") || !strcmp(strategy, "subtree")) {
+		int clean;
+		struct commit *result;
+		struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
+		int index_fd;
+		struct commit_list *reversed = NULL;
+		struct merge_options o;
+
+		if (remoteheads->next) {
+			error("Not handling anything other than two heads merge.");
+			return 2;
+		}
+
+		init_merge_options(&o);
+		if (!strcmp(strategy, "subtree"))
+			o.subtree_merge = 1;
+
+		o.branch1 = head_arg;
+		o.branch2 = remoteheads->item->util;
+
+		for (j = common; j; j = j->next)
+			commit_list_insert(j->item, &reversed);
+
+		index_fd = hold_locked_index(lock, 1);
+		clean = merge_recursive(&o, lookup_commit(head),
+				remoteheads->item, reversed, &result);
+		if (active_cache_changed &&
+				(write_cache(index_fd, active_cache, active_nr) ||
+				 commit_locked_index(lock)))
+			die ("unable to write %s", get_index_file());
+		return clean ? 0 : 1;
+	} else {
+		args = xmalloc((4 + commit_list_count(common) +
+					commit_list_count(remoteheads)) * sizeof(char *));
+		strbuf_init(&buf, 0);
+		strbuf_addf(&buf, "merge-%s", strategy);
+		args[i++] = buf.buf;
+		for (j = common; j; j = j->next)
+			args[i++] = xstrdup(sha1_to_hex(j->item->object.sha1));
+		args[i++] = "--";
+		args[i++] = head_arg;
+		for (j = remoteheads; j; j = j->next)
+			args[i++] = xstrdup(sha1_to_hex(j->item->object.sha1));
+		args[i] = NULL;
+		ret = run_command_v_opt(args, RUN_GIT_CMD);
+		strbuf_release(&buf);
+		i = 1;
+		for (j = common; j; j = j->next)
+			free((void *)args[i++]);
+		i += 2;
+		for (j = remoteheads; j; j = j->next)
+			free((void *)args[i++]);
+		free(args);
+		discard_cache();
+		if (read_cache() < 0)
+			die("failed to read the cache");
+		return -ret;
+	}
 }
 
 static void count_diff_files(struct diff_queue_struct *q,
@@ -743,10 +780,6 @@ static int evaluate_result(void)
 	int cnt = 0;
 	struct rev_info rev;
 
-	discard_cache();
-	if (read_cache() < 0)
-		die("failed to read the cache");
-
 	/* Check how many files differ. */
 	init_revisions(&rev, "");
 	setup_revisions(0, NULL, &rev, NULL);
@@ -880,12 +913,14 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 	for (i = 0; i < argc; i++) {
 		struct object *o;
+		struct commit *commit;
 
 		o = peel_to_type(argv[i], 0, NULL, OBJ_COMMIT);
 		if (!o)
 			die("%s - not something we can merge", argv[i]);
-		remotes = &commit_list_insert(lookup_commit(o->sha1),
-			remotes)->next;
+		commit = lookup_commit(o->sha1);
+		commit->util = (void *)argv[i];
+		remotes = &commit_list_insert(commit, remotes)->next;
 
 		strbuf_addf(&buf, "GITHEAD_%s", sha1_to_hex(o->sha1));
 		setenv(buf.buf, argv[i], 1);
@@ -1079,7 +1114,6 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		}
 
 		/* Automerge succeeded. */
-		discard_cache();
 		write_tree_trivial(result_tree);
 		automerge_was_ok = 1;
 		break;

-- 
1.6.0
Previous: Junio C HamanoNext: Junio C Hamano
Message 11 of 22 in “[bug] git `next' does not do trivial merges”
  1. Paolo BonziniAug 22, 2008
  2. Jeff KingAug 22, 2008
  3. Miklos VajnaAug 23, 2008
  4. Fix in-index merge.Miklos Vajna, Aug 23, 2008
  5. builtin-merge: fail properly when we are in the middle of a conflicted mergeMiklos Vajna, Aug 23, 2008
  6. Junio C HamanoAug 23, 2008
  7. builtin-merge: fail properly when we are in the middle of a conflicted mergeMiklos Vajna, Aug 23, 2008
  8. Junio C HamanoAug 23, 2008
  9. 1/2 merge: fix numerus bugs around "trivial merge" areaJunio C Hamano, Aug 23, 2008
  10. Junio C HamanoAug 24, 2008
  11. builtin-merge: avoid run_command_v_opt() for recursive and subtreeMiklos Vajna, Aug 28, 2008
  12. 2/2 unpack_trees(): protect the handcrafted in-core index from read_cache()Junio C Hamano, Aug 23, 2008
  13. Junio C HamanoAug 23, 2008
  14. Junio C HamanoAug 23, 2008
  15. Fix in-index merge.Miklos Vajna, Aug 23, 2008
  16. Paolo BonziniAug 23, 2008
  17. Junio C HamanoAug 23, 2008
  18. Junio C HamanoAug 23, 2008
  19. [RFH] two and half potential fixlets to the in-core index handlingJunio C Hamano, Aug 23, 2008
  20. Linus TorvaldsAug 23, 2008
  21. Junio C HamanoAug 23, 2008
  22. Junio C HamanoAug 23, 2008

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.