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

[PATCH (1b)] merge-recursive.c: Add more generic merge_recursive_generic()

From
Stephan Beyer <s-beyer@gmx.net>
Date
Aug 12, 2008, 20:13 UTC
Message-ID
<1218572040-23362-1-git-send-email-s-beyer@gmx.net>
In-Reply-To
<1218559514-16890-1-git-send-email-vmiklos@frugalware.org>

merge_recursive_generic() takes, in comparison to to merge_recursive(), no commit ("struct commit *") arguments but SHA ids ("unsigned char *"), and no commit list of bases but an array of refs ("const char **").

This makes it more generic in the case that it can also take the SHA of a tree to merge trees without commits, for the bases, the head and the remote.

merge_recursive_generic() also handles locking and updating of the index, which is a common use case of merge_recursive().

This patch also rewrites builtin-merge-recursive.c to make use of merge_recursive_generic(). By doing this, I stumbled over the limitation of 20 bases and I've added a warning if this limitation is exceeded.

This patch qualifies make_virtual_commit() as static again because this function is not needed anymore outside merge-recursive.c.

Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
	Hi,
	so this patch is marked as (1b) since it is based on Miklos'
	patch that I called (1a) in my mind :-)
	I'm unsure if it's good to squash both, but that's the decision
	of the maintainer - if taken at all.
	Some further remark that I thought about when writig the commit
	message:
	 1. It could be cleaner to choose a "struct string_list" for the
	    base_list in merge_recursive_generic()
	 2. If we do so, we've had lifted the limitation to 20 bases
	    automatically.
	 2. Since the patchset that is in work in this thread is for
	    post 1.6.0, I wonder if there should be a patch for the
	    20 bases limitation in git-merge-recursive for 1.6.0.
	    Either lifting it (ALLOC_GROW foo bar) or printing a warning.
	
	Regards,
	  Stephan
 builtin-merge-recursive.c |   64 +++++++++++++-------------------------------
 merge-recursive.c         |   53 +++++++++++++++++++++++++++++++++++++
 merge-recursive.h         |    4 ++-
 3 files changed, 75 insertions(+), 46 deletions(-)
diff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c
index 8bf2fa5..25f540b 100644
--- a/builtin-merge-recursive.c
+++ b/builtin-merge-recursive.c
@@ -15,36 +15,13 @@ static const char *better_branch_name(const char *branch)
 	return name ? name : branch;
 }
 
