{"thread":{"id":"8004","subject":"[PATCH] submodule merge support","startedAt":"2007-05-06T19:02:24Z","lastAt":"2007-05-07T19:33:14Z","messageCount":10,"participants":["Martin Waitz","Alex Riesen","Linus Torvalds","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"41237","messageId":"20070506190224.GG30511@admingilde.org","threadId":"8004","inReplyTo":null,"subject":"[PATCH] submodule merge support","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2007-05-06T19:02:24Z","receivedAt":"2007-05-06T19:02:24Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"When merge-recursive gets to a dirlink, it starts an automatic submodule merge\nand then uses the resulting merge commit for the top-level tree.\nThe submodule merge is done in another process to decouple object databases.\n\nSigned-off-by: Martin Waitz <tali@admingilde.org>\n---\n .gitignore           |    1 +\n Makefile             |    2 +-\n git-dirlink-merge.sh |   28 ++++++++++++++++++++++++++++\n merge-recursive.c    |   36 ++++++++++++++++++++++++++++++++++++\n 4 files changed, 66 insertions(+), 1 deletions(-)\n create mode 100644 git-dirlink-merge.sh\n\ndiff --git a/.gitignore b/.gitignore\nindex 8436a83..b076e2f 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -38,6 +38,7 @@ git-diff-files\n git-diff-index\n git-diff-tree\n git-dirlink-checkout\n+git-dirlink-merge\n git-describe\n git-fast-import\n git-fetch\ndiff --git a/Makefile b/Makefile\nindex fcd0125..ff24477 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -196,7 +196,7 @@ SCRIPT_SH = \\\n \tgit-merge-one-file.sh git-mergetool.sh git-parse-remote.sh \\\n \tgit-pull.sh git-rebase.sh \\\n \tgit-repack.sh git-request-pull.sh git-reset.sh \\\n-\tgit-sh-setup.sh git-dirlink-checkout.sh \\\n+\tgit-sh-setup.sh git-dirlink-checkout.sh git-dirlink-merge.sh \\\n \tgit-tag.sh git-verify-tag.sh \\\n \tgit-applymbox.sh git-applypatch.sh git-am.sh \\\n \tgit-merge.sh git-merge-stupid.sh git-merge-octopus.sh \\\ndiff --git a/git-dirlink-merge.sh b/git-dirlink-merge.sh\nnew file mode 100644\nindex 0000000..e719b1a\n--- /dev/null\n+++ b/git-dirlink-merge.sh\n@@ -0,0 +1,28 @@\n+#!/bin/sh -e\n+# Merge a submodule\n+# (c) 2006 Martin Waitz\n+\n+USAGE=\"submodule orig-sha1 a-sha1 b-sha1\"\n+\n+unset GIT_DIR\n+cd \"$1\"\n+\n+. git-sh-setup\n+\n+test $# -eq 4 || usage\n+\n+orig=\"$2\"\n+ours=\"$3\"\n+theirs=\"$4\"\n+\n+base=`git-merge-base \"$ours\" \"$theirs\"`\n+\n+if test `git-merge-base \"$orig\" \"$base\"` != \"$orig\"; then\n+\tdie \"$1 cannot be merged: other side switched branches\"\n+fi\n+\n+if test `git-rev-parse --verify HEAD` != \"$ours\"; then\n+\tdie \"$1: HEAD != ours\"\n+fi\n+\n+exec git-merge $theirs\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 8f72b2c..4b67cd0 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -11,6 +11,7 @@\n #include \"diff.h\"\n #include \"diffcore.h\"\n #include \"run-command.h\"\n+#include \"refs.h\"\n #include \"tag.h\"\n #include \"unpack-trees.h\"\n #include \"path-list.h\"\n@@ -574,6 +575,21 @@ static void update_file_flags(const unsigned char *sha,\n \t\tvoid *buf;\n \t\tunsigned long size;\n \n+\t\tif (S_ISDIRLNK(mode)) {\n+\t\t\t/* defer dirlinks to another process, don't try to */\n+\t\t\t/* read the object \"sha\" here */\n+\t\t\tconst char *dirlink_checkout[] = {\n+\t\t\t\t\"dirlink-checkout\", path, sha1_to_hex(sha), NULL\n+\t\t\t};\n+\t\t\tstruct child_process cmd = {\n+\t\t\t\t.argv = dirlink_checkout,\n+\t\t\t\t.git_cmd = 1,\n+\t\t\t};\n+\n+\t\t\trun_command(&cmd);\n+\t\t\tgoto update_index;\n+\t\t}\n+\n \t\tbuf = read_sha1_file(sha, &type, &size);\n \t\tif (!buf)\n \t\t\tdie(\"cannot read object %s '%s'\", sha1_to_hex(sha), path);\n@@ -1069,6 +1085,26 @@ 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 if (S_ISDIRLNK(a->mode)) {\n+\t\t\tconst char *dirlink_merge[] = {\n+\t\t\t\t\"dirlink-merge\", a->path,\n+\t\t\t\tsha1_to_hex(o->sha1),\n+\t\t\t\tsha1_to_hex(a->sha1),\n+\t\t\t\tsha1_to_hex(b->sha1),\n+\t\t\t\tNULL\n+\t\t\t};\n+\t\t\tstruct child_process cmd = {\n+\t\t\t\t.argv = dirlink_merge,\n+\t\t\t\t.git_cmd = 1,\n+\t\t\t};\n+\t\t\t/* recurse into the submodule in a different process */\n+\t\t\tresult.clean = !run_command(&cmd);\n+\t\t\tif (result.clean) {\n+\t\t\t\t/* get the new merged version */\n+\t\t\t\tif (resolve_gitlink_ref(a->path, \"HEAD\",\n+\t\t\t\t                        result.sha) < 0)\n+\t\t\t\t\tresult.clean = 0;\n+\t\t\t}\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.1.2.247.gaef5a\n\n\n-- \nMartin Waitz\n"},{"id":"41255","messageId":"20070506220745.GA2439@steel.home","threadId":"8004","inReplyTo":"20070506190224.GG30511@admingilde.org","subject":"Re: [PATCH] submodule merge support","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-05-06T22:07:45Z","receivedAt":"2007-05-06T22:07:45Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Martin Waitz, Sun, May 06, 2007 21:02:24 +0200:\n> When merge-recursive gets to a dirlink, it starts an automatic\n> submodule merge and then uses the resulting merge commit for the\n> top-level tree.\n\nmerge-recursive is a mess already, you just made even more so.\nBesides, you completely forgot all other merge strategies.\n\nHow about making all existing strategies just ignore submodules, and\nmove recursive merge in the merge driver (git-merge.sh)?\n"},{"id":"41256","messageId":"20070506221621.GH30511@admingilde.org","threadId":"8004","inReplyTo":"20070506220745.GA2439@steel.home","subject":"Re: [PATCH] submodule merge support","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2007-05-06T22:16:23Z","receivedAt":"2007-05-06T22:16:23Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nOn Mon, May 07, 2007 at 12:07:45AM +0200, Alex Riesen wrote:\n> Martin Waitz, Sun, May 06, 2007 21:02:24 +0200:\n> > When merge-recursive gets to a dirlink, it starts an automatic\n> > submodule merge and then uses the resulting merge commit for the\n> > top-level tree.\n> \n> merge-recursive is a mess already, you just made even more so.\n> Besides, you completely forgot all other merge strategies.\n> \n> How about making all existing strategies just ignore submodules, and\n> move recursive merge in the merge driver (git-merge.sh)?\n\nWell, I don't think it can be done in the merge driver but it does make\nsense to consolidate all the low-level file-based merging (and the new\nsubmodule merge is exactly that) between merge strategies.\n\n-- \nMartin Waitz\n"},{"id":"41257","messageId":"alpine.LFD.0.98.0705061517380.12945@woody.linux-foundation.org","threadId":"8004","inReplyTo":"20070506220745.GA2439@steel.home","subject":"Re: [PATCH] submodule merge support","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-05-06T22:18:53Z","receivedAt":"2007-05-06T22:18:53Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 7 May 2007, Alex Riesen wrote:\n\n> Martin Waitz, Sun, May 06, 2007 21:02:24 +0200:\n> > When merge-recursive gets to a dirlink, it starts an automatic\n> > submodule merge and then uses the resulting merge commit for the\n> > top-level tree.\n> \n> merge-recursive is a mess already, you just made even more so.\n> Besides, you completely forgot all other merge strategies.\n> \n> How about making all existing strategies just ignore submodules, and\n> move recursive merge in the merge driver (git-merge.sh)?\n\nYes, I think that's the right thing to do.\n\nI think it's the right thing for another reason: in a true \"recursive\" \nmerge, the submodules shouldn't be recursively merged anyway. *THEIR* \nmerge will have its own history, and doing it based on some random history \nof the superproject is actually wrong anyway!\n\n\t\tLinus\n"},{"id":"41310","messageId":"20070507090346.GI30511@admingilde.org","threadId":"8004","inReplyTo":"alpine.LFD.0.98.0705061517380.12945@woody.linux-foundation.org","subject":"Re: [PATCH] submodule merge support","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2007-05-07T09:03:46Z","receivedAt":"2007-05-07T09:03:46Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nOn Sun, May 06, 2007 at 03:18:53PM -0700, Linus Torvalds wrote:\n> On Mon, 7 May 2007, Alex Riesen wrote:\n> > merge-recursive is a mess already, you just made even more so.\n> > Besides, you completely forgot all other merge strategies.\n> > \n> > How about making all existing strategies just ignore submodules, and\n> > move recursive merge in the merge driver (git-merge.sh)?\n> \n> Yes, I think that's the right thing to do.\n> \n> I think it's the right thing for another reason: in a true \"recursive\" \n> merge, the submodules shouldn't be recursively merged anyway. *THEIR* \n> merge will have its own history, and doing it based on some random history \n> of the superproject is actually wrong anyway!\n\nOf course the submodule has to get its own history, it's not possible\nto do otherwise.  But you have to trigger the submodule merge when you\nfind a submodule-level conflict in the supermodule merge, just as\nyou trigger file-level three-way merges, too.\nThis is really all which my patch does -- it starts a new instance of\nthe merge driver which only merges the submodule commit from the other\nsupermodule tree.\nSo it's not recursive in the merge-recursive way, but in a\ncall-git-merge-again way.\n\nThe comment about merge-recursive not being the best place is correct of\ncourse.\n\n-- \nMartin Waitz\n"},{"id":"41314","messageId":"463EFFC6.12A1B0A1@eudaptics.com","threadId":"8004","inReplyTo":"20070507090346.GI30511@admingilde.org","subject":"Re: [PATCH] submodule merge support","fromName":"Johannes Sixt","fromEmail":"j.sixt@eudaptics.com","sentAt":"2007-05-07T10:30:30Z","receivedAt":"2007-05-07T10:30:30Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Martin Waitz wrote:\n> On Sun, May 06, 2007 at 03:18:53PM -0700, Linus Torvalds wrote:\n> > On Mon, 7 May 2007, Alex Riesen wrote:\n> > > How about making all existing strategies just ignore submodules, and\n> > > move recursive merge in the merge driver (git-merge.sh)?\n> >\n> > Yes, I think that's the right thing to do.\n> >\n> > I think it's the right thing for another reason: in a true \"recursive\"\n> > merge, the submodules shouldn't be recursively merged anyway. *THEIR*\n> > merge will have its own history, and doing it based on some random history\n> > of the superproject is actually wrong anyway!\n> \n> Of course the submodule has to get its own history, it's not possible\n> to do otherwise.  But you have to trigger the submodule merge when you\n> find a submodule-level conflict in the supermodule merge, just as\n> you trigger file-level three-way merges, too.\n\nI think you missed Linus's point: If the supermodule's merge leads to a\nconflict in the submodule links, it is not appropriate to merge the\nsubmodule.\n\nSay you are merging commits A and B in the supermodule, and A uses v1.0\nof the submodule and B uses v2.0 of submodule, then you can't just merge\nv1.0 and v2.0 - instead, you have to make a decision whether the\nsupermodule's merge result is going to use v1.0 or v2.0 or even\nsomething different like v2.1. An automatic merge cannot make this\ndecision for you (unless there was no conflict in the first place).\n\n-- Hannes\n"},{"id":"41343","messageId":"alpine.LFD.0.98.0705070857100.3802@woody.linux-foundation.org","threadId":"8004","inReplyTo":"463EFFC6.12A1B0A1@eudaptics.com","subject":"Re: [PATCH] submodule merge support","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-05-07T16:02:00Z","receivedAt":"2007-05-07T16:02:00Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 7 May 2007, Johannes Sixt wrote:\n> \n> I think you missed Linus's point: If the supermodule's merge leads to a\n> conflict in the submodule links, it is not appropriate to merge the\n> submodule.\n\nThat is true, but no, that wasn't what I was trying to say.\n\nWhat I was trying to say was really that the merge-base in the \nsuper-module is simply totally irrelevant to the sub-module, and any merge \nat all that thinks it is is obviously broken.\n\nNow, for a _normal_ merge (with just a single merge-base), this is not an \nissue, since the proposed submodule merger wouldn't care about the \nsupermodule merge base anyway.\n\nBut if you have multiple merge-bases and you do a recursive merge to \ncreate a new *combined* merge-base, trying to do that for the submodule is \njust pointless. You shouldn't. The merge-base for the submodule will be \nirrelevant for the final merge *anyway* (since the submodule history comes \nfrom itself), so in a recursive sub-merge, you shouldn't even *try* to \nmerge the submodule. The end result would never be used anyway, and the \nonly thing you can do is make for more complexity.\n\nSo not doing it in the low-level merger is right - because it is simply \nirrelevant at that stage. The low-level merger might as well ignore \nsubmodules.\n\nI think.\n\n\t\t\tLinus\n"},{"id":"41344","messageId":"20070507163758.GJ30511@admingilde.org","threadId":"8004","inReplyTo":"463EFFC6.12A1B0A1@eudaptics.com","subject":"Re: [PATCH] submodule merge support","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2007-05-07T16:37:58Z","receivedAt":"2007-05-07T16:37:58Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nOn Mon, May 07, 2007 at 12:30:30PM +0200, Johannes Sixt wrote:\n> Martin Waitz wrote:\n> > On Sun, May 06, 2007 at 03:18:53PM -0700, Linus Torvalds wrote:\n> > > On Mon, 7 May 2007, Alex Riesen wrote:\n> > > > How about making all existing strategies just ignore submodules, and\n> > > > move recursive merge in the merge driver (git-merge.sh)?\n> > >\n> > > Yes, I think that's the right thing to do.\n> > >\n> > > I think it's the right thing for another reason: in a true \"recursive\"\n> > > merge, the submodules shouldn't be recursively merged anyway. *THEIR*\n> > > merge will have its own history, and doing it based on some random history\n> > > of the superproject is actually wrong anyway!\n> > \n> > Of course the submodule has to get its own history, it's not possible\n> > to do otherwise.  But you have to trigger the submodule merge when you\n> > find a submodule-level conflict in the supermodule merge, just as\n> > you trigger file-level three-way merges, too.\n> \n> I think you missed Linus's point: If the supermodule's merge leads to a\n> conflict in the submodule links, it is not appropriate to merge the\n> submodule.\n\nwhy?\n\nWe do file-level merging, too.\nA submodule is not that different, besides being much more complex ;-)\nWe should try to automatically merge the content if possible and leave\nconflict markers if not.\n\n> Say you are merging commits A and B in the supermodule, and A uses v1.0\n> of the submodule and B uses v2.0 of submodule, then you can't just merge\n> v1.0 and v2.0 - instead, you have to make a decision whether the\n> supermodule's merge result is going to use v1.0 or v2.0 or even\n> something different like v2.1. An automatic merge cannot make this\n> decision for you (unless there was no conflict in the first place).\n\nSo when should we try to merge and when should we leave conflict\nmarkers?\n\nLet's look at submodule commits O, A, B which are the commits that are\nused by the (supermodule-) merge base, our supermodule version and their\nsupermodule version.\n\nThe trivial cases are:\n\n O,A -..- B        O,B -..- A\n\nThen there is the fast-forward case:\n\n O -..- A -..- B      O -..- B -..- A\n\nAll of those do not require to create any new submodule commit, just use\nthe one on the right side of the chain.  I think they are not\ncontroversial.\n\nNow to the more complex parts:\n\n          /-..- A\n O -..- X\n          \\-..- B\n\nand\n\n   /-..- A\n X\n   \\-..- O -..- B\n\nHere we don't have any existing submodule commit that can be used.\nIn the second case an automatic merge cannot be done -- one supermodule\nbranch choose to rewind the submodule (e.g. by switching to another\nbranch in the submodule) and there really is no way to automatically\ndetect what should be done.\nBut when the submodule got branched after the supermodule merge-base\nthen an automatic submodule merge can be tried.\nAnd it really makes sense to do so:\nIf you rely on submodule for your project, you will have people\nsubmitting features which touch several submodules.  They can simply\ncreate a new supermodule version incorporating all their changes and\nsend it to their supermodule-upstream.  After review,\nsupermodule-upstream wants all the new submodule versions merged into\nher tree and it would be wrong to refuse to do that automatically.\n\nThe above behaviour is exactly what is implemented in the patch.\n(OK, I could write a more elaborate commit message... ;-)\n\n-- \nMartin Waitz\n"},{"id":"41345","messageId":"20070507164434.GK30511@admingilde.org","threadId":"8004","inReplyTo":"alpine.LFD.0.98.0705070857100.3802@woody.linux-foundation.org","subject":"Re: [PATCH] submodule merge support","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2007-05-07T16:44:34Z","receivedAt":"2007-05-07T16:44:34Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nOn Mon, May 07, 2007 at 09:02:00AM -0700, Linus Torvalds wrote:\n> But if you have multiple merge-bases and you do a recursive merge to \n> create a new *combined* merge-base, trying to do that for the submodule is \n> just pointless.\n\nAh, now I see your point.\nAnd yes, you are right.\n\n> So not doing it in the low-level merger is right - because it is simply \n> irrelevant at that stage. The low-level merger might as well ignore \n> submodules.\n\nBut where to do it then?\nDo a first run which simply ignores dirlinks and then do another run\nwhich looks for all unmerged index entries and processes all dirlinks\nby merging the submodule?\n\n-- \nMartin Waitz\n"},{"id":"41355","messageId":"20070507193314.GL30511@admingilde.org","threadId":"8004","inReplyTo":"alpine.LFD.0.98.0705071015230.3802@woody.linux-foundation.org","subject":"Re: [PATCH] submodule merge support","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2007-05-07T19:33:14Z","receivedAt":"2007-05-07T19:33:14Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nOn Mon, May 07, 2007 at 10:17:11AM -0700, Linus Torvalds wrote:\n> Oh, btw, you should also look out for the case where the superproject \n> doesn't actually have the subprojects in question checked out. What \n> happens then? Just leave them conflicted?\n\nyes, I think that's the only sane thing one can do.\n\n-- \nMartin Waitz\n"}]}