@matheusbsbs / matheus-bagliano / commits / 4e5d9de96c

merge-ort: add new merge_ort_generic() function

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> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Mar 13, 2025 at 02:46 UTC 4e5d9de96c5a9d2202530aaedbf638540811bd58
4 files changed +93 -5
merge-ort-wrappers.c
+64
@@ -1,9 +1,13 @@
1 #include "git-compat-util.h"
2 #include "gettext.h"
3 #include "hash.h"
4 +#include "hex.h"
5 +#include "lockfile.h"
6 #include "merge-ort.h"
7 #include "merge-ort-wrappers.h"
8 #include "read-cache-ll.h"
9 +#include "repository.h"
10 +#include "tag.h"
11 #include "tree.h"
12
13 #include "commit.h"
@@ -64,3 +68,63 @@ int merge_ort_recursive(struct merge_options *opt,
68
69 return tmp.clean;
70 }
71 +
72 +static struct commit *get_ref(struct repository *repo,
73 + const struct object_id *oid,
74 + const char *name)
75 +{
76 + struct object *object;
77 +
78 + object = deref_tag(repo, parse_object(repo, oid),
79 + name, strlen(name));
80 + if (!object)
81 + return NULL;
82 + if (object->type == OBJ_TREE)
83 + return make_virtual_commit(repo, (struct tree*)object, name);
84 + if (object->type != OBJ_COMMIT)
85 + return NULL;
86 + if (repo_parse_commit(repo, (struct commit *)object))
87 + return NULL;
88 + return (struct commit *)object;
89 +}
90 +
91 +int merge_ort_generic(struct merge_options *opt,
92 + const struct object_id *head,
93 + const struct object_id *merge,
94 + int num_merge_bases,
95 + const struct object_id *merge_bases,
96 + struct commit **result)
97 +{
98 + int clean;
99 + struct lock_file lock = LOCK_INIT;
100 + struct commit *head_commit = get_ref(opt->repo, head, opt->branch1);
101 + struct commit *next_commit = get_ref(opt->repo, merge, opt->branch2);
102 + struct commit_list *ca = NULL;
103 +
104 + if (merge_bases) {
105 + int i;
106 + for (i = 0; i < num_merge_bases; ++i) {
107 + struct commit *base;
108 + if (!(base = get_ref(opt->repo, &merge_bases[i],
109 + oid_to_hex(&merge_bases[i]))))
110 + return error(_("Could not parse object '%s'"),
111 + oid_to_hex(&merge_bases[i]));
112 + commit_list_insert(base, &ca);
113 + }
114 + }
115 +
116 + repo_hold_locked_index(opt->repo, &lock, LOCK_DIE_ON_ERROR);
117 + clean = merge_ort_recursive(opt, head_commit, next_commit, ca,
118 + result);
119 + free_commit_list(ca);
120 + if (clean < 0) {
121 + rollback_lock_file(&lock);
122 + return clean;
123 + }
124 +
125 + if (write_locked_index(opt->repo->index, &lock,
126 + COMMIT_LOCK | SKIP_IF_UNCHANGED))
127 + return error(_("Unable to write index."));
128 +
129 + return clean ? 0 : 1;
130 +}
merge-ort-wrappers.h
+12
@@ -22,4 +22,16 @@ int merge_ort_recursive(struct merge_options *opt,
22 const struct commit_list *ancestors,
23 struct commit **result);
24
25 +/*
26 + * rename-detecting three-way merge. num_merge_bases must be at least 1.
27 + * Recursive ancestor consolidation will be performed if num_merge_bases > 1.
28 + * Wrapper mimicking the old merge_recursive_generic() function.
29 + */
30 +int merge_ort_generic(struct merge_options *opt,
31 + const struct object_id *head,
32 + const struct object_id *merge,
33 + int num_merge_bases,
34 + const struct object_id *merge_bases,
35 + struct commit **result);
36 +
37 #endif
merge-ort.c
+12 -5
@@ -4878,9 +4878,9 @@ static inline void set_commit_tree(struct commit *c, struct tree *t)
4878 c->maybe_tree = t;
4879 }
4880
4881 -static struct commit *make_virtual_commit(struct repository *repo,
4882 - struct tree *tree,
4883 - const char *comment)
4881 +struct commit *make_virtual_commit(struct repository *repo,
4882 + struct tree *tree,
4883 + const char *comment)
4884 {
4885 struct commit *commit = alloc_commit_node(repo);
4886
@@ -5186,6 +5186,8 @@ static void merge_ort_internal(struct merge_options *opt,
5186 ancestor_name = "empty tree";
5187 } else if (merge_bases) {
5188 ancestor_name = "merged common ancestors";
5189 + } else if (opt->ancestor) {
5190 + ancestor_name = opt->ancestor;
5191 } else {
5192 strbuf_add_unique_abbrev(&merge_base_abbrev,
5193 &merged_merge_bases->object.oid,
@@ -5275,8 +5277,13 @@ void merge_incore_recursive(struct merge_options *opt,
5277 {
5278 trace2_region_enter("merge", "incore_recursive", opt->repo);
5279
5278 - /* We set the ancestor label based on the merge_bases */
5279 - assert(opt->ancestor == NULL);
5280 + /*
5281 + * We set the ancestor label based on the merge_bases...but we
5282 + * allow one exception through so that builtin/am can override
5283 + * with its constructed fake ancestor.
5284 + */
5285 + assert(opt->ancestor == NULL ||
5286 + (merge_bases && !merge_bases->next));
5287
5288 trace2_region_enter("merge", "merge_start", opt->repo);
5289 merge_start(opt, result);
merge-ort.h
+5
@@ -44,6 +44,11 @@ struct merge_result {
44 unsigned _properly_initialized;
45 };
46
47 +/* Mostly internal function also used by merge-ort-wrappers.c */
48 +struct commit *make_virtual_commit(struct repository *repo,
49 + struct tree *tree,
50 + const char *comment);
51 +
52 /*
53 * rename-detecting three-way merge with recursive ancestor consolidation.
54 * working tree and index are untouched.