Skip to content

Commit 4e5d9de

Browse files
newrengitster
authored andcommitted
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 874cf2a (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 <[email protected]> Reviewed-by: Taylor Blau <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent a36e024 commit 4e5d9de

File tree

4 files changed

+93
-5
lines changed

4 files changed

+93
-5
lines changed

merge-ort-wrappers.c

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
11
#include "git-compat-util.h"
22
#include "gettext.h"
33
#include "hash.h"
4+
#include "hex.h"
5+
#include "lockfile.h"
46
#include "merge-ort.h"
57
#include "merge-ort-wrappers.h"
68
#include "read-cache-ll.h"
9+
#include "repository.h"
10+
#include "tag.h"
711
#include "tree.h"
812

913
#include "commit.h"
@@ -64,3 +68,63 @@ int merge_ort_recursive(struct merge_options *opt,
6468

6569
return tmp.clean;
6670
}
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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,16 @@ int merge_ort_recursive(struct merge_options *opt,
2222
const struct commit_list *ancestors,
2323
struct commit **result);
2424

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+
2537
#endif

merge-ort.c

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4878,9 +4878,9 @@ static inline void set_commit_tree(struct commit *c, struct tree *t)
48784878
c->maybe_tree = t;
48794879
}
48804880

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)
48844884
{
48854885
struct commit *commit = alloc_commit_node(repo);
48864886

@@ -5186,6 +5186,8 @@ static void merge_ort_internal(struct merge_options *opt,
51865186
ancestor_name = "empty tree";
51875187
} else if (merge_bases) {
51885188
ancestor_name = "merged common ancestors";
5189+
} else if (opt->ancestor) {
5190+
ancestor_name = opt->ancestor;
51895191
} else {
51905192
strbuf_add_unique_abbrev(&merge_base_abbrev,
51915193
&merged_merge_bases->object.oid,
@@ -5275,8 +5277,13 @@ void merge_incore_recursive(struct merge_options *opt,
52755277
{
52765278
trace2_region_enter("merge", "incore_recursive", opt->repo);
52775279

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));
52805287

52815288
trace2_region_enter("merge", "merge_start", opt->repo);
52825289
merge_start(opt, result);

merge-ort.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,11 @@ struct merge_result {
4444
unsigned _properly_initialized;
4545
};
4646

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+
4752
/*
4853
* rename-detecting three-way merge with recursive ancestor consolidation.
4954
* working tree and index are untouched.

0 commit comments

Comments
 (0)