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

[PATCH v2 1/6] merge-ort: add new merge_ort_generic() function

From
Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>
Date
Mar 13, 2025, 02:46 UTC
Message-ID
<c2f2e3e0cd646eae2801bf8fb079266f43219921.1741834001.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1875.v2.git.1741834001.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
merge-recursive.[ch] have three entry points:
  * merge_trees()
  * merge_recursive()
  * merge_recursive_generic()
merge-ort*.[ch] only has equivalents for the first two.  Add an
equivalent for the final entry point, so we can switch callers to
use it and remove merge-recursive.[ch].

While porting it over, finally fix the issue with the label for the ancestor (used when merge.conflictStyle=diff3 as a conflict label). merge-recursive.c has traditionally not allowed callers to set that label, but I have found that problematic for years.

(Side note: This function was initially part of the merge-ort rewrite, but reviewers questioned the ancestor label funnyness which I was never really happy with anyway. It resulted in me jettisoning it and hoping at the time that I would eventually be able to force the existing callers to use some other API. That worked with `git stash`, as per 874cf2a60444 (stash: apply stash using 'merge_ort_nonrecursive()', 2022-05-10), but this API is the most reasonable one for `git am` and `git merge-recursive`, if we can just allow them some freedom over the ancestor label.)

The merge_recursive_generic() function did not know whether it was being invoked by `git stash`, `git merge-recursive`, or `git am`, and the choice of meaningful ancestor label, when there is a unique ancestor, varies for these different callers:

  * git am: ancestor is a constructed "fake ancestor" that user knows
            nothing about and has no access to.  (And is different than
            the normal thing we mean by a "virtual merge base" which is
            the merging of merge bases.)
  * git merge-recursive: ancestor might be a tree, but at least it
                         was one specified by the user (if they invoked
                         merge-recursive directly)
  * git stash: ancestor was the commit serving as the stash base

Thus, using a label like "constructed merge base" (as merge_recursive_generic() does) presupposes that `git am` is the only caller; it is incorrect for other callers. This label has thrown me off more than once. Allow the caller to override when there is a unique merge base.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-ort-wrappers.c | 64 ++++++++++++++++++++++++++++++++++++++++++++
 merge-ort-wrappers.h | 12 +++++++++
 merge-ort.c          | 17 ++++++++----
 merge-ort.h          |  5 ++++
 4 files changed, 93 insertions(+), 5 deletions(-)
diff --git a/merge-ort-wrappers.c b/merge-ort-wrappers.c
index d6f61359965..62834c30e9e 100644
--- a/merge-ort-wrappers.c
+++ b/merge-ort-wrappers.c
@@ -1,9 +1,13 @@
 #include "git-compat-util.h"
 #include "gettext.h"
 #include "hash.h"
+#include "hex.h"
+#include "lockfile.h"
 #include "merge-ort.h"
 #include "merge-ort-wrappers.h"
 #include "read-cache-ll.h"
+#include "repository.h"
+#include "tag.h"
 #include "tree.h"
 
 #include "commit.h"
@@ -64,3 +68,63 @@ int merge_ort_recursive(struct merge_options *opt,
 
 	return tmp.clean;
 }
+
+static struct commit *get_ref(struct repository *repo,
+			      const struct object_id *oid,
+			      const char *name)
+{
+	struct object *object;
+
+	object = deref_tag(repo, parse_object(repo, oid),
+			   name, strlen(name));
+	if (!object)
+		return NULL;
+	if (object->type == OBJ_TREE)
+		return make_virtual_commit(repo, (struct tree*)object, name);
+	if (object->type != OBJ_COMMIT)
+		return NULL;
+	if (repo_parse_commit(repo, (struct commit *)object))
+		return NULL;
+	return (struct commit *)object;
+}
+
+int merge_ort_generic(struct merge_options *opt,
+		      const struct object_id *head,
+		      const struct object_id *merge,
+		      int num_merge_bases,
+		      const struct object_id *merge_bases,
+		      struct commit **result)
+{
+	int clean;
+	struct lock_file lock = LOCK_INIT;
+	struct commit *head_commit = get_ref(opt->repo, head, opt->branch1);
+	struct commit *next_commit = get_ref(opt->repo, merge, opt->branch2);
+	struct commit_list *ca = NULL;
+
+	if (merge_bases) {
+		int i;
+		for (i = 0; i < num_merge_bases; ++i) {
+			struct commit *base;
+			if (!(base = get_ref(opt->repo, &merge_bases[i],
+					     oid_to_hex(&merge_bases[i]))))
+				return error(_("Could not parse object '%s'"),
+					     oid_to_hex(&merge_bases[i]));
+			commit_list_insert(base, &ca);
+		}
+	}
+
+	repo_hold_locked_index(opt->repo, &lock, LOCK_DIE_ON_ERROR);
+	clean = merge_ort_recursive(opt, head_commit, next_commit, ca,
+				    result);
+	free_commit_list(ca);
+	if (clean < 0) {
+		rollback_lock_file(&lock);
+		return clean;
+	}
+
+	if (write_locked_index(opt->repo->index, &lock,
+			       COMMIT_LOCK | SKIP_IF_UNCHANGED))
+		return error(_("Unable to write index."));
+
+	return clean ? 0 : 1;
+}
diff --git a/merge-ort-wrappers.h b/merge-ort-wrappers.h
index 90af1f69c55..aeffa1c87b4 100644
--- a/merge-ort-wrappers.h
+++ b/merge-ort-wrappers.h
@@ -22,4 +22,16 @@ int merge_ort_recursive(struct merge_options *opt,
 			const struct commit_list *ancestors,
 			struct commit **result);
 
