Better errors when trying to merge a submodule

Instead of dying with weird errors when trying to merge submodules from a supermodule, emit errors that show what the problem is.

---

If you try to merge a submodule from a supermodule, you get some very strange error messages. With this patch you get a nice clean error message indicating that this isn't supported instead.

 git-merge-one-file.sh |    7 +++++++
 merge-recursive.c     |    2 ++
 2 files changed, 9 insertions(+), 0 deletions(-)

index 1e7727d..7aee342 100755
@@ -82,6 +82,13 @@ case "${1:-.}${2:-.}${3:-.}" in
 		;;
 	esac
 
+	case ",$6,$7," in
+	*,160000,*)
+		echo "ERROR: $4: Not merging submodule."
+		exit 1
+		;;
+	esac
+
 	src2=`git-unpack-file $3`
 	case "$1" in
 	'')

index 9a1e2f2..ecae8ea 100644
@@ -1046,6 +1046,8 @@ static struct merge_file_info merge_file(struct diff_filespec *o,
 
 			free(result_buf.ptr);
 			result.clean = (merge_status == 0);
+                } else if (S_ISGITLINK(a->mode) || S_ISGITLINK(b->mode)) {
+                        die("cannot merge submodules!");
 		} else {
 			if (!(S_ISLNK(a->mode) || S_ISLNK(b->mode)))
 				die("cannot merge modes?");

1.5.3.7.1149.g591a-dirty

Thanks.

Your change to merge-one-file.sh is Ok, although I'd reword the message a bit, and fold it as a new case arm to the existing case statement immediately above.

Your change to merge-recursive is not quite right, although you spotted correctly what codepath needs to be fixed. merge_file() should not die in such a case but set result.clean to 0 to signal that the result has conflicts and cannot be merged, pick the sha1 from the current tree (i.e. side A), and let the caller deal with the conflict. If you die there, the user cannot resolve a merge if this happens while building a virtual ancestor commit during a recursive merge of two crisscrossing histories.

Perhaps something like this...

-- >8 --
Support a merge with conflicting gitlink change

merge-recursive did not support merging trees that have conflicting changes in submodules they contain, and died. Support it exactly the same way as how it handles conflicting symbolic link changes --- mark it as a conflict, take the tentative result from the current side, and letting the caller resolve the conflict, without dying in merge_file() function.

Also reword the error message issued when merge_file() has to die because it sees a tree entry of type it does not support yet.

---

 git-merge-one-file.sh |    4 ++++
 merge-recursive.c     |   10 ++++++----
 2 files changed, 10 insertions(+), 4 deletions(-)

index 1e7727d..9ee3f80 100755
@@ -80,6 +80,10 @@ case "${1:-.}${2:-.}${3:-.}" in
 		echo "ERROR: $4: Not merging symbolic link changes."
 		exit 1
 		;;
+	*,160000,*)
+		echo "ERROR: $4: Not merging conflicting submodule changes."
+		exit 1
+		;;
 	esac
 
 	src2=`git-unpack-file $3`

index 9a1e2f2..2a58dad 100644
@@ -1046,14 +1046,16 @@ static struct merge_file_info merge_file(struct diff_filespec *o,
 
 			free(result_buf.ptr);
 			result.clean = (merge_status == 0);
-		} else {
-			if (!(S_ISLNK(a->mode) || S_ISLNK(b->mode)))
-				die("cannot merge modes?");
-
+		} else if (S_ISGITLINK(a->mode)) {
+			result.clean = 0;
+			hashcpy(result.sha, a->sha1);
+		} else if (S_ISLNK(a->mode)) {
 			hashcpy(result.sha, a->sha1);
 
 			if (!sha_eq(a->sha1, b->sha1))
 				result.clean = 0;
+		} else {
+			die("unsupported object type in the tree");
 		}
 	}
 

Your patch is obviously much nicer than the one I sent in, so please put it in if/when convenient!

On another note, has there been any though to get merge to support sub-module merging properly? It seems like it should be possible (and it would make submodules a lot more useful)

- Finn Arne