Skip to content

Commit 68d03e4

Browse files
hvoigtgitster
authored andcommitted
Implement automatic fast-forward merge for submodules
This implements a simple merge strategy for submodule hashes. We check whether one side of the merge candidates is already contained in the other and then merge automatically. If both sides contain changes we search for a merge in the submodule. In case a single one exists we check that out and suggest it as the merge resolution. A list of candidates is returned when we find multiple merges that contain both sides of the changes. This is useful for a workflow in which the developers can publish topic branches in submodules and a separate maintainer merges them. In case the developers always wait until their branch gets merged before tracking them in the superproject all merges of branches that contain submodule changes will be resolved automatically. If developers choose to track their feature branch the maintainer might get a conflict but git will search the submodule for a merge and suggest it/them as a resolution. Signed-off-by: Heiko Voigt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 9ef6aeb commit 68d03e4

File tree

4 files changed

+291
-9
lines changed

4 files changed

+291
-9
lines changed

merge-recursive.c

+6-3
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "attr.h"
2121
#include "merge-recursive.h"
2222
#include "dir.h"
23+
#include "submodule.h"
2324

2425
static struct tree *shift_tree_object(struct tree *one, struct tree *two,
2526
const char *subtree_shift)
@@ -525,13 +526,15 @@ static void update_file_flags(struct merge_options *o,
525526
void *buf;
526527
unsigned long size;
527528

528-
if (S_ISGITLINK(mode))
529+
if (S_ISGITLINK(mode)) {
529530
/*
530531
* We may later decide to recursively descend into
531532
* the submodule directory and update its index
532533
* and/or work tree, but we do not do that now.
533534
*/
535+
update_wd = 0;
534536
goto update_index;
537+
}
535538

536539
buf = read_sha1_file(sha, &type, &size);
537540
if (!buf)
@@ -716,8 +719,8 @@ static struct merge_file_info merge_file(struct merge_options *o,
716719
free(result_buf.ptr);
717720
result.clean = (merge_status == 0);
718721
} else if (S_ISGITLINK(a->mode)) {
719-
result.clean = 0;
720-
hashcpy(result.sha, a->sha1);
722+
result.clean = merge_submodule(result.sha, one->path, one->sha1,
723+
a->sha1, b->sha1);
721724
} else if (S_ISLNK(a->mode)) {
722725
hashcpy(result.sha, a->sha1);
723726

submodule.c

+161
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "revision.h"
77
#include "run-command.h"
88
#include "diffcore.h"
9+
#include "refs.h"
910

1011
static int add_submodule_odb(const char *path)
1112
{
@@ -218,3 +219,163 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)
218219
strbuf_release(&buf);
219220
return dirty_submodule;
220221
}
222+
223+
static int find_first_merges(struct object_array *result, const char *path,
224+
struct commit *a, struct commit *b)
225+
{
226+
int i, j;
227+
struct object_array merges;
228+
struct commit *commit;
229+
int contains_another;
230+
231+
char merged_revision[42];
232+
const char *rev_args[] = { "rev-list", "--merges", "--ancestry-path",
233+
"--all", merged_revision, NULL };
234+
struct rev_info revs;
235+
struct setup_revision_opt rev_opts;
236+
237+
memset(&merges, 0, sizeof(merges));
238+
memset(result, 0, sizeof(struct object_array));
239+
memset(&rev_opts, 0, sizeof(rev_opts));
240+
241+
/* get all revisions that merge commit a */
242+
snprintf(merged_revision, sizeof(merged_revision), "^%s",
243+
sha1_to_hex(a->object.sha1));
244+
init_revisions(&revs, NULL);
245+
rev_opts.submodule = path;
246+
setup_revisions(sizeof(rev_args)/sizeof(char *)-1, rev_args, &revs, &rev_opts);
247+
248+
/* save all revisions from the above list that contain b */
249+
if (prepare_revision_walk(&revs))
250+
die("revision walk setup failed");
251+
while ((commit = get_revision(&revs)) != NULL) {
252+
struct object *o = &(commit->object);
253+
if (in_merge_bases(b, &commit, 1))
254+
add_object_array(o, NULL, &merges);
255+
}
256+
257+
/* Now we've got all merges that contain a and b. Prune all
258+
* merges that contain another found merge and save them in
259+
* result.
260+
*/
261+
for (i = 0; i < merges.nr; i++) {
262+
struct commit *m1 = (struct commit *) merges.objects[i].item;
263+
264+
contains_another = 0;
265+
for (j = 0; j < merges.nr; j++) {
266+
struct commit *m2 = (struct commit *) merges.objects[j].item;
267+
if (i != j && in_merge_bases(m2, &m1, 1)) {
268+
contains_another = 1;
269+
break;
270+
}
271+
}
272+
273+
if (!contains_another)
274+
add_object_array(merges.objects[i].item,
275+
merges.objects[i].name, result);
276+
}
277+
278+
free(merges.objects);
279+
return result->nr;
280+
}
281+
282+
static void print_commit(struct commit *commit)
283+
{
284+
struct strbuf sb = STRBUF_INIT;
285+
struct pretty_print_context ctx = {0};
286+
ctx.date_mode = DATE_NORMAL;
287+
format_commit_message(commit, " %h: %m %s", &sb, &ctx);
288+
fprintf(stderr, "%s\n", sb.buf);
289+
strbuf_release(&sb);
290+
}
291+
292+
#define MERGE_WARNING(path, msg) \
293+
warning("Failed to merge submodule %s (%s)", path, msg);
294+
295+
int merge_submodule(unsigned char result[20], const char *path,
296+
const unsigned char base[20], const unsigned char a[20],
297+
const unsigned char b[20])
298+
{
299+
struct commit *commit_base, *commit_a, *commit_b;
300+
int parent_count;
301+
struct object_array merges;
302+
303+
int i;
304+
305+
/* store a in result in case we fail */
306+
hashcpy(result, a);
307+
308+
/* we can not handle deletion conflicts */
309+
if (is_null_sha1(base))
310+
return 0;
311+
if (is_null_sha1(a))
312+
return 0;
313+
if (is_null_sha1(b))
314+
return 0;
315+
316+
if (add_submodule_odb(path)) {
317+
MERGE_WARNING(path, "not checked out");
318+
return 0;
319+
}
320+
321+
if (!(commit_base = lookup_commit_reference(base)) ||
322+
!(commit_a = lookup_commit_reference(a)) ||
323+
!(commit_b = lookup_commit_reference(b))) {
324+
MERGE_WARNING(path, "commits not present");
325+
return 0;
326+
}
327+
328+
/* check whether both changes are forward */
329+
if (!in_merge_bases(commit_base, &commit_a, 1) ||
330+
!in_merge_bases(commit_base, &commit_b, 1)) {
331+
MERGE_WARNING(path, "commits don't follow merge-base");
332+
return 0;
333+
}
334+
335+
/* Case #1: a is contained in b or vice versa */
336+
if (in_merge_bases(commit_a, &commit_b, 1)) {
337+
hashcpy(result, b);
338+
return 1;
339+
}
340+
if (in_merge_bases(commit_b, &commit_a, 1)) {
341+
hashcpy(result, a);
342+
return 1;
343+
}
344+
345+
/*
346+
* Case #2: There are one or more merges that contain a and b in
347+
* the submodule. If there is only one, then present it as a
348+
* suggestion to the user, but leave it marked unmerged so the
349+
* user needs to confirm the resolution.
350+
*/
351+
352+
/* find commit which merges them */
353+
parent_count = find_first_merges(&merges, path, commit_a, commit_b);
354+
switch (parent_count) {
355+
case 0:
356+
MERGE_WARNING(path, "merge following commits not found");
357+
break;
358+
359+
case 1:
360+
MERGE_WARNING(path, "not fast-forward");
361+
fprintf(stderr, "Found a possible merge resolution "
362+
"for the submodule:\n");
363+
print_commit((struct commit *) merges.objects[0].item);
364+
fprintf(stderr,
365+
"If this is correct simply add it to the index "
366+
"for example\n"
367+
"by using:\n\n"
368+
" git update-index --cacheinfo 160000 %s \"%s\"\n\n"
369+
"which will accept this suggestion.\n",
370+
sha1_to_hex(merges.objects[0].item->sha1), path);
371+
break;
372+
373+
default:
374+
MERGE_WARNING(path, "multiple merges found");
375+
for (i = 0; i < merges.nr; i++)
376+
print_commit((struct commit *) merges.objects[i].item);
377+
}
378+
379+
free(merges.objects);
380+
return 0;
381+
}

submodule.h

+2
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,7 @@ void show_submodule_summary(FILE *f, const char *path,
99
unsigned dirty_submodule,
1010
const char *del, const char *add, const char *reset);
1111
unsigned is_submodule_modified(const char *path, int ignore_untracked);
12+
int merge_submodule(unsigned char result[20], const char *path, const unsigned char base[20],
13+
const unsigned char a[20], const unsigned char b[20]);
1214

1315
#endif

t/t7405-submodule-merge.sh

+122-6
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,129 @@ test_expect_success setup '
5454
git merge -s ours a
5555
'
5656

57-
test_expect_success 'merging with modify/modify conflict' '
57+
# History setup
58+
#
59+
# b
60+
# / \
61+
# a d
62+
# \ /
63+
# c
64+
#
65+
# a in the main repository records to sub-a in the submodule and
66+
# analogous b and c. d should be automatically found by merging c into
67+
# b in the main repository.
68+
test_expect_success 'setup for merge search' '
69+
mkdir merge-search &&
70+
cd merge-search &&
71+
git init &&
72+
mkdir sub &&
73+
(cd sub &&
74+
git init &&
75+
echo "file-a" > file-a &&
76+
git add file-a &&
77+
git commit -m "sub-a" &&
78+
git branch sub-a) &&
79+
git add sub &&
80+
git commit -m "a" &&
81+
git branch a &&
82+
83+
git checkout -b b &&
84+
(cd sub &&
85+
git checkout -b sub-b &&
86+
echo "file-b" > file-b &&
87+
git add file-b &&
88+
git commit -m "sub-b") &&
89+
git commit -a -m "b" &&
90+
91+
git checkout -b c a &&
92+
(cd sub &&
93+
git checkout -b sub-c sub-a &&
94+
echo "file-c" > file-c &&
95+
git add file-c &&
96+
git commit -m "sub-c") &&
97+
git commit -a -m "c" &&
98+
99+
git checkout -b d a &&
100+
(cd sub &&
101+
git checkout -b sub-d sub-b &&
102+
git merge sub-c) &&
103+
git commit -a -m "d" &&
104+
git branch test b &&
105+
cd ..
106+
'
107+
108+
test_expect_success 'merge with one side as a fast-forward of the other' '
109+
(cd merge-search &&
110+
git checkout -b test-forward b &&
111+
git merge d &&
112+
git ls-tree test-forward sub | cut -f1 | cut -f3 -d" " > actual &&
113+
(cd sub &&
114+
git rev-parse sub-d > ../expect) &&
115+
test_cmp actual expect)
116+
'
117+
118+
test_expect_success 'merging should conflict for non fast-forward' '
119+
(cd merge-search &&
120+
git checkout -b test-nonforward b &&
121+
(cd sub &&
122+
git rev-parse sub-d > ../expect) &&
123+
test_must_fail git merge c 2> actual &&
124+
grep $(cat expect) actual > /dev/null &&
125+
git reset --hard)
126+
'
127+
128+
test_expect_success 'merging should fail for ambiguous common parent' '
129+
cd merge-search &&
130+
git checkout -b test-ambiguous b &&
131+
(cd sub &&
132+
git checkout -b ambiguous sub-b &&
133+
git merge sub-c &&
134+
git rev-parse sub-d > ../expect1 &&
135+
git rev-parse ambiguous > ../expect2) &&
136+
test_must_fail git merge c 2> actual &&
137+
grep $(cat expect1) actual > /dev/null &&
138+
grep $(cat expect2) actual > /dev/null &&
139+
git reset --hard &&
140+
cd ..
141+
'
142+
143+
# in a situation like this
144+
#
145+
# submodule tree:
146+
#
147+
# sub-a --- sub-b --- sub-d
148+
#
149+
# main tree:
150+
#
151+
# e (sub-a)
152+
# /
153+
# bb (sub-b)
154+
# \
155+
# f (sub-d)
156+
#
157+
# A merge between e and f should fail because one of the submodule
158+
# commits (sub-a) does not descend from the submodule merge-base (sub-b).
159+
#
160+
test_expect_success 'merging should fail for changes that are backwards' '
161+
cd merge-search &&
162+
git checkout -b bb a &&
163+
(cd sub &&
164+
git checkout sub-b) &&
165+
git commit -a -m "bb" &&
166+
167+
git checkout -b e bb &&
168+
(cd sub &&
169+
git checkout sub-a) &&
170+
git commit -a -m "e" &&
171+
172+
git checkout -b f bb &&
173+
(cd sub &&
174+
git checkout sub-d) &&
175+
git commit -a -m "f" &&
58176
59-
git checkout -b test1 a &&
60-
test_must_fail git merge b &&
61-
test -f .git/MERGE_MSG &&
62-
git diff &&
63-
test -n "$(git ls-files -u)"
177+
git checkout -b test-backward e &&
178+
test_must_fail git merge f &&
179+
cd ..
64180
'
65181

66182
test_expect_success 'merging with a modify/modify conflict between merge bases' '

0 commit comments

Comments
 (0)