{"thread":{"id":"11225","subject":"[PATCH] Better errors when trying to merge a submodule","startedAt":"2007-12-10T12:44:35Z","lastAt":"2007-12-11T18:11:10Z","messageCount":3,"participants":["Finn Arne Gangstad","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"62568","messageId":"20071210124435.GA4788@pvv.org","threadId":"11225","inReplyTo":null,"subject":"[PATCH] Better errors when trying to merge a submodule","fromName":"Finn Arne Gangstad","fromEmail":"finnag@pvv.org","sentAt":"2007-12-10T12:44:35Z","receivedAt":"2007-12-10T12:44:35Z","isPatch":true,"sender":{"key":"finnag@pvv.org","avatar":"https://gravatar.com/avatar/b421ddd58c3f0f93aa473e17b98bb8d53c221fef741746bc8cb59fae4ec6d95e?d=mp&s=160"},"body":"\nInstead of dying with weird errors when trying to merge submodules from a\nsupermodule, emit errors that show what the problem is.\n\nSigned-off-by: Finn Arne Gangstad <finnag@pvv.org>\n---\n\nIf you try to merge a submodule from a supermodule, you get some very\nstrange error messages. With this patch you get a nice clean error\nmessage indicating that this isn't supported instead.\n\n\n git-merge-one-file.sh |    7 +++++++\n merge-recursive.c     |    2 ++\n 2 files changed, 9 insertions(+), 0 deletions(-)\n\ndiff --git a/git-merge-one-file.sh b/git-merge-one-file.sh\nindex 1e7727d..7aee342 100755\n--- a/git-merge-one-file.sh\n+++ b/git-merge-one-file.sh\n@@ -82,6 +82,13 @@ case \"${1:-.}${2:-.}${3:-.}\" in\n \t\t;;\n \tesac\n \n+\tcase \",$6,$7,\" in\n+\t*,160000,*)\n+\t\techo \"ERROR: $4: Not merging submodule.\"\n+\t\texit 1\n+\t\t;;\n+\tesac\n+\n \tsrc2=`git-unpack-file $3`\n \tcase \"$1\" in\n \t'')\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 9a1e2f2..ecae8ea 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1046,6 +1046,8 @@ static struct merge_file_info merge_file(struct diff_filespec *o,\n \n \t\t\tfree(result_buf.ptr);\n \t\t\tresult.clean = (merge_status == 0);\n+                } else if (S_ISGITLINK(a->mode) || S_ISGITLINK(b->mode)) {\n+                        die(\"cannot merge submodules!\");\n \t\t} else {\n \t\t\tif (!(S_ISLNK(a->mode) || S_ISLNK(b->mode)))\n \t\t\t\tdie(\"cannot merge modes?\");\n-- \n1.5.3.7.1149.g591a-dirty\n"},{"id":"62607","messageId":"7vsl2al5ia.fsf@gitster.siamese.dyndns.org","threadId":"11225","inReplyTo":"20071210124435.GA4788@pvv.org","subject":"Re: [PATCH] Better errors when trying to merge a submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-10T19:22:05Z","receivedAt":"2007-12-10T19:22:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Finn Arne Gangstad <finnag@pvv.org> writes:\n\n> Instead of dying with weird errors when trying to merge submodules from a\n> supermodule, emit errors that show what the problem is.\n\nThanks.\n\nYour change to merge-one-file.sh is Ok, although I'd reword the message\na bit, and fold it as a new case arm to the existing case statement\nimmediately above.\n\nYour change to merge-recursive is not quite right, although you spotted\ncorrectly what codepath needs to be fixed.  merge_file() should not die\nin such a case but set result.clean to 0 to signal that the result has\nconflicts and cannot be merged, pick the sha1 from the current tree\n(i.e. side A), and let the caller deal with the conflict.  If you die\nthere, the user cannot resolve a merge if this happens while building a\nvirtual ancestor commit during a recursive merge of two crisscrossing\nhistories.\n\nPerhaps something like this...\n\n-- >8 --\nSupport a merge with conflicting gitlink change\n\nmerge-recursive did not support merging trees that have conflicting\nchanges in submodules they contain, and died.  Support it exactly the\nsame way as how it handles conflicting symbolic link changes --- mark it\nas a conflict, take the tentative result from the current side, and\nletting the caller resolve the conflict, without dying in merge_file()\nfunction.\n\nAlso reword the error message issued when merge_file() has to die\nbecause it sees a tree entry of type it does not support yet.\n\n---\n\n git-merge-one-file.sh |    4 ++++\n merge-recursive.c     |   10 ++++++----\n 2 files changed, 10 insertions(+), 4 deletions(-)\n\ndiff --git a/git-merge-one-file.sh b/git-merge-one-file.sh\nindex 1e7727d..9ee3f80 100755\n--- a/git-merge-one-file.sh\n+++ b/git-merge-one-file.sh\n@@ -80,6 +80,10 @@ case \"${1:-.}${2:-.}${3:-.}\" in\n \t\techo \"ERROR: $4: Not merging symbolic link changes.\"\n \t\texit 1\n \t\t;;\n+\t*,160000,*)\n+\t\techo \"ERROR: $4: Not merging conflicting submodule changes.\"\n+\t\texit 1\n+\t\t;;\n \tesac\n \n \tsrc2=`git-unpack-file $3`\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 9a1e2f2..2a58dad 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1046,14 +1046,16 @@ static struct merge_file_info merge_file(struct diff_filespec *o,\n \n \t\t\tfree(result_buf.ptr);\n \t\t\tresult.clean = (merge_status == 0);\n-\t\t} else {\n-\t\t\tif (!(S_ISLNK(a->mode) || S_ISLNK(b->mode)))\n-\t\t\t\tdie(\"cannot merge modes?\");\n-\n+\t\t} else if (S_ISGITLINK(a->mode)) {\n+\t\t\tresult.clean = 0;\n+\t\t\thashcpy(result.sha, a->sha1);\n+\t\t} else if (S_ISLNK(a->mode)) {\n \t\t\thashcpy(result.sha, a->sha1);\n \n \t\t\tif (!sha_eq(a->sha1, b->sha1))\n \t\t\t\tresult.clean = 0;\n+\t\t} else {\n+\t\t\tdie(\"unsupported object type in the tree\");\n \t\t}\n \t}\n \n"},{"id":"62758","messageId":"20071211181110.GA16491@pvv.org","threadId":"11225","inReplyTo":"7vsl2al5ia.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Better errors when trying to merge a submodule","fromName":"Finn Arne Gangstad","fromEmail":"finnag@pvv.org","sentAt":"2007-12-11T18:11:10Z","receivedAt":"2007-12-11T18:11:10Z","isPatch":true,"sender":{"key":"finnag@pvv.org","avatar":"https://gravatar.com/avatar/b421ddd58c3f0f93aa473e17b98bb8d53c221fef741746bc8cb59fae4ec6d95e?d=mp&s=160"},"body":"On Mon, Dec 10, 2007 at 11:22:05AM -0800, Junio C Hamano wrote:\n> Finn Arne Gangstad <finnag@pvv.org> writes:\n> \n> > Instead of dying with weird errors when trying to merge submodules from a\n> > supermodule, emit errors that show what the problem is.\n> \n> Thanks.\n> \n> Your change to merge-one-file.sh is Ok, although I'd reword the message\n> a bit, and fold it as a new case arm to the existing case statement\n> immediately above.\n> [...]\n> merge-recursive did not support merging trees that have conflicting\n> changes in submodules they contain, and died.  Support it exactly the\n> same way as how it handles conflicting symbolic link changes --- mark it\n> as a conflict, take the tentative result from the current side, and\n> letting the caller resolve the conflict, without dying in merge_file()\n> function.\n> [...]\n\nYour patch is obviously much nicer than the one I sent in, so please\nput it in if/when convenient! \n\nOn another note, has there been any though to get merge to support\nsub-module merging properly? It seems like it should be possible (and\nit would make submodules a lot more useful)\n\n- Finn Arne\n"}]}