Hi,
Johan and Junio thank you very much for the review. Here is a new
iteration with your comments incorporated.
Further below you can find the changes between the last iteration as
interdiff.
Heiko Voigt (4):
add missing && to submodule-merge testcase
teach ref iteration module about submodules
setup_revisions(): allow walking history in a submodule
implement automatic fast-forward merge for submodules
cache.h | 3 +
merge-recursive.c | 9 ++-
path.c | 38 ++++++++++
refs.c | 149 +++++++++++++++++++++++++++++++---------
refs.h | 8 ++
revision.c | 32 +++++----
revision.h | 1 +
submodule.c | 162 ++++++++++++++++++++++++++++++++++++++++++++
submodule.h | 2 +
t/t7405-submodule-merge.sh | 130 +++++++++++++++++++++++++++++++++--
10 files changed, 478 insertions(+), 56 deletions(-)
--
1.7.2.rc1.13.g513fa.dirty
diff --git a/submodule.c b/submodule.c
index 62dcabe..9bc4b80 100644
--- a/submodule.c
+++ b/submodule.c
@@ -240,7 +240,7 @@ static int find_first_merges(struct object_array *result, const char *path,
/* get all revisions that merge commit a */
snprintf(merged_revision, sizeof(merged_revision), "^%s",
- find_unique_abbrev(a->object.sha1, 40));
+ sha1_to_hex(a->object.sha1));
init_revisions(&revs, NULL);
rev_opts.submodule = path;
setup_revisions(sizeof(rev_args)/sizeof(char *)-1, rev_args, &revs, &rev_opts);
@@ -250,14 +250,15 @@ static int find_first_merges(struct object_array *result, const char *path,
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)) {
+ if (in_merge_bases(b, &commit, 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. */
+ * result.
+ */
for (i = 0; i < merges.nr; i++) {
struct commit *m1 = (struct commit *) merges.objects[i].item;
@@ -281,17 +282,19 @@ static int find_first_merges(struct object_array *result, const char *path,
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);
+ format_commit_message(commit, " %h: %m %s", &sb, &ctx);
+ fprintf(stderr, "%s\n", 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])
+#define MERGE_WARNING(path, msg) \
+ warning("Failed to merge submodule %s (%s)", path, msg);
+
+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;
@@ -311,7 +314,7 @@ int merge_submodule(unsigned char result[20], const char *path, const unsigned c
return 0;
if (add_submodule_odb(path)) {
- warning("Failed to merge submodule %s (not checked out)", path);
+ MERGE_WARNING(path, "not checked out");
return 0;
}
@@ -319,7 +322,7 @@ int merge_submodule(unsigned char result[20], const char *path, const unsigned c
!(commit_a = lookup_commit_reference(a)) ||
!(commit_b = lookup_commit_reference(b)))
{
- warning("Failed to merge submodule %s (commits not present)", path);
+ MERGE_WARNING(path, "commits not present");
return 0;
}
@@ -327,11 +330,11 @@ int merge_submodule(unsigned char result[20], const char *path, const unsigned c
if (!in_merge_bases(commit_base, &commit_a, 1) ||
!in_merge_bases(commit_base, &commit_b, 1))
{
- warning("Submodule rewound can not merge");
+ MERGE_WARNING(path, "commits don't follow merge-base");
return 0;
}
- /* 1. case a is contained in b or vice versa */
+ /* Case #1: a is contained in b or vice versa */
if (in_merge_bases(commit_a, &commit_b, 1)) {
hashcpy(result, b);
return 1;
@@ -341,35 +344,39 @@ int merge_submodule(unsigned char result[20], const char *path, const unsigned c
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 */
+ /* Case #2: There are one or more merges that contain a and b in
+ * the submodule. If there is only one, then present it as a
+ * suggestion to the user, but leave it marked unmerged so the
+ * user needs to confirm the resolution.
+ */
/* 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;
- }
+ switch (parent_count) {
+ case 0:
+ MERGE_WARNING(path, "merge following commits not found");
+ break;
- 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;
- }
+ case 1:
+ MERGE_WARNING(path,"not fast-forward");
+ 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 update-index --cacheinfo 160000 %s \"%s\"\n\n"
+ "which will accept this suggestion.\n",
+ sha1_to_hex(merges.objects[0].item->sha1), path);
+ break;
- 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 update-index --cacheinfo 160000 %s \"%s\"\n\n"
- "which will accept this suggestion.\n",
- sha1_to_hex(merges.objects[0].item->sha1), path);
+ default:
+ MERGE_WARNING(path, "multiple merges found");
+ for (i = 0; i < merges.nr; i++)
+ print_commit((struct commit *) merges.objects[i].item);
+ }
-finish:
free(merges.objects);
return 0;
}
diff --git a/t/t7405-submodule-merge.sh b/t/t7405-submodule-merge.sh
index 0f568ab..6ec559d 100755
--- a/t/t7405-submodule-merge.sh
+++ b/t/t7405-submodule-merge.sh
@@ -75,10 +75,10 @@ test_expect_success 'setup for merge search' '
echo "file-a" > file-a &&
git add file-a &&
git commit -m "sub-a" &&
- git checkout -b sub-a) &&
+ git branch sub-a) &&
git add sub &&
git commit -m "a" &&
- git checkout -b a &&
+ git branch a &&
git checkout -b b &&
(cd sub &&
@@ -101,7 +101,7 @@ test_expect_success 'setup for merge search' '
git checkout -b sub-d sub-b &&
git merge sub-c) &&
git commit -a -m "d" &&
- git checkout -b test b &&
+ git branch test b &&
cd ..
'
@@ -109,7 +109,7 @@ test_expect_success 'merge with one side as a fast-forward of the other' '
(cd merge-search &&
git checkout -b test-forward b &&
git merge d &&
- git ls-tree test-forward | grep sub | cut -f1 | cut -f3 -d" " > actual &&
+ git ls-tree test-forward sub | cut -f1 | cut -f3 -d" " > actual &&
(cd sub &&
git rev-parse sub-d > ../expect) &&
test_cmp actual expect)
@@ -121,21 +121,21 @@ test_expect_success 'merging should conflict for non fast-forward' '
(cd sub &&
git rev-parse sub-d > ../expect) &&
test_must_fail git merge c 2> actual &&
- grep $(<expect) actual > /dev/null &&
+ grep $(cat expect) actual > /dev/null &&
git reset --hard)
'
-test_expect_success 'merging should fail for ambigous common parent' '
+test_expect_success 'merging should fail for ambiguous common parent' '
cd merge-search &&
- git checkout -b test-ambigous b &&
+ git checkout -b test-ambiguous b &&
(cd sub &&
- git checkout -b ambigous sub-b &&
+ git checkout -b ambiguous sub-b &&
git merge sub-c &&
git rev-parse sub-d > ../expect1 &&
- git rev-parse ambigous > ../expect2) &&
+ git rev-parse ambiguous > ../expect2) &&
test_must_fail git merge c 2> actual &&
- grep $(<expect1) actual > /dev/null &&
- grep $(<expect2) actual > /dev/null &&
+ grep $(cat expect1) actual > /dev/null &&
+ grep $(cat expect2) actual > /dev/null &&
git reset --hard &&
cd ..
'
@@ -154,8 +154,9 @@ test_expect_success 'merging should fail for ambigous common parent' '
# \
# f (sub-d)
#
-# A merge should fail because one change points backwards.
-
+# A merge between e and f should fail because one of the submodule
+# commits (sub-a) does not descend from the submodule merge-base (sub-b).
+#
test_expect_success 'merging should fail for changes that are backwards' '
cd merge-search &&
git checkout -b bb a &&
--
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