{"thread":{"id":"42896","subject":"[PATCH] git-subtree.sh: Use --allow-unrelated-histories when splitting with --rejoin","startedAt":"2016-07-21T01:34:19Z","lastAt":"2016-07-22T17:15:49Z","messageCount":6,"participants":["Brett Cundal","David Aguilar","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"291901","messageId":"010201560af48050-012ea887-a1e1-4d1b-82d3-4799ac7788bc-000000@eu-west-1.amazonses.com","threadId":"42896","inReplyTo":null,"subject":"[PATCH] git-subtree.sh: Use --allow-unrelated-histories when splitting with --rejoin","fromName":"Brett Cundal","fromEmail":"brett.cundal@iugome.com","sentAt":"2016-07-21T00:56:51Z","receivedAt":"2016-07-21T01:34:19Z","isPatch":true,"sender":{"key":"brett.cundal@iugome.com","avatar":null},"body":"---\n contrib/subtree/git-subtree.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex 7a39b30..556cd92 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -661,7 +661,7 @@ cmd_split()\n \tif [ -n \"$rejoin\" ]; then\n \t\tdebug \"Merging split branch into HEAD...\"\n \t\tlatest_old=$(cache_get latest_old)\n-\t\tgit merge -s ours \\\n+\t\tgit merge -s ours --allow-unrelated-histories \\\n \t\t\t-m \"$(rejoin_msg \"$dir\" $latest_old $latest_new)\" \\\n \t\t\t$latest_new >&2 || exit $?\n \tfi\n\n--\nhttps://github.com/git/git/pull/274\n"},{"id":"291905","messageId":"20160721052625.GA31423@gmail.com","threadId":"42896","inReplyTo":"010201560af48050-012ea887-a1e1-4d1b-82d3-4799ac7788bc-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH] git-subtree.sh: Use --allow-unrelated-histories when splitting with --rejoin","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2016-07-21T05:26:25Z","receivedAt":"2016-07-21T05:26:34Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"[cc'd Roberto for submitGit q's]\n\nOn Thu, Jul 21, 2016 at 12:56:51AM +0000, Brett Cundal wrote:\n> ---\n\n<emtpy commit message>\n\nThe message on the pull request[1] has a better justification\nfor this change, which would have been nice in the commit\nmessage itself:\n\n\n\tGit 2.9 added a check against merging unrelated histories, which\n\tis exactly what git subtree with --rejoin does. Adding the\n\t--allow-unrelated-histories flag to merge will override this\n\tcheck.\n\n\nIs it possible that maybe submitGit can detect an empty commit\nmessage for single-commit PRs and transplant that message onto it?\n\nAs-is, the commit itself should probably be amended to contain\nthat information.\n\n>  contrib/subtree/git-subtree.sh | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\n> index 7a39b30..556cd92 100755\n> --- a/contrib/subtree/git-subtree.sh\n> +++ b/contrib/subtree/git-subtree.sh\n> @@ -661,7 +661,7 @@ cmd_split()\n>  \tif [ -n \"$rejoin\" ]; then\n>  \t\tdebug \"Merging split branch into HEAD...\"\n>  \t\tlatest_old=$(cache_get latest_old)\n> -\t\tgit merge -s ours \\\n> +\t\tgit merge -s ours --allow-unrelated-histories \\\n>  \t\t\t-m \"$(rejoin_msg \"$dir\" $latest_old $latest_new)\" \\\n>  \t\t\t$latest_new >&2 || exit $?\n>  \tfi\n> \n> --\n\nWith the above description this change makes more sense,\nbut it seems that the existing tests do not detect the breakage\nfixed by this patch.\n\nCan you please add a test case in t/t7900-subtree.sh\ndemonstrating the breakage?\n\nLooks good otherwise.\n\n[1] https://github.com/git/git/pull/274\n-- \nDavid\n"},{"id":"291921","messageId":"alpine.DEB.2.20.1607211724430.14111@virtualbox","threadId":"42896","inReplyTo":"20160721052625.GA31423@gmail.com","subject":"Re: [PATCH] git-subtree.sh: Use --allow-unrelated-histories when splitting with --rejoin","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-21T15:25:41Z","receivedAt":"2016-07-21T15:26:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi David,\n\nOn Wed, 20 Jul 2016, David Aguilar wrote:\n\n> As-is, the commit itself should probably be amended to contain\n> that information [the better explanation].\n\nDefinitely. Keep in mind: if this gets merged or cherry-picked elsewhere,\nthe Pull Request's message is just as lost.\n\nCiao,\nJohannes\n"},{"id":"291929","messageId":"CAMaPC4JTerX7TSg5_M=DOztyaGUVd5_kK8cmTJuWrCekGgSodw@mail.gmail.com","threadId":"42896","inReplyTo":"alpine.DEB.2.20.1607211724430.14111@virtualbox","subject":"Re: [PATCH] git-subtree.sh: Use --allow-unrelated-histories when splitting with --rejoin","fromName":"Brett Cundal","fromEmail":"brett.cundal@iugome.com","sentAt":"2016-07-21T18:09:32Z","receivedAt":"2016-07-21T18:09:38Z","isPatch":true,"sender":{"key":"brett.cundal@iugome.com","avatar":null},"body":"[re-sending as plain-text so the list software doesn't reject as\nspam... what decade is this?]\n\nHey,\n\nSorry about the empty commit message... there was one originally\n(albeit not as detailed as the pull request), but I guess it got\nstripped? As you have probably guessed, I have no idea how you guys do\npatch submission.\n\nI'm not currently able to spend much time getting up to speed with git\ndevelopment just to submit this fix... I would have just filed a bug,\nbut I couldn't figure out how (I guess it's just this mailing list?)\nand I figured a patch would be more useful. I had a look at writing a\ntest initially, but I didn't see any existing tests for 'git subtree',\nand I'm not familiar with the test framework.\n\nThe case that fails is basically any usage of split --rejoin... e.g.\nmake a new repo, make changes to directory a, make changes to\ndirectory b... do a 'git subtree split -P a --rejoin' and it will fail\nat the end when attempting to merge the split commits back to the main\nline, due to the two lines being unrelated (now disallowed by default\nin git 2.9).\n\nHopefully I'm not just doing something wrong - I'm surprised such a\nmajor bug has not been fixed already... I guess noone else uses\n--rejoin? Anyhow, this patch fixes the issue for me.\n\nHope that helps,\n\n-- Brett\n\nOn 21 July 2016 at 08:25, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi David,\n>\n> On Wed, 20 Jul 2016, David Aguilar wrote:\n>\n>> As-is, the commit itself should probably be amended to contain\n>> that information [the better explanation].\n>\n> Definitely. Keep in mind: if this gets merged or cherry-picked elsewhere,\n> the Pull Request's message is just as lost.\n>\n> Ciao,\n> Johannes\n"},{"id":"291942","messageId":"alpine.DEB.2.20.1607220954470.14111@virtualbox","threadId":"42896","inReplyTo":"CAMaPC4JTerX7TSg5_M=DOztyaGUVd5_kK8cmTJuWrCekGgSodw@mail.gmail.com","subject":"Re: [PATCH] git-subtree.sh: Use --allow-unrelated-histories when splitting with --rejoin","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-22T08:11:07Z","receivedAt":"2016-07-22T08:11:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Brett,\n\nOn Thu, 21 Jul 2016, Brett Cundal wrote:\n\n> [re-sending as plain-text so the list software doesn't reject as\n> spam... what decade is this?]\n\nI know it feels good to get frustration off of your chest by ranting. I am\nvery sympathetic to that. Please note, however, that it puts the people\nwho are ready to help you immediately into a defensive corner. Probably\nunintentional?\n\n> Sorry about the empty commit message... there was one originally (albeit\n> not as detailed as the pull request), but I guess it got stripped? As\n> you have probably guessed, I have no idea how you guys do patch\n> submission.\n\nIt is okay. For historical reasons, the patch submission is a bit more\ninvolved than simply opening a Pull Request. We also frown upon\ntop-posting, among other rules of netiquette. You may dislike it, but you\nwould have to build more of a \"street cred\" if you truly wanted to try to\nconvince the Git developers to change it.\n\nBack to the patch you wanted to submit: since this is an important bug\nfix, I spent the time to clean it up. The only missing bit that requires\nfurther effort from your side is that we need your Sign-off. See\nhttps://github.com/git/git/blob/v2.9.2/Documentation/SubmittingPatches#L239-L291\nfor an explanation. Essentially, you are stating explicitly that you are\nnot legally prohibited from contributing this patch. If you can say that,\nsimply reply with a\n\n\tSigned-off-by: Brett Cundal <brett.cundal@iugome.com>\n\nWe can take it from there by inserting it into the following patch:\n\n-- snipsnap --\nFrom: Brett Cundal <brett.cundal@iugome.com>\nSubject: [PATCH] subtree: allow unrelated histories when splitting with --rejoin\n\nGit 2.9 added a check against merging unrelated histories, which is\nexactly what git subtree with --rejoin does. Adding the\n--allow-unrelated-histories flag to merge will override this check.\n---\n contrib/subtree/git-subtree.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex 7a39b30..556cd92 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -661,7 +661,7 @@ cmd_split()\n \tif [ -n \"$rejoin\" ]; then\n \t\tdebug \"Merging split branch into HEAD...\"\n \t\tlatest_old=$(cache_get latest_old)\n-\t\tgit merge -s ours \\\n+\t\tgit merge -s ours --allow-unrelated-histories \\\n \t\t\t-m \"$(rejoin_msg \"$dir\" $latest_old $latest_new)\" \\\n \t\t\t$latest_new >&2 || exit $?\n \tfi\n-- \n2.9.0.281.g286a8d9\n\n\n"},{"id":"291980","messageId":"CAMaPC4+rjr3bSkKcUFipDwW6es4y9Rgq7Z-3SPz0A7qLMw77Pw@mail.gmail.com","threadId":"42896","inReplyTo":"alpine.DEB.2.20.1607220954470.14111@virtualbox","subject":"Re: [PATCH] git-subtree.sh: Use --allow-unrelated-histories when splitting with --rejoin","fromName":"Brett Cundal","fromEmail":"brett.cundal@iugome.com","sentAt":"2016-07-22T17:15:43Z","receivedAt":"2016-07-22T17:15:49Z","isPatch":true,"sender":{"key":"brett.cundal@iugome.com","avatar":null},"body":"On 22 July 2016 at 01:11, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n\n> I know it feels good to get frustration off of your chest by ranting. I am\n> very sympathetic to that. Please note, however, that it puts the people\n> who are ready to help you immediately into a defensive corner. Probably\n> unintentional?\n\nShortest rant ever. :) Sorry if I caused any offense. It just seemed very odd to\nblock multi-part MIME that happened to contain a text/html part in addition to\ntext/plain, given that (AFAIK) the vast majority of email clients in\nrecent years\nproduce such a thing. Anyhow, off-topic... your list policy is your business.\n\n> Back to the patch you wanted to submit: since this is an important bug\n> fix, I spent the time to clean it up. The only missing bit that requires\n> further effort from your side is that we need your Sign-off. See\n> https://github.com/git/git/blob/v2.9.2/Documentation/SubmittingPatches#L239-L291\n> for an explanation. Essentially, you are stating explicitly that you are\n> not legally prohibited from contributing this patch. If you can say that,\n> simply reply with a\n>\n>         Signed-off-by: Brett Cundal <brett.cundal@iugome.com>\n>\n> We can take it from there by inserting it into the following patch:\n\nSo, again I'll have to apologise... I should have submitted this as a bug\nreport rather than a patch, since the ownership is not legally clear. Didn't\neven occur to me for such a trivial fix. If you can just treat this as\na bug report\nat this point and 'reimplement' it, that would probably be the\nsimplest thing for\neveryone... I guess this may be tricky since there are limited ways to possibly\nimplement this, but for the same reason it would be impossible IMO for me or\nanyone else to legally claim authorship of it (but IANAL). If this is\ngoing to cause\nthis fix to be jammed in limbo for all eternity, then let me know and\nI'll see what\nI can do to get the proper authorization.\n\n-- Brett\n"}]}