Re: [PATCH] submodule: Demonstrate known breakage during recursive merge
From: Heiko Voigt <hidden>
Date: 2016-06-15 22:51:54
Subsystem:
the rest · Maintainer:
Linus Torvalds
Hi, thanks for finding this subtle bug! On Wed, Aug 24, 2011 at 09:59:50AM -0400, Brad King wrote:
Since commit 68d03e4a (Implement automatic fast-forward merge for
submodules, 2010-07-07) we try to suggest submodule commits that resolve
a conflict. Consider a true recursive merge case
b---bc
/ \ /
o X
\ / \
c---cbAnd here is a patch[1] that you can apply on top of yours which should fix this. An extra pair of merge machinery knowing eyes appreciated. Its a little bit workaroundish so if anymore has an idea how to fix this in nicer way, please tell me. [1]--8<---- From: Heiko Voigt <redacted> Subject: [PATCH] protect submodule merge search against multiple calls for the same path When multiple merge-bases are found for two commits to be merged the merge machinery will ask twice for a merge resolution. Currently its not possible to use the revision-walking api for walking the same commits multiple times. Since the result will not change we can simply fail here if we are asked for a resolution of the same path again. Signed-off-by: Heiko Voigt <redacted> --- submodule.c | 9 +++++++++ t/t7405-submodule-merge.sh | 2 +- 2 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/submodule.c b/submodule.c
index 1ba9646..a4af08e 100644
--- a/submodule.c
+++ b/submodule.c@@ -646,6 +646,7 @@ 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]) { + static char last_path[PATH_MAX] = {'\0'}; struct commit *commit_base, *commit_a, *commit_b; int parent_count; struct object_array merges;
@@ -699,6 +700,13 @@ int merge_submodule(unsigned char result[20], const char *path, * user needs to confirm the resolution. */ + /* in case of multiple merge-bases the merge algorithm will ask + * again for a resolution. We should not search twice for the + * same path. + */ + if (!strcmp(path, last_path)) + return 0; + /* find commit which merges them */ parent_count = find_first_merges(&merges, path, commit_a, commit_b); switch (parent_count) {
@@ -726,6 +734,7 @@ int merge_submodule(unsigned char result[20], const char *path, print_commit((struct commit *) merges.objects[i].item); } + memcpy(last_path, path, strlen(path) + 1); free(merges.objects); return 0; }
diff --git a/t/t7405-submodule-merge.sh b/t/t7405-submodule-merge.sh
index 8f6f2d6..603fb72 100755
--- a/t/t7405-submodule-merge.sh
+++ b/t/t7405-submodule-merge.sh@@ -269,7 +269,7 @@ test_expect_success 'setup for recursive merge with submodule' ' ' # merge should leave submodule unmerged in index -test_expect_failure 'recursive merge with submodule' ' +test_expect_success 'recursive merge with submodule' ' (cd merge-recursive && test_must_fail git merge top-bc && echo "160000 $(git rev-parse top-cb:sub) 2 sub" > expect2 &&
--
1.7.6.551.g4266ca