{"thread":{"id":"10746","subject":"[BUG] git-rebase fails when a commit message contains a diff","startedAt":"2007-11-09T01:12:14Z","lastAt":"2007-11-11T22:31:34Z","messageCount":17,"participants":["Jonas Fonseca","Junio C Hamano","Johannes Schindelin","Steffen Prohaska","Benoit Sigoure","Johannes Sixt","Ralf Wildenhues","Andreas Ericsson","Björn Steinbrink"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"58964","messageId":"20071109011214.GA5903@diku.dk","threadId":"10746","inReplyTo":null,"subject":"[BUG] git-rebase fails when a commit message contains a diff","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2007-11-09T01:12:14Z","receivedAt":"2007-11-09T01:12:14Z","isPatch":false,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"I have concocted a test that illustrates the problem: patch below. The problem\nis that the patches in .dotest are not properly formatted to make it easy to\ndistinguish commit message from commit diff.\n\nWhen running the test git-rebase fails with:\n\n\tApplying Test message with diff in it\n\terror: config: does not exist in index\n\tfatal: sha1 information is lacking or useless (config).\n\tRepository lacks necessary blobs to fall back on 3-way merge.\n\tCannot fall back to three-way merge.\n\tPatch failed at 0004.\n\nBy modifying the test below, a similar problem related with handling of the\nend-of-message marker (---\\n) can be exposed. A commit message that contains\nthis marker and text afterwards ends up only having the text preceeding the\nmarker.\n\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex 95e33b5..75babdb 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -83,4 +83,25 @@ test_expect_success 'rebase a single mode change' '\n GIT_TRACE=1 git rebase master\n '\n \n+cat >commitdiff-message < W &&\n+ git add W &&\n+ git commit -F commitdiff-message &&\n+ git rebase master\n+'\n+\n test_done\n\n-- \nJonas Fonseca\n"},{"id":"58974","messageId":"7vlk98xkm3.fsf@gitster.siamese.dyndns.org","threadId":"10746","inReplyTo":"20071109011214.GA5903@diku.dk","subject":"Re: [BUG] git-rebase fails when a commit message contains a diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-09T01:39:48Z","receivedAt":"2007-11-09T01:39:48Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"That's a known design limitation of applymbox/mailinfo. Any\nline that looks like a beginning of a patch in e-mail (\"^--- \",\n\"^---$\", \"^diff -\", and \"^Index: \") terminates the commit log.\n"},{"id":"58977","messageId":"7vhcjwxk1s.fsf@gitster.siamese.dyndns.org","threadId":"10746","inReplyTo":"7vlk98xkm3.fsf@gitster.siamese.dyndns.org","subject":"Re: [BUG] git-rebase fails when a commit message contains a diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-09T01:51:59Z","receivedAt":"2007-11-09T01:51:59Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> That's a known design limitation of applymbox/mailinfo. Any\n> line that looks like a beginning of a patch in e-mail (\"^--- \",\n> \"^---$\", \"^diff -\", and \"^Index: \") terminates the commit log.\n\nOk, so that explains the symptom. What's the next step?\n\n * The applymbox/mailinfo pair should continue to split the\n commit log message at the first such line. There is no point\n breaking established workflow, and people in communities that\n exchange patches via e-mail already know to avoid this issue\n by indenting quoted diff snippet in the log message,\n e.g. 5be507fc.\n\n * There is no fundamental reason for rebase to use e-mail\n format to express what \"format-patch | am -3\" pipeline does.\n We do it currently because (1) it was expedient to reuse what\n was already there, and because (2) the original target\n audience of git are e-mail oriented communities, so there was\n not strong incentive to make rebase independent of the\n applymbox/mailinfo limitation (that is, even if you make\n rebase able to handle such a patch, you cannot send out the\n result over e-mail *anyway*).\n\n This however does not mean we should always use merging\n rebase. patch based approach \"format-patch | am -3\" pipeline\n uses is often much faster. Instead of using \"format-patch |\n am -3\", we could use more careful patch and message\n generation, like git-rebase--interactive.sh:make_patch()\n does.\n"},{"id":"58980","messageId":"7v640cxitc.fsf@gitster.siamese.dyndns.org","threadId":"10746","inReplyTo":"7vhcjwxk1s.fsf@gitster.siamese.dyndns.org","subject":"Re: [BUG] git-rebase fails when a commit message contains a diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-09T02:18:39Z","receivedAt":"2007-11-09T02:18:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I wonder if this is a sensible thing to do, regardless of the\nissue of commit log message that contains anything.\n\nThe patch replaces git-rebase with git-rebase--interactive. The\nonly difference from the existing \"git-rebase -i\" is if the\ncommand is called without \"-i\" the initial \"here is the to-do\nlist. please rearrange the lines, modify 'pick' to 'edit' or\nwhatever as appropriate\" step is done without letting the user\nedit the list.\n\n---\n\n Makefile | 2 +-\n git-rebase--interactive.sh => git-rebase.sh | 14 ++++++++++----\n 2 files changed, 11 insertions(+), 5 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 0d5590f..f0e5e38 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -214,7 +214,7 @@ SCRIPT_SH = \\\n \tgit-clean.sh git-clone.sh git-commit.sh \\\n \tgit-ls-remote.sh \\\n \tgit-merge-one-file.sh git-mergetool.sh git-parse-remote.sh \\\n-\tgit-pull.sh git-rebase.sh git-rebase--interactive.sh \\\n+\tgit-pull.sh git-rebase.sh \\\n \tgit-repack.sh git-request-pull.sh \\\n \tgit-sh-setup.sh \\\n \tgit-am.sh \\\ndiff --git a/git-rebase--interactive.sh b/git-rebase.sh\nsimilarity index 98%\nrename from git-rebase--interactive.sh\nrename to git-rebase.sh\nindex 76dc679..1dd6f6d 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase.sh\n@@ -11,7 +11,7 @@\n # http://article.gmane.org/gmane.comp.version-control.git/22407\n \n USAGE='(--continue | --abort | --skip | [--preserve-merges] [--verbose]\n-\t[--onto ] [])'\n+\t[--interactive] [--onto ] [])'\n \n . git-sh-setup\n require_work_tree\n@@ -25,6 +25,7 @@ REWRITTEN=\"$DOTEST\"/rewritten\n PRESERVE_MERGES=\n STRATEGY=\n VERBOSE=\n+INTERACTIVE=\n test -d \"$REWRITTEN\" && PRESERVE_MERGES=t\n test -f \"$DOTEST\"/strategy && STRATEGY=\"$(cat \"$DOTEST\"/strategy)\"\n test -f \"$DOTEST\"/verbose && VERBOSE=t\n@@ -346,6 +347,9 @@ do_rest () {\n while test $# != 0\n do\n \tcase \"$1\" in\n+\t--interactive|-i)\n+\t\tINTERACTIVE=t\n+\t\t;;\n \t--continue)\n \t\tcomment_for_reflog continue\n \n@@ -504,9 +508,11 @@ EOF\n \t\t\tdie_abort \"Nothing to do\"\n \n \t\tcp \"$TODO\" \"$TODO\".backup\n-\t\tgit_editor \"$TODO\" ||\n-\t\t\tdie \"Could not execute editor\"\n-\n+\t\tcase \"$INTERACTIVE\" in\n+\t\tt)\n+\t\t\tgit_editor \"$TODO\" || die \"Could not execute editor\"\n+\t\t\t;;\n+\t\tesac\n \t\thas_action \"$TODO\" ||\n \t\t\tdie_abort \"Nothing to do\"\n \n"},{"id":"58981","messageId":"Pine.LNX.4.64.0711090225110.4362@racer.site","threadId":"10746","inReplyTo":"7v640cxitc.fsf@gitster.siamese.dyndns.org","subject":"Re: [BUG] git-rebase fails when a commit message contains a diff","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-09T02:28:15Z","receivedAt":"2007-11-09T02:28:15Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 8 Nov 2007, Junio C Hamano wrote:\n\n> I wonder if this is a sensible thing to do, regardless of the issue of \n> commit log message that contains anything.\n> \n> The patch replaces git-rebase with git-rebase--interactive. The only \n> difference from the existing \"git-rebase -i\" is if the command is called \n> without \"-i\" the initial \"here is the to-do list. please rearrange the \n> lines, modify 'pick' to 'edit' or whatever as appropriate\" step is done \n> without letting the user edit the list.\n\nHmm. I don't know, really. I had the impression that the \"git \nformat-patch | git am\" pipeline would be faster.\n\nBut if we were to do this, we'd get a progress meter for free. And bugs \nexposed, no doubt.\n\n> Makefile | 2 +-\n> git-rebase--interactive.sh => git-rebase.sh | 14 ++++++++++----\n> 2 files changed, 11 insertions(+), 5 deletions(-)\n\nWhat about the existing git-rebase.sh?\n\n> diff --git a/git-rebase--interactive.sh b/git-rebase.sh\n> similarity index 98%\n> rename from git-rebase--interactive.sh\n> rename to git-rebase.sh\n> index 76dc679..1dd6f6d 100755\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase.sh\n> @@ -346,6 +347,9 @@ do_rest () {\n> while test $# != 0\n> do\n> \tcase \"$1\" in\n> +\t--interactive|-i)\n> +\t\tINTERACTIVE=t\n> +\t\t;;\n\nThere is already a case for that, further down.\n\n> @@ -504,9 +508,11 @@ EOF\n> \t\t\tdie_abort \"Nothing to do\"\n> \n> \t\tcp \"$TODO\" \"$TODO\".backup\n> -\t\tgit_editor \"$TODO\" ||\n> -\t\t\tdie \"Could not execute editor\"\n> -\n> +\t\tcase \"$INTERACTIVE\" in\n> +\t\tt)\n> +\t\t\tgit_editor \"$TODO\" || die \"Could not execute editor\"\n> +\t\t\t;;\n> +\t\tesac\n\n\nWould that not be easier to read as\n\n\t\ttest t = \"$INTERACTIVE\" &&\n\t\t\tgit_editor \"$TODO\" || die \"Could not execute editor\"\n\nHmm?\n\nCiao,\nDscho\n"},{"id":"58982","messageId":"7v1wb0xhxq.fsf@gitster.siamese.dyndns.org","threadId":"10746","inReplyTo":"Pine.LNX.4.64.0711090225110.4362@racer.site","subject":"Re: [BUG] git-rebase fails when a commit message contains a diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-09T02:37:37Z","receivedAt":"2007-11-09T02:37:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> On Thu, 8 Nov 2007, Junio C Hamano wrote:\n>\n>> I wonder if this is a sensible thing to do, regardless of the issue of \n>> commit log message that contains anything.\n>> \n>> The patch replaces git-rebase with git-rebase--interactive. The only \n>> difference from the existing \"git-rebase -i\" is if the command is called \n>> without \"-i\" the initial \"here is the to-do list. please rearrange the \n>> lines, modify 'pick' to 'edit' or whatever as appropriate\" step is done \n>> without letting the user edit the list.\n>\n> Hmm. I don't know, really. I had the impression that the \"git \n> format-patch | git am\" pipeline would be faster.\n\nHeh, I did not read rebase--interactive carefully enough.\n\nUnless told to use merge with \"rebase -m\", rebase replays the\nchange by extracting and applying patches, and speed comparison\nwas about that vs merge based replaying; I thought make_patch\nwas done in order to avoid using cherry-pick (which is based on\nmerge-recursive) and doing patch application with three-way\nfallback. Apparently that is not what \"interactive\" does.\n\nPerhaps pick_one () could be taught to perform the 3-way\nfallback dance git-am plays correctly. The patch I sent to make\ngit-rebase--interactive take over git-rebase would then become\nquite reasonable, I would think.\n"},{"id":"58995","messageId":"B2BAA0E1-8DA7-417D-AE25-53D8690810ED@zib.de","threadId":"10746","inReplyTo":"7vhcjwxk1s.fsf@gitster.siamese.dyndns.org","subject":"Re: [BUG] git-rebase fails when a commit message contains a diff","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2007-11-09T07:03:03Z","receivedAt":"2007-11-09T07:03:03Z","isPatch":false,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"\nOn Nov 9, 2007, at 2:51 AM, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n>\n>> That's a known design limitation of applymbox/mailinfo. Any\n>> line that looks like a beginning of a patch in e-mail (\"^--- \",\n>> \"^---$\", \"^diff -\", and \"^Index: \") terminates the commit log.\n>\n> Ok, so that explains the symptom. What's the next step?\n>\n> * The applymbox/mailinfo pair should continue to split the\n> commit log message at the first such line. There is no point\n> breaking established workflow, and people in communities that\n> exchange patches via e-mail already know to avoid this issue\n> by indenting quoted diff snippet in the log message,\n> e.g. 5be507fc.\n\nI wasn't aware of this.\n\nMaybe git-commit should validate commit messages? If people\nhave a notion of a well-formed commit message this should be\nverified. The message should not contain strings that indicate\ntermination of the commit message when sent by email. If commit\ndid this the evil strings would never enter a commit message\nin the first place.\n\nActually I sometimes use '---' as a separator in text. I'm\nsure I'll forget at some point that I must not use it in a\ncommit message. I'd be happy if git saved me from this.\n\nIf I understand correctly, the problem will remain when\nsending patches by emails; even if git-rebase was changed\nto use merge. Or does git-format-patch quote evil strings in\ncommit messages?\n\n\tSteffen\n"},{"id":"59001","messageId":"6FCE17E3-9FAA-4676-B12A-369B31743DA6@lrde.epita.fr","threadId":"10746","inReplyTo":"Pine.LNX.4.64.0711090225110.4362@racer.site","subject":"Re: [BUG] git-rebase fails when a commit message contains a diff","fromName":"Benoit Sigoure","fromEmail":"tsuna@lrde.epita.fr","sentAt":"2007-11-09T07:57:00Z","receivedAt":"2007-11-09T07:57:00Z","isPatch":false,"sender":{"key":"tsunanet@gmail.com","avatar":"https://avatars.githubusercontent.com/u/128281?v=4"},"body":"On Nov 9, 2007, at 3:28 AM, Johannes Schindelin wrote:\n\n> Would that not be easier to read as\n>\n> \t\ttest t = \"$INTERACTIVE\" &&\n> \t\t\tgit_editor \"$TODO\" || die \"Could not execute editor\"\n\nHmm this will `die' if you're not running interactively.\n\nOff topic question: why do you guys always do this instead of doing, \nsay, this:\n\nINTERACTIVE=false\n\ncase $1 in\n.\n.\n.\n --interactive|-i)\n INTERACTIVE=:\n ... ;;\nesac\n.\n.\n.\nif $INTERACTIVE; then\n git_editor \"$TODO\" || die ...\nfi\n\n\n?\n\n-- \nBenoit Sigoure aka Tsuna\nEPITA Research and Development Laboratory\n\n\n"},{"id":"59006","messageId":"473420FE.7010807@viscovery.net","threadId":"10746","inReplyTo":"6FCE17E3-9FAA-4676-B12A-369B31743DA6@lrde.epita.fr","subject":"Re: [BUG] git-rebase fails when a commit message contains a diff","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2007-11-09T08:57:34Z","receivedAt":"2007-11-09T08:57:34Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Benoit Sigoure schrieb:\n> Off topic question: why do you guys always do this instead of doing, \n> say, this:\n> \n> INTERACTIVE=false\n> \n> case $1 in\n> --interactive|-i)\n> INTERACTIVE=:\n> ... ;;\n> esac\n> if $INTERACTIVE; then\n> git_editor \"$TODO\" || die ...\n> fi\n\nBecause in some shells 'false' is not a built-in.\n\nBut then this might do it without the extra process:\n\n\tINTERACTIVE=\"! :\"\t# false\n\n\tcase $1 in\n\t--interactive|-i)\n\t INTERACTIVE=:\n\t ... ;;\n\tesac\n\tif $INTERACTIVE; then\n\t git_editor \"$TODO\" || die ...\n\tfi\n\n-- Hannes\n"},{"id":"59008","messageId":"20071109090509.GA20121@ins.uni-bonn.de","threadId":"10746","inReplyTo":"473420FE.7010807@viscovery.net","subject":"Re: [BUG] git-rebase fails when a commit message contains a diff","fromName":"Ralf Wildenhues","fromEmail":"ralf.wildenhues@gmx.de","sentAt":"2007-11-09T09:05:10Z","receivedAt":"2007-11-09T09:05:10Z","isPatch":false,"sender":{"key":"ralf.wildenhues@gmx.de","avatar":null},"body":"* Johannes Sixt wrote on Fri, Nov 09, 2007 at 09:57:34AM CET:\n> Benoit Sigoure schrieb:\n>> Off topic question: why do you guys always do this instead of doing, say, \n>> this:\n>>\n>> INTERACTIVE=false\n[...]\n>> if $INTERACTIVE; then\n>> git_editor \"$TODO\" || die ...\n>> fi\n>\n> Because in some shells 'false' is not a built-in.\n[...]\n> INTERACTIVE=\"! :\" # false\n\nWhich shells do not have 'false' but do support '!' as process exit\nstatus negation? For them, is it important to save another fork?\nSolaris sh has neither (it will error on the '!'), but it's ruled out\nfor git anyway. \n\nCheers,\nRalf\n"},{"id":"59010","messageId":"87747E02-0C25-4330-84F4-1C56C0196BD5@lrde.epita.fr","threadId":"10746","inReplyTo":"473420FE.7010807@viscovery.net","subject":"Re: [BUG] git-rebase fails when a commit message contains a diff","fromName":"Benoit Sigoure","fromEmail":"tsuna@lrde.epita.fr","sentAt":"2007-11-09T09:07:47Z","receivedAt":"2007-11-09T09:07:47Z","isPatch":false,"sender":{"key":"tsunanet@gmail.com","avatar":"https://avatars.githubusercontent.com/u/128281?v=4"},"body":"On Nov 9, 2007, at 9:57 AM, Johannes Sixt wrote:\n\n> Benoit Sigoure schrieb:\n>> Off topic question: why do you guys always do this instead of \n>> doing, say, this:\n>> INTERACTIVE=false\n>> case $1 in\n>> --interactive|-i)\n>> INTERACTIVE=:\n>> ... ;;\n>> esac\n>> if $INTERACTIVE; then\n>> git_editor \"$TODO\" || die ...\n>> fi\n>\n> Because in some shells 'false' is not a built-in.\n\nCan you name such a shell? (besides Solaris' brain-damaged, b0rken \nand foobared /bin/sh which will most likely not work with Git anyway)\n\n> But then this might do it without the extra process:\n>\n> \tINTERACTIVE=\"! :\"\t# false\n\n`!' is not portable either. In particular, I highly doubt `!' will \nwork on shells that don't have `false' as a builtin (Hello Mr Solaris).\n\n>\n> \tcase $1 in\n> \t--interactive|-i)\n> \t INTERACTIVE=:\n> \t ... ;;\n> \tesac\n> \tif $INTERACTIVE; then\n> \t git_editor \"$TODO\" || die ...\n> \tfi\n\nCorrect me if I'm wrong but I think that some shells don't have \n`test' as a builtin either.\n\n-- \nBenoit Sigoure aka Tsuna\nEPITA Research and Development Laboratory\n\n\n"},{"id":"59016","messageId":"47343683.7080904@op5.se","threadId":"10746","inReplyTo":"Pine.LNX.4.64.0711090225110.4362@racer.site","subject":"Re: [BUG] git-rebase fails when a commit message contains a diff","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-11-09T10:29:23Z","receivedAt":"2007-11-09T10:29:23Z","isPatch":false,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Johannes Schindelin wrote:\n>> +\t\tcase \"$INTERACTIVE\" in\n>> +\t\tt)\n>> +\t\t\tgit_editor \"$TODO\" || die \"Could not execute editor\"\n>> +\t\t\t;;\n>> +\t\tesac\n> \n> \n> Would that not be easier to read as\n> \n> \t\ttest t = \"$INTERACTIVE\" &&\n> \t\t\tgit_editor \"$TODO\" || die \"Could not execute editor\"\n> \n\nWritten like that, it would die every time $INTERACTIVE isn't \"t\". You'd need\ncurly braces around \n\n\tgit_editor \"$TODO\" || die \"Could not execute editor\"\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"59040","messageId":"Pine.LNX.4.64.0711091319420.4362@racer.site","threadId":"10746","inReplyTo":"6FCE17E3-9FAA-4676-B12A-369B31743DA6@lrde.epita.fr","subject":"Re: [BUG] git-rebase fails when a commit message contains a diff","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-09T13:20:40Z","receivedAt":"2007-11-09T13:20:40Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 9 Nov 2007, Benoit Sigoure wrote:\n\n> On Nov 9, 2007, at 3:28 AM, Johannes Schindelin wrote:\n> \n> > Would that not be easier to read as\n> > \n> > \t\ttest t = \"$INTERACTIVE\" &&\n> > \t\t\tgit_editor \"$TODO\" || die \"Could not execute editor\"\n> \n> Hmm this will `die' if you're not running interactively.\n\nD'oh. Of course I meant\n\n\t\ttest -z \"$INTERACTIVE\" || git_editor \"$TODO\" || die ...\n\nCiao,\nDscho\n"},{"id":"59050","messageId":"Pine.LNX.4.64.0711091417500.4362@racer.site","threadId":"10746","inReplyTo":"B2BAA0E1-8DA7-417D-AE25-53D8690810ED@zib.de","subject":"Re: [BUG] git-rebase fails when a commit message contains a diff","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-09T14:18:15Z","receivedAt":"2007-11-09T14:18:15Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 9 Nov 2007, Steffen Prohaska wrote:\n\n> On Nov 9, 2007, at 2:51 AM, Junio C Hamano wrote:\n> \n> > Junio C Hamano writes:\n> > \n> > > That's a known design limitation of applymbox/mailinfo. Any\n> > > line that looks like a beginning of a patch in e-mail (\"^--- \",\n> > > \"^---$\", \"^diff -\", and \"^Index: \") terminates the commit log.\n> > \n> > Ok, so that explains the symptom. What's the next step?\n> > \n> > * The applymbox/mailinfo pair should continue to split the\n> > commit log message at the first such line. There is no point\n> > breaking established workflow, and people in communities that\n> > exchange patches via e-mail already know to avoid this issue\n> > by indenting quoted diff snippet in the log message,\n> > e.g. 5be507fc.\n> \n> I wasn't aware of this.\n\nBut there's a really easy workaround: use --merge with git rebase.\n\nCiao,\nDscho\n"},{"id":"59051","messageId":"Pine.LNX.4.64.0711091418510.4362@racer.site","threadId":"10746","inReplyTo":"7v1wb0xhxq.fsf@gitster.siamese.dyndns.org","subject":"Re: [BUG] git-rebase fails when a commit message contains a diff","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-09T14:19:56Z","receivedAt":"2007-11-09T14:19:56Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 8 Nov 2007, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > On Thu, 8 Nov 2007, Junio C Hamano wrote:\n> >\n> >> I wonder if this is a sensible thing to do, regardless of the issue \n> >> of commit log message that contains anything.\n> >> \n> >> The patch replaces git-rebase with git-rebase--interactive. The only \n> >> difference from the existing \"git-rebase -i\" is if the command is \n> >> called without \"-i\" the initial \"here is the to-do list. please \n> >> rearrange the lines, modify 'pick' to 'edit' or whatever as \n> >> appropriate\" step is done without letting the user edit the list.\n> >\n> > Hmm. I don't know, really. I had the impression that the \"git \n> > format-patch | git am\" pipeline would be faster.\n> \n> Heh, I did not read rebase--interactive carefully enough.\n> \n> Unless told to use merge with \"rebase -m\", rebase replays the change by \n> extracting and applying patches, and speed comparison was about that vs \n> merge based replaying; I thought make_patch was done in order to avoid \n> using cherry-pick (which is based on merge-recursive) and doing patch \n> application with three-way fallback. Apparently that is not what \n> \"interactive\" does.\n> \n> Perhaps pick_one () could be taught to perform the 3-way fallback dance \n> git-am plays correctly. The patch I sent to make \n> git-rebase--interactive take over git-rebase would then become quite \n> reasonable, I would think.\n\nI have a different idea: How about changing cherry-pick to try a simple \npatch first? If that fails, we can always go back to merge-recursive (or \nmerge-nu once that is ready).\n\nCiao,\nDscho\n"},{"id":"59053","messageId":"04B901C5-E8A4-494D-A8D0-4D430F818803@zib.de","threadId":"10746","inReplyTo":"Pine.LNX.4.64.0711091417500.4362@racer.site","subject":"Re: [BUG] git-rebase fails when a commit message contains a diff","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2007-11-09T14:29:05Z","receivedAt":"2007-11-09T14:29:05Z","isPatch":false,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"\nOn Nov 9, 2007, at 3:18 PM, Johannes Schindelin wrote:\n\n> Hi,\n>\n> On Fri, 9 Nov 2007, Steffen Prohaska wrote:\n>\n>> On Nov 9, 2007, at 2:51 AM, Junio C Hamano wrote:\n>>\n>>> Junio C Hamano writes:\n>>>\n>>>> That's a known design limitation of applymbox/mailinfo. Any\n>>>> line that looks like a beginning of a patch in e-mail (\"^--- \",\n>>>> \"^---$\", \"^diff -\", and \"^Index: \") terminates the commit log.\n>>>\n>>> Ok, so that explains the symptom. What's the next step?\n>>>\n>>> * The applymbox/mailinfo pair should continue to split the\n>>> commit log message at the first such line. There is no point\n>>> breaking established workflow, and people in communities that\n>>> exchange patches via e-mail already know to avoid this issue\n>>> by indenting quoted diff snippet in the log message,\n>>> e.g. 5be507fc.\n>>\n>> I wasn't aware of this.\n>\n> But there's a really easy workaround: use --merge with git rebase.\n\nIt's no longer a problem for me. I now know the limitation and\nthe work around. But others could get bitten, too. We could\nsave them.\n\n\tSteffen\n"},{"id":"59393","messageId":"20071111223134.GA12699@atjola.homenet","threadId":"10746","inReplyTo":"7v1wb0xhxq.fsf@gitster.siamese.dyndns.org","subject":"Re: [BUG] git-rebase fails when a commit message contains a diff","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2007-11-11T22:31:34Z","receivedAt":"2007-11-11T22:31:34Z","isPatch":false,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"On 2007.11.08 18:37:37 -0800, Junio C Hamano wrote:\n> Johannes Schindelin writes:\n> \n> > On Thu, 8 Nov 2007, Junio C Hamano wrote:\n> >\n> >> I wonder if this is a sensible thing to do, regardless of the issue of \n> >> commit log message that contains anything.\n> >> \n> >> The patch replaces git-rebase with git-rebase--interactive. The only \n> >> difference from the existing \"git-rebase -i\" is if the command is called \n> >> without \"-i\" the initial \"here is the to-do list. please rearrange the \n> >> lines, modify 'pick' to 'edit' or whatever as appropriate\" step is done \n> >> without letting the user edit the list.\n> >\n> > Hmm. I don't know, really. I had the impression that the \"git \n> > format-patch | git am\" pipeline would be faster.\n> \n> Heh, I did not read rebase--interactive carefully enough.\n> \n> Unless told to use merge with \"rebase -m\", rebase replays the\n> change by extracting and applying patches, and speed comparison\n> was about that vs merge based replaying; I thought make_patch\n> was done in order to avoid using cherry-pick (which is based on\n> merge-recursive) and doing patch application with three-way\n> fallback. Apparently that is not what \"interactive\" does.\n> \n> Perhaps pick_one () could be taught to perform the 3-way\n> fallback dance git-am plays correctly. The patch I sent to make\n> git-rebase--interactive take over git-rebase would then become\n> quite reasonable, I would think.\n\nNote that git-rebase--interactive also doesn't really support the\n--strategy parameter which git-rebase handles using git-merge-* instead\nof git-am. Only merge commits during a -i -p run actually apply the\nstrategy.\n\nBjörn\n"}]}