-static struct commit *get_ref(const char *ref)
-{
-	unsigned char sha1[20];
-	struct object *object;
-
-	if (get_sha1(ref, sha1))
-		die("Could not resolve ref '%s'", ref);
-	object = deref_tag(parse_object(sha1), ref, strlen(ref));
-	if (!object)
-		return NULL;
-	if (object->type == OBJ_TREE)
-		return make_virtual_commit((struct tree*)object,
-			better_branch_name(ref));
-	if (object->type != OBJ_COMMIT)
-		return NULL;
-	if (parse_commit((struct commit *)object))
-		die("Could not parse commit '%s'", sha1_to_hex(object->sha1));
-	return (struct commit *)object;
-}
-
 int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
 {
-	static const char *bases[20];
-	static unsigned bases_count = 0;
-	int i, clean;
+	const char *bases[21];
+	unsigned bases_count = 0;
+	int i, failed;
 	const char *branch1, *branch2;
-	struct commit *result, *h1, *h2;
-	struct commit_list *ca = NULL;
-	struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
-	int index_fd;
+	unsigned char h1[20], h2[20];
 	int subtree_merge = 0;
 
 	if (argv[0]) {
@@ -60,10 +37,15 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
 		die("Usage: %s <base>... -- <head> <remote> ...\n", argv[0]);
 
 	for (i = 1; i < argc; ++i) {
-		if (!strcmp(argv[i], "--"))
+		if (!strcmp(argv[i], "--")) {
+			bases[bases_count] = NULL;
 			break;
-		if (bases_count < sizeof(bases)/sizeof(*bases))
+		}
+		if (bases_count < ARRAY_SIZE(bases)-1)
 			bases[bases_count++] = argv[i];
+		else
+			warning("Cannot handle more than %zu bases. "
+				"Ignoring %s.", ARRAY_SIZE(bases)-1, argv[i]);
 	}
 	if (argc - i != 3) /* "--" "<head>" "<remote>" */
 		die("Not handling anything other than two heads merge.");
@@ -71,8 +53,10 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
 	branch1 = argv[++i];
 	branch2 = argv[++i];
 
-	h1 = get_ref(branch1);
-	h2 = get_ref(branch2);
+	if (get_sha1(branch1, h1))
+		die("Could not resolve ref '%s'", branch1);
+	if (get_sha1(branch2, h2))
+		die("Could not resolve ref '%s'", branch2);
 
 	branch1 = better_branch_name(branch1);
 	branch2 = better_branch_name(branch2);
@@ -80,18 +64,8 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
 	if (merge_recursive_verbosity >= 3)
 		printf("Merging %s with %s\n", branch1, branch2);
 
-	index_fd = hold_locked_index(lock, 1);
-
-	for (i = 0; i < bases_count; i++) {
-		struct commit *ancestor = get_ref(bases[i]);
-		ca = commit_list_insert(ancestor, &ca);
-	}
-	clean = merge_recursive(h1, h2, branch1, branch2, ca, &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;
+	failed = merge_recursive_generic(bases, h1, branch1, h2, branch2);
+	if (failed < 0)
+		return 128; /* die() error code */
+	return failed;
 }
diff --git a/merge-recursive.c b/merge-recursive.c
index c3a57ac..74a9fdc 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1303,6 +1303,59 @@ int merge_recursive(struct commit *h1,
 	return clean;
 }
 
+static struct commit *get_ref(const unsigned char *sha1, const char *name)
+{
+	struct object *object;
+
+	object = deref_tag(parse_object(sha1), name, strlen(name));
+	if (!object)
+		return NULL;
+	if (object->type == OBJ_TREE)
+		return make_virtual_commit((struct tree*)object, name);
+	if (object->type != OBJ_COMMIT)
+		return NULL;
+	if (parse_commit((struct commit *)object))
+		return NULL;
+	return (struct commit *)object;
+}
+
+int merge_recursive_generic(const char **base_list,
+		const unsigned char *head_sha1, const char *head_name,
+		const unsigned char *next_sha1, const char *next_name)
+{
+	int clean, index_fd;
+	struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
+	struct commit *result;
+	struct commit *head_commit = get_ref(head_sha1, head_name);
+	struct commit *next_commit = get_ref(next_sha1, next_name);
+	struct commit_list *ca = NULL;
+
+	if (base_list) {
+		int i;
+		for (i = 0; base_list[i]; ++i) {
+			unsigned char sha[20];
+			struct commit *base;
+			if (get_sha1(base_list[i], sha))
+				return error("Could not resolve ref '%s'",
+								base_list[i]);
+			if (!(base = get_ref(sha, base_list[i])))
+				return error("Could not parse object '%s'",
+								base_list[i]);
+			commit_list_insert(base, &ca);
+		}
+	}
+
+	index_fd = hold_locked_index(lock, 1);
+	clean = merge_recursive(head_commit, next_commit,
+				head_name, next_name, ca, &result);
+	if (active_cache_changed &&
+			(write_cache(index_fd, active_cache, active_nr) ||
+			 commit_locked_index(lock)))
+		return error("Unable to write index.");
+
+	return clean ? 0 : 1;
+}
+
 int merge_recursive_config(const char *var, const char *value, void *cb)
 {
 	if (!strcasecmp(var, "merge.verbosity")) {
diff --git a/merge-recursive.h b/merge-recursive.h
index 73e4413..4dd6476 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -14,7 +14,9 @@ int merge_trees(struct tree *head,
 		const char *branch1,
 		const char *branch2,
 		struct tree **result);
-struct commit *make_virtual_commit(struct tree *tree, const char *comment);
+extern int merge_recursive_generic(const char **base_list,
+		const unsigned char *head_sha1, const char *head_name,
+		const unsigned char *next_sha1, const char *next_name);
 int merge_recursive_config(const char *var, const char *value, void *cb);
 void merge_recursive_setup(int is_subtree_merge);
 struct tree *write_tree_from_memory(void);

-- 
1.6.0.rc2.281.g6f6cf
Previous: Miklos VajnaNext: Stephan Beyer
Message 26 of 38 in “Avoid run_command() for recursive in builtin-merge”
  1. 0/2 Avoid run_command() for recursive in builtin-mergeMiklos Vajna, Aug 10, 2008
  2. 1/2 merge-recursive: prepare merge_recursive() to be called from builtinsMiklos Vajna, Aug 10, 2008
  3. 2/2 builtin-merge: avoid run_command_v_opt() for recursiveMiklos Vajna, Aug 10, 2008
  4. Junio C HamanoAug 11, 2008
  5. Miklos VajnaAug 11, 2008
  6. Junio C HamanoAug 11, 2008
  7. Miklos VajnaAug 11, 2008
  8. Add a new test to ensure merging a submodule is handled properly.Miklos Vajna, Aug 11, 2008
  9. Stephan BeyerAug 11, 2008
  10. Miklos VajnaAug 11, 2008
  11. Junio C HamanoAug 11, 2008
  12. Stephan BeyerAug 11, 2008
  13. builtin-revert.c: Make use of merge_recursive()Stephan Beyer, Aug 11, 2008
  14. Johannes SchindelinAug 11, 2008
  15. Stephan BeyerAug 11, 2008
  16. Miklos VajnaAug 11, 2008
  17. builtin-revert: Make use of merge_recursive()Stephan Beyer, Aug 11, 2008
  18. Stephan BeyerAug 11, 2008
  19. Junio C HamanoAug 11, 2008
  20. Junio C HamanoAug 11, 2008
  21. Stephan BeyerAug 11, 2008
  22. Junio C HamanoAug 11, 2008
  23. Split out merge_recursive() to merge-recursive.cMiklos Vajna, Aug 12, 2008
  24. Stephan BeyerAug 12, 2008
  25. Miklos VajnaAug 12, 2008
  26. merge-recursive.c: Add more generic merge_recursive_generic()Stephan Beyer, Aug 12, 2008
  27. Make builtin-revert.c use merge_recursive_generic()Stephan Beyer, Aug 12, 2008
  28. Miklos VajnaAug 12, 2008
  29. Stephan BeyerAug 13, 2008
  30. Miklos VajnaAug 13, 2008
  31. Daniel BarkalowAug 13, 2008
  32. Stephan BeyerAug 13, 2008
  33. Daniel BarkalowAug 13, 2008
  34. Junio C HamanoAug 13, 2008
  35. Stephan BeyerAug 13, 2008
  36. Daniel BarkalowAug 13, 2008
  37. Junio C HamanoAug 13, 2008
  38. Junio C HamanoAug 14, 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.