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 seperate 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 <hvoigt@xxxxxxxxxx> --- This patch replaces the last one of the previously discussed series. It is still work in progress but already implements the scheme discussed. I have not had the time to adjust the tests so they fail currently. The extension to setup_revisions() is still very hackyish which I will also cleanup in a later iteration. The whole series can also be found on: http://github.com/hvoigt/git/tree/submodule_merge_v2 merge-recursive.c | 9 ++- refs.c | 6 ++ refs.h | 1 + revision.c | 29 ++++++--- revision.h | 1 + submodule.c | 155 ++++++++++++++++++++++++++++++++++++++++++++ submodule.h | 2 + t/t7405-submodule-merge.sh | 115 +++++++++++++++++++++++++++++++-- 8 files changed, 300 insertions(+), 18 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index 206c103..a032a8b 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -20,6 +20,7 @@ #include "attr.h" #include "merge-recursive.h" #include "dir.h" +#include "submodule.h" static struct tree *shift_tree_object(struct tree *one, struct tree *two, const char *subtree_shift) @@ -525,13 +526,15 @@ static void update_file_flags(struct merge_options *o, void *buf; unsigned long size; - if (S_ISGITLINK(mode)) + if (S_ISGITLINK(mode)) { /* * We may later decide to recursively descend into * the submodule directory and update its index * and/or work tree, but we do not do that now. */ + update_wd = 0; goto update_index; + } buf = read_sha1_file(sha, &type, &size); if (!buf) @@ -716,8 +719,8 @@ static struct merge_file_info merge_file(struct merge_options *o, free(result_buf.ptr); result.clean = (merge_status == 0); } else if (S_ISGITLINK(a->mode)) { - result.clean = 0; - hashcpy(result.sha, a->sha1); + result.clean = merge_submodule(result.sha, one->path, one->sha1, + a->sha1, b->sha1); } else if (S_ISLNK(a->mode)) { hashcpy(result.sha, a->sha1); diff --git a/refs.c b/refs.c index f2de9f5..3882131 100644 --- a/refs.c +++ b/refs.c @@ -703,6 +703,12 @@ int for_each_ref(each_ref_fn fn, void *cb_data) return do_for_each_ref(NULL, "refs/", fn, 0, 0, cb_data); } +int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data) +{ + return do_for_each_ref(submodule, "refs/", fn, 0, + DO_FOR_EACH_INCLUDE_BROKEN, cb_data); +} + int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data) { return do_for_each_ref(NULL, prefix, fn, strlen(prefix), 0, cb_data); diff --git a/refs.h b/refs.h index 384e311..0889d8b 100644 --- a/refs.h +++ b/refs.h @@ -19,6 +19,7 @@ struct ref_lock { */ typedef int each_ref_fn(const char *refname, const unsigned char *sha1, int flags, void *cb_data); extern int head_ref(each_ref_fn, void *); +extern int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data); extern int for_each_ref(each_ref_fn, void *); extern int for_each_ref_in(const char *, each_ref_fn, void *); extern int for_each_tag_ref(each_ref_fn, void *); diff --git a/revision.c b/revision.c index b209d49..8955d7c 100644 --- a/revision.c +++ b/revision.c @@ -721,12 +721,15 @@ static void init_all_refs_cb(struct all_refs_cb *cb, struct rev_info *revs, cb->all_flags = flags; } -static void handle_refs(struct rev_info *revs, unsigned flags, +static void handle_refs(const char *submodule, struct rev_info *revs, unsigned flags, int (*for_each)(each_ref_fn, void *)) { struct all_refs_cb cb; init_all_refs_cb(&cb, revs, flags); - for_each(handle_one_ref, &cb); + if (!submodule) + for_each(handle_one_ref, &cb); + else + for_each_ref_submodule(submodule, handle_one_ref, &cb); } static void handle_one_reflog_commit(unsigned char *sha1, void *cb_data) @@ -1357,6 +1360,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s { int i, flags, left, seen_dashdash, read_from_stdin, got_rev_arg = 0; const char **prune_data = NULL; + const char *submodule = NULL; + + if (opt) + submodule = opt->submodule; /* First, search for "--" */ seen_dashdash = 0; @@ -1381,26 +1388,30 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s int opts; if (!strcmp(arg, "--all")) { - handle_refs(revs, flags, for_each_ref); - handle_refs(revs, flags, head_ref); + if (submodule) { + handle_refs(submodule, revs, flags, NULL); + } else { + handle_refs(NULL, revs, flags, for_each_ref); + handle_refs(NULL, revs, flags, head_ref); + } continue; } if (!strcmp(arg, "--branches")) { - handle_refs(revs, flags, for_each_branch_ref); + handle_refs(NULL, revs, flags, for_each_branch_ref); continue; } if (!strcmp(arg, "--bisect")) { - handle_refs(revs, flags, for_each_bad_bisect_ref); - handle_refs(revs, flags ^ UNINTERESTING, for_each_good_bisect_ref); + handle_refs(NULL, revs, flags, for_each_bad_bisect_ref); + handle_refs(NULL, revs, flags ^ UNINTERESTING, for_each_good_bisect_ref); revs->bisect = 1; continue; } if (!strcmp(arg, "--tags")) { - handle_refs(revs, flags, for_each_tag_ref); + handle_refs(NULL, revs, flags, for_each_tag_ref); continue; } if (!strcmp(arg, "--remotes")) { - handle_refs(revs, flags, for_each_remote_ref); + handle_refs(NULL, revs, flags, for_each_remote_ref); continue; } if (!prefixcmp(arg, "--glob=")) { diff --git a/revision.h b/revision.h index 568f1c9..0fe4322 100644 --- a/revision.h +++ b/revision.h @@ -145,6 +145,7 @@ extern volatile show_early_output_fn_t show_early_output; struct setup_revision_opt { const char *def; void (*tweak)(struct rev_info *, struct setup_revision_opt *); + const char *submodule; }; extern void init_revisions(struct rev_info *revs, const char *prefix); diff --git a/submodule.c b/submodule.c index abd5fd5..ac76791 100644 --- a/submodule.c +++ b/submodule.c @@ -6,6 +6,7 @@ #include "revision.h" #include "run-command.h" #include "diffcore.h" +#include "refs.h" static int add_submodule_odb(const char *path) { @@ -242,3 +243,157 @@ int checkout_submodule(const char *path, const unsigned char sha1[20], int force return 0; } + +static int find_first_merges(struct object_array *result, const char *path, + struct commit *a, struct commit *b) +{ + int i, j; + struct object_array merges; + struct commit *commit; + int contains_another; + + char merged_revision[42]; + const char *rev_args[] = { "rev-list", "--merges", "--all", merged_revision, NULL }; + struct rev_info revs; + struct setup_revision_opt rev_opts; + + memset(&merges, 0, sizeof(merges)); + memset(result, 0, sizeof(struct object_array)); + memset(&rev_opts, 0, sizeof(rev_opts)); + + /* get all revisions that merge commit a */ + snprintf(merged_revision, sizeof(merged_revision), "^%s", + find_unique_abbrev(a->object.sha1, 40)); + init_revisions(&revs, NULL); + rev_opts.submodule = path; + setup_revisions(sizeof(rev_args)/sizeof(char *)-1, rev_args, &revs, &rev_opts); + + /* save all revisions from the above list that contain b */ + if (prepare_revision_walk(&revs)) + die("revision walk setup failed"); + while ((commit = get_revision(&revs)) != NULL) { + struct object *o = &(commit->object); + if (in_merge_bases(b, (struct commit **) &o, 1)) { + add_object_array(o, NULL, &merges); + } + } + + /* Now we've got all merges that contain a and b. Prune all + * merges that contain another found merge and save them in + * result. */ + for (i = 0; i < merges.nr; i++) { + struct commit *m1 = (struct commit *) merges.objects[i].item; + + contains_another = 0; + for (j = 0; j < merges.nr; j++) { + struct commit *m2 = (struct commit *) merges.objects[j].item; + if (i != j && in_merge_bases(m2, &m1, 1)) { + contains_another = 1; + break; + } + } + + if (!contains_another) + add_object_array(merges.objects[i].item, + merges.objects[i].name, result); + } + + free(merges.objects); + return result->nr; +} + +static void print_commit(struct commit *commit) +{ + static const char *format = " %h: %m %s"; + struct strbuf sb = STRBUF_INIT; + struct pretty_print_context ctx = {0}; + ctx.date_mode = DATE_NORMAL; + format_commit_message(commit, format, &sb, &ctx); + strbuf_addstr(&sb, "\n"); + fprintf(stderr, "%s", sb.buf); +} + +int merge_submodule(unsigned char result[20], const char *path, const unsigned char base[20], + const unsigned char a[20], const unsigned char b[20]) +{ + struct commit *commit_base, *commit_a, *commit_b; + int parent_count; + struct object_array merges; + + int i; + + /* store a in result in case we fail */ + hashcpy(result, a); + + /* we can not handle deletion conflicts */ + if (is_null_sha1(base)) + return 0; + if (is_null_sha1(a)) + return 0; + if (is_null_sha1(b)) + return 0; + + if (add_submodule_odb(path)) { + warning("Failed to merge submodule %s (not checked out)", path); + return 0; + } + + if (!(commit_base = lookup_commit_reference(base)) || + !(commit_a = lookup_commit_reference(a)) || + !(commit_b = lookup_commit_reference(b))) + { + warning("Failed to merge submodule %s (commits not present)", path); + return 0; + } + + /* 1. case a is contained in b or vice versa */ + if (in_merge_bases(commit_a, &commit_b, 1)) { + hashcpy(result, b); + return 1; + } + if (in_merge_bases(commit_b, &commit_a, 1)) { + hashcpy(result, a); + return 1; + } + + /* 2. case there are one ore more merges that contain a and b the + * submodule. If there is a single one check it out but leave it + * marked unmerged so the user needs to confirm the resolution */ + + /* are both changes forward */ + if (!in_merge_bases(commit_base, &commit_a, 1) || + !in_merge_bases(commit_base, &commit_b, 1)) + { + warning("Submodule rewound can not merge"); + return 0; + } + + /* find commit which merges them */ + parent_count = find_first_merges(&merges, path, commit_a, commit_b); + if (!parent_count) { + warning("Failed to merge submodule %s (merge not found)", path); + goto finish; + } + + if (parent_count != 1) { + warning("Failed to merge submodule %s (multiple merges found):", path); + for (i = 0; i < merges.nr; i++) { + print_commit((struct commit *) merges.objects[i].item); + } + goto finish; + } + + warning("Failed to merge submodule %s (not fast-forward):\n", path); + fprintf(stderr, "Found a possible merge resolution for the submodule:\n"); + print_commit((struct commit *) merges.objects[0].item); + fprintf(stderr, "If this is correct simply add it to the index for example\n" + "by using:\n\n" + " git add %s\n\n" + "which will accept this suggestion.\n", path); + + checkout_submodule(path, merges.objects[0].item->sha1, 0); + +finish: + free(merges.objects); + return 0; +} diff --git a/submodule.h b/submodule.h index fc6909e..12d6d73 100644 --- a/submodule.h +++ b/submodule.h @@ -7,5 +7,7 @@ void show_submodule_summary(FILE *f, const char *path, const char *del, const char *add, const char *reset); unsigned is_submodule_modified(const char *path, int ignore_untracked); int checkout_submodule(const char *path, const unsigned char sha1[20], int force); +int merge_submodule(unsigned char result[20], const char *path, const unsigned char base[20], + const unsigned char a[20], const unsigned char b[20]); #endif diff --git a/t/t7405-submodule-merge.sh b/t/t7405-submodule-merge.sh index 4a7b893..04dc371 100755 --- a/t/t7405-submodule-merge.sh +++ b/t/t7405-submodule-merge.sh @@ -54,13 +54,116 @@ test_expect_success setup ' git merge -s ours a ' -test_expect_success 'merging with modify/modify conflict' ' +# History setup +# +# b +# / \ +# a d +# \ / +# c +# +# a in the main repository records to sub-a in the submodule and +# analogous b and c. d should be automatically found by merging c into +# b in the main repository. +test_expect_success 'setup for merge search' ' + mkdir merge-search && + cd merge-search && + git init && + mkdir sub && + (cd sub && + git init && + echo "file-a" > file-a && + git add file-a && + git commit -m "sub-a" && + git checkout -b sub-a) && + git add sub && + git commit -m "a" && + git checkout -b a && + + git checkout -b b && + (cd sub && + git checkout -b sub-b && + echo "file-b" > file-b && + git add file-b && + git commit -m "sub-b") && + git commit -a -m "b" && + + git checkout -b c a && + (cd sub && + git checkout -b sub-c sub-a && + echo "file-c" > file-c && + git add file-c && + git commit -m "sub-c") && + git commit -a -m "c" && + + (cd sub && + git checkout -b sub-d sub-b && + git merge sub-c && + git checkout sub-b) && + git checkout -b test b && + cd .. +' + +test_expect_success 'merging with common parent search' ' + cd merge-search && + git checkout -b test-parent b && + git merge c && + git ls-tree test-parent | grep sub | cut -f1 | cut -f3 -d" " > actual && + (cd sub && + git rev-parse sub-d > ../expect) && + test_cmp actual expect && + cd .. +' + +test_expect_success 'merging should fail for ambigous common parent' ' + cd merge-search && + git checkout -b test-ambigous b && + (cd sub && + git checkout -b ambigous sub-d && + echo "ambigous-file" > ambigous-file && + git add ambigous-file && + git commit -m "ambigous") && + test_must_fail git merge c && + git reset --hard && + cd .. +' + +# in a situation like this +# +# submodule tree: +# +# sub-a --- sub-b --- sub-d +# +# main tree: +# +# e (sub-a) +# / +# d (sub-b) +# \ +# f (sub-d) +# +# A merge should fail because one change points backwards. + +test_expect_success 'merging should fail for changes that are backwards' ' + cd merge-search && + git checkout -b d a && + (cd sub && + git checkout sub-b) && + git commit -a -m "d" && + + git checkout -b e d && + (cd sub && + git checkout sub-a) && + git commit -a -m "e" && + + git checkout -b f d && + (cd sub && + git checkout sub-d) && + git commit -a -m "f" && - git checkout -b test1 a && - test_must_fail git merge b && - test -f .git/MERGE_MSG && - git diff && - test -n "$(git ls-files -u)" + git checkout -b test-backward e && + test_must_fail git merge f && + cd .. ' test_expect_success 'merging with a modify/modify conflict between merge bases' ' -- 1.7.1.465.g3692.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html