+/*
+ * rename-detecting three-way merge.  num_merge_bases must be at least 1.
+ * Recursive ancestor consolidation will be performed if num_merge_bases > 1.
+ * Wrapper mimicking the old merge_recursive_generic() function.
+ */
+int merge_ort_generic(struct merge_options *opt,
+		      const struct object_id *head,
+		      const struct object_id *merge,
+		      int num_merge_bases,
+		      const struct object_id *merge_bases,
+		      struct commit **result);
+
 #endif
diff --git a/merge-ort.c b/merge-ort.c
index 46e78c3ffa6..b4ff24403a1 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -4878,9 +4878,9 @@ static inline void set_commit_tree(struct commit *c, struct tree *t)
 	c->maybe_tree = t;
 }
 
-static struct commit *make_virtual_commit(struct repository *repo,
-					  struct tree *tree,
-					  const char *comment)
+struct commit *make_virtual_commit(struct repository *repo,
+				   struct tree *tree,
+				   const char *comment)
 {
 	struct commit *commit = alloc_commit_node(repo);
 
@@ -5186,6 +5186,8 @@ static void merge_ort_internal(struct merge_options *opt,
 		ancestor_name = "empty tree";
 	} else if (merge_bases) {
 		ancestor_name = "merged common ancestors";
+	} else if (opt->ancestor) {
+		ancestor_name = opt->ancestor;
 	} else {
 		strbuf_add_unique_abbrev(&merge_base_abbrev,
 					 &merged_merge_bases->object.oid,
@@ -5275,8 +5277,13 @@ void merge_incore_recursive(struct merge_options *opt,
 {
 	trace2_region_enter("merge", "incore_recursive", opt->repo);
 
-	/* We set the ancestor label based on the merge_bases */
-	assert(opt->ancestor == NULL);
+	/*
+	 * We set the ancestor label based on the merge_bases...but we
+	 * allow one exception through so that builtin/am can override
+	 * with its constructed fake ancestor.
+	 */
+	assert(opt->ancestor == NULL ||
+	       (merge_bases && !merge_bases->next));
 
 	trace2_region_enter("merge", "merge_start", opt->repo);
 	merge_start(opt, result);
diff --git a/merge-ort.h b/merge-ort.h
index 82f2b3222d2..b63bc5424e7 100644
--- a/merge-ort.h
+++ b/merge-ort.h
@@ -44,6 +44,11 @@ struct merge_result {
 	unsigned _properly_initialized;
 };
 
+/* Mostly internal function also used by merge-ort-wrappers.c */
+struct commit *make_virtual_commit(struct repository *repo,
+				   struct tree *tree,
+				   const char *comment);
+
 /*
  * rename-detecting three-way merge with recursive ancestor consolidation.
  * working tree and index are untouched.
-- 
gitgitgadget
Previous: Elijah Newren via GitGitGadgetNext: Elijah Newren via GitGitGadget
Message 19 of 25 in “Small new merge-ort features, prepping for deletion of merge-recursive.[ch]”
  1. 0/3 Small new merge-ort features, prepping for deletion of merge-recursive.[ch]Elijah Newren via GitGitGadget, Mar 7, 2025
  2. 1/3 merge-ort: add new merge_ort_generic() functionElijah Newren via GitGitGadget, Mar 7, 2025
  3. Patrick SteinhardtMar 12, 2025
  4. Taylor BlauMar 12, 2025
  5. Elijah NewrenMar 12, 2025
  6. 2/3 merge-ort: allow rename detection to be disabledElijah Newren via GitGitGadget, Mar 7, 2025
  7. Patrick SteinhardtMar 12, 2025
  8. Taylor BlauMar 12, 2025
  9. Elijah NewrenMar 12, 2025
  10. Taylor BlauMar 12, 2025
  11. Jeff KingMar 13, 2025
  12. 3/3 merge-ort: support having merge verbosity be set to 0Elijah Newren via GitGitGadget, Mar 7, 2025
  13. Taylor BlauMar 12, 2025
  14. Elijah NewrenMar 12, 2025
  15. Taylor BlauMar 12, 2025
  16. Patrick SteinhardtMar 12, 2025
  17. Taylor BlauMar 12, 2025
  18. 0/6 Small new merge-ort features, prepping for deletion of merge-recursive.[ch]Elijah Newren via GitGitGadget, Mar 13, 2025
  19. 1/6 merge-ort: add new merge_ort_generic() functionElijah Newren via GitGitGadget, Mar 13, 2025
  20. 3/6 merge-ort: support having merge verbosity be set to 0Elijah Newren via GitGitGadget, Mar 13, 2025
  21. 2/6 merge-ort: allow rename detection to be disabledElijah Newren via GitGitGadget, Mar 13, 2025
  22. 4/6 t3650: document bug when directory renames are turned offJohannes Schindelin via GitGitGadget, Mar 13, 2025
  23. 5/6 merge-ort: fix merge.directoryRenames=falseElijah Newren via GitGitGadget, Mar 13, 2025
  24. 6/6 am: switch from merge_recursive_generic() to merge_ort_generic()Elijah Newren via GitGitGadget, Mar 13, 2025
  25. Taylor BlauMar 17, 2025

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.