{"thread":{"id":"28337","subject":"Git Bug - diff in commit message.","startedAt":"2011-09-08T14:49:13Z","lastAt":"2011-10-25T14:11:33Z","messageCount":9,"participants":["anikey","Junio C Hamano","Michael Witten","Thomas Rast","Johannes Sixt","Martin von Zweigbergk"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"175099","messageId":"1315493353942-6772145.post@n2.nabble.com","threadId":"28337","inReplyTo":null,"subject":"Git Bug - diff in commit message.","fromName":"anikey","fromEmail":"arty.anikey@gmail.com","sentAt":"2011-09-08T14:49:13Z","receivedAt":"2011-09-08T14:49:13Z","isPatch":false,"sender":{"key":"arty.anikey@gmail.com","avatar":null},"body":"Hi, peops. I'm pretty much sure that's a bug.\n\nWhat I did was putting git diff (i needed to tell people that for my changes\nto start working they needed to aplly message-inline patch to some code\nwhich was not under git) in commit message. Like adding:\n\ndiff --git a/app/controllers/settings_controller.rb\nb/app/controllers/settings_controller.rb\nindex 937da74..0e8440d 100644\n--- a/app/controllers/settings_controller.rb\n+++ b/app/controllers/settings_controller.rb\n@@ -42,7 +42,7 @@ class SettingsController < ApplicationController\n   end\n \n   def snmp_mibs\n-    render layout: 'ext3'\n+    render layout: 'ext3_2'\n   end\n \n   def cfg_auth_keys(auth_type=:all)\n\nthough the commit itself didn't contain that change. So while `git rebase\nsome_branch_name` I started getting:\n\nFirst, rewinding head to replay your work on top of it...\nApplying: My cool patch.\nfatal: sha1 information is lacking or useless\n(app/controllers/settings_controller.rb).\nRepository lacks necessary blobs to fall back on 3-way merge.\nCannot fall back to three-way merge.\nPatch failed at 0001 My cool patch.\n\nWhen you have resolved this problem run \"git rebase --continue\".\nIf you would prefer to skip this patch, instead run \"git rebase --skip\".\nTo restore the original branch and stop rebasing run \"git rebase --abort\".\n\nI wasn't able to figure out what was wrong for a very long time, when things\ncame to my mind.\n\nThanks.\n\n--\nView this message in context: http://git.661346.n2.nabble.com/Git-Bug-diff-in-commit-message-tp6772145p6772145.html\nSent from the git mailing list archive at Nabble.com.\n"},{"id":"175125","messageId":"CAMOZ1BtbpbG+19G6Hfau_2_F5L3Ad+x-Payd9aKajJxU_V_tyA@mail.gmail.com","threadId":"28337","inReplyTo":"1315493353942-6772145.post@n2.nabble.com","subject":"Re: Git Bug - diff in commit message.","fromName":"Michael Witten","fromEmail":"mfwitten@gmail.com","sentAt":"2011-09-08T16:26:01Z","receivedAt":"2011-09-08T16:26:01Z","isPatch":false,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"On Thu, Sep 8, 2011 at 14:49, anikey <arty.anikey@gmail.com> wrote:\n> Hi, peops. I'm pretty much sure that's a bug.\n>\n> What I did was putting git diff (i needed to tell people that for my changes\n> to start working they needed to aplly message-inline patch to some code\n> which was not under git) in commit message. Like adding:\n>\n> diff --git a/app/controllers/settings_controller.rb\n> b/app/controllers/settings_controller.rb\n> index 937da74..0e8440d 100644\n> --- a/app/controllers/settings_controller.rb\n> +++ b/app/controllers/settings_controller.rb\n> @@ -42,7 +42,7 @@ class SettingsController < ApplicationController\n>   end\n>\n>   def snmp_mibs\n> -    render layout: 'ext3'\n> +    render layout: 'ext3_2'\n>   end\n>\n>   def cfg_auth_keys(auth_type=:all)\n>\n> though the commit itself didn't contain that change. So while `git rebase\n> some_branch_name` I started getting:\n>\n> First, rewinding head to replay your work on top of it...\n> Applying: My cool patch.\n> fatal: sha1 information is lacking or useless\n> (app/controllers/settings_controller.rb).\n> Repository lacks necessary blobs to fall back on 3-way merge.\n> Cannot fall back to three-way merge.\n> Patch failed at 0001 My cool patch.\n>\n> When you have resolved this problem run \"git rebase --continue\".\n> If you would prefer to skip this patch, instead run \"git rebase --skip\".\n> To restore the original branch and stop rebasing run \"git rebase --abort\".\n\nHa!\n\nIt would appear that `git rebase' is in fact producing patches with\n`git format-patch' and then applying the resulting patches with `git\nam', which gets confused by your inline diff; this can be clearly seen\nin the `git-rebase--am[.sh]' file.\n\nPerhaps `git rebase' should be reimplemented to use `git cherry-pick',\nor does that suffer from the same problem?\n"},{"id":"175117","messageId":"7v39g7x7v1.fsf@alter.siamese.dyndns.org","threadId":"28337","inReplyTo":"1315493353942-6772145.post@n2.nabble.com","subject":"Re: Git Bug - diff in commit message.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-08T17:27:14Z","receivedAt":"2011-09-08T17:27:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"anikey <arty.anikey@gmail.com> writes:\n\n> Hi, peops. I'm pretty much sure that's a bug.\n\nI am not Peops, but will try to help ;-).\n\nAs \"rebase\" is essentially \"format-patch\" output piped into \"am\",\nunindented diff in a random place inside the commit message will be\nindistingushable from the beginning of the patch. You cannot have an\nunadulterated \"diff\" output in your commit message for this reason, and\nthis is not likely to change.\n\nOften what people do is to indent them, as the reason they quote patch in\nthe message is to make it serve as a supporting material. Incidentally,\nthat makes the resulting commit log message easier to read as well; e.g.\n\n        Fix bloop bug\n\n        An earlier commit had this patch that is quite bogus.\n\n            diff --git a/bloop.php b/bloop.php\n            index 937da74..0e8440d 100644\n            --- a/bloop.php\n            +++ b/bloop.php\n            @@ -42,7 +42,7 @@\n            ...\n\n        The assignment to frotz in the above hunk should happen after\n        nitfol variable is initialized.\n\n        This patch fixes it.\n\n        Signed-off-by: Cont Ributer <contributor@example.com>\n"},{"id":"175136","messageId":"201109090256.36306.trast@student.ethz.ch","threadId":"28337","inReplyTo":"CAMOZ1BtbpbG+19G6Hfau_2_F5L3Ad+x-Payd9aKajJxU_V_tyA@mail.gmail.com","subject":"Re: Git Bug - diff in commit message.","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2011-09-09T00:56:36Z","receivedAt":"2011-09-09T00:56:36Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Michael Witten wrote:\n> On Thu, Sep 8, 2011 at 14:49, anikey <arty.anikey@gmail.com> wrote:\n> > Hi, peops. I'm pretty much sure that's a bug.\n> >\n> > What I did was putting git diff (i needed to tell people that for my changes\n> > to start working they needed to aplly message-inline patch to some code\n> > which was not under git) in commit message. Like adding:\n> \n> It would appear that `git rebase' is in fact producing patches with\n> `git format-patch' and then applying the resulting patches with `git\n> am', which gets confused by your inline diff; this can be clearly seen\n> in the `git-rebase--am[.sh]' file.\n> \n> Perhaps `git rebase' should be reimplemented to use `git cherry-pick',\n> or does that suffer from the same problem?\n\nIt doesn't, since it uses a three-way merge.  But there's no real\nreason to reimplement anything; just use an interactive rebase, it\nuses cherry-pick in a loop.\n\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"175149","messageId":"4E69AEBB.7080007@viscovery.net","threadId":"28337","inReplyTo":"201109090256.36306.trast@student.ethz.ch","subject":"Re: Git Bug - diff in commit message.","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-09-09T06:14:19Z","receivedAt":"2011-09-09T06:14:19Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 9/9/2011 2:56, schrieb Thomas Rast:\n> Michael Witten wrote:\n>> Perhaps `git rebase' should be reimplemented to use `git cherry-pick',\n>> or does that suffer from the same problem?\n> \n> It doesn't, since it uses a three-way merge.  But there's no real\n> reason to reimplement anything; just use an interactive rebase, it\n> uses cherry-pick in a loop.\n\nBut you can just as well use 'git rebase -m', which uses cherry-pick (or\nsomething equivalent, dunno).\n\n-- Hannes\n"},{"id":"175206","messageId":"7vpqj9vh87.fsf@alter.siamese.dyndns.org","threadId":"28337","inReplyTo":"CAMOZ1BtbpbG+19G6Hfau_2_F5L3Ad+x-Payd9aKajJxU_V_tyA@mail.gmail.com","subject":"Re: Git Bug - diff in commit message.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-09T16:00:08Z","receivedAt":"2011-09-09T16:00:08Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Witten <mfwitten@gmail.com> writes:\n\n> It would appear that `git rebase' is in fact producing patches with\n> `git format-patch' and then applying the resulting patches with `git\n> am', which gets confused by your inline diff; this can be clearly seen\n> in the `git-rebase--am[.sh]' file.\n>\n> Perhaps `git rebase' should be reimplemented to use `git cherry-pick',\n> or does that suffer from the same problem?\n\nI think it just is a simple matter of this one-liner.  We were already\nbending backwards to preserve the original message by not parsing and\nrunning stripspace the message in the output from mailinfo, and instead\nusing the log message from the original, but were still using the patch\ntext that came from mailinfo that was split incorrectly because it was\nfooled by the diff in the commit log message.\n\nIn the longer term, I think \"git-rebase--am.sh\" should be rewritten to\nhave format-patch avoid the cost of actually generating the patch text,\nand the \"mailinfo\" call that comes above the context shown in this patch\nshould be made conditional---when using \"am\" for rebasing we do not really\ncare anything but the commit object names, and everything else is figured\nout from the commit this codepath.\n\n git-am.sh |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex 016b505..9a4cb2d 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -632,6 +632,7 @@ To restore the original branch and stop patching run \\\"\\$cmdline --abort\\\".\"\n \t\t\tsed -e '1,/^$/d' >\"$dotest/msg-clean\"\n \t\t\techo \"$commit\" > \"$dotest/original-commit\"\n \t\t\tget_author_ident_from_commit \"$commit\" > \"$dotest/author-script\"\n+\t\t\tgit diff-tree --root --binary -m --first-parent \"$commit\" >\"$dotest/patch\"\n \t\telse\n \t\t\t{\n \t\t\t\tsed -n '/^Subject/ s/Subject: //p' \"$dotest/info\"\n"},{"id":"177415","messageId":"7v62jvdthi.fsf@alter.siamese.dyndns.org","threadId":"28337","inReplyTo":"7vpqj9vh87.fsf@alter.siamese.dyndns.org","subject":"Re: Git Bug - diff in commit message.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-11T23:00:09Z","receivedAt":"2011-10-11T23:00:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> In the longer term, I think \"git-rebase--am.sh\" should be rewritten to\n> have format-patch avoid the cost of actually generating the patch text,\n> and the \"mailinfo\" call that comes above the context shown in this patch\n> should be made conditional---when using \"am\" for rebasing we do not really\n> care anything but the commit object names, and everything else is figured\n> out from the commit this codepath.\n\nAnd here is a quick hack to do that using \"log --cherry-pick --right-only\".\n\n git-am.sh         |   44 ++++++++++++++++++++++++--------------------\n git-rebase--am.sh |   12 +++++++++---\n 2 files changed, 33 insertions(+), 23 deletions(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex 9042432..b79ccc5 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -641,32 +641,36 @@ do\n \t# by the user, or the user can tell us to do so by --resolved flag.\n \tcase \"$resume\" in\n \t'')\n-\t\tgit mailinfo $keep $no_inbody_headers $scissors $utf8 \"$dotest/msg\" \"$dotest/patch\" \\\n-\t\t\t<\"$dotest/$msgnum\" >\"$dotest/info\" ||\n-\t\t\tstop_here $this\n-\n-\t\t# skip pine's internal folder data\n-\t\tsane_grep '^Author: Mail System Internal Data$' \\\n-\t\t\t<\"$dotest\"/info >/dev/null &&\n-\t\t\tgo_next && continue\n-\n-\t\ttest -s \"$dotest/patch\" || {\n-\t\t\teval_gettextln \"Patch is empty.  Was it split wrong?\n-If you would prefer to skip this patch, instead run \\\"\\$cmdline --skip\\\".\n-To restore the original branch and stop patching run \\\"\\$cmdline --abort\\\".\"\n-\t\t\tstop_here $this\n-\t\t}\n \t\trm -f \"$dotest/original-commit\" \"$dotest/author-script\"\n-\t\tif test -f \"$dotest/rebasing\" &&\n+\n+\t\tif test -f \"$dotest/rebasing\"\n+\t\tthen\n \t\t\tcommit=$(sed -e 's/^From \\([0-9a-f]*\\) .*/\\1/' \\\n \t\t\t\t-e q \"$dotest/$msgnum\") &&\n-\t\t\ttest \"$(git cat-file -t \"$commit\")\" = commit\n-\t\tthen\n+\t\t\ttest \"$(git cat-file -t \"$commit\")\" = commit || {\n+\t\t\t\tstop_here $this\n+\t\t\t}\n \t\t\tgit cat-file commit \"$commit\" |\n \t\t\tsed -e '1,/^$/d' >\"$dotest/msg-clean\"\n-\t\t\techo \"$commit\" > \"$dotest/original-commit\"\n-\t\t\tget_author_ident_from_commit \"$commit\" > \"$dotest/author-script\"\n+\t\t\techo \"$commit\" >\"$dotest/original-commit\"\n+\t\t\tget_author_ident_from_commit \"$commit\" >\"$dotest/author-script\"\n+\t\t\tgit diff-tree -p --root \"$commit\" >\"$dotest/patch\"\n \t\telse\n+\t\t\tgit mailinfo $keep $no_inbody_headers \\\n+\t\t\t$scissors $utf8 \"$dotest/msg\" \"$dotest/patch\" \\\n+\t\t\t<\"$dotest/$msgnum\" >\"$dotest/info\" ||\n+\t\t\tstop_here $this\n+\n+\t\t\t# skip pine's internal folder data\n+\t\t\tsane_grep '^Author: Mail System Internal Data$' \\\n+\t\t\t\t<\"$dotest\"/info >/dev/null &&\n+\t\t\t\tgo_next && continue\n+\t\t\ttest -s \"$dotest/patch\" || {\n+\t\t\t\teval_gettextln \"Patch is empty.  Was it split wrong?\n+If you would prefer to skip this patch, instead run \\\"\\$cmdline --skip\\\".\n+To restore the original branch and stop patching run \\\"\\$cmdline --abort\\\".\"\n+\t\t\t\tstop_here $this\n+\t\t\t}\n \t\t\t{\n \t\t\t\tsed -n '/^Subject/ s/Subject: //p' \"$dotest/info\"\n \t\t\t\techo\ndiff --git a/git-rebase--am.sh b/git-rebase--am.sh\n\n[...]\n"},{"id":"178253","messageId":"CAOeW2eG_muSdbWUaPG36=1-Ay6h6-4qHgWPNdjqY5Zpb52XisQ@mail.gmail.com","threadId":"28337","inReplyTo":"7v62jvdthi.fsf@alter.siamese.dyndns.org","subject":"Re: Git Bug - diff in commit message.","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-10-24T23:24:15Z","receivedAt":"2011-10-24T23:24:15Z","isPatch":false,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Hi,\n\nOn Tue, Oct 11, 2011 at 4:00 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> In the longer term, I think \"git-rebase--am.sh\" should be rewritten to\n>> have format-patch avoid the cost of actually generating the patch text,\n\nI'm (slowly) working on this.\n\n> And here is a quick hack to do that using \"log --cherry-pick --right-only\".\n\nThanks. I had done something similar. I have now adopted most of your\nchanges into my patches, but  I have a few questions.\n\n> +               if test -f \"$dotest/rebasing\"\n> +               then\n>                        commit=$(sed -e 's/^From \\([0-9a-f]*\\) .*/\\1/' \\\n>                                -e q \"$dotest/$msgnum\") &&\n> +                       test \"$(git cat-file -t \"$commit\")\" = commit || {\n> +                               stop_here $this\n> +                       }\n\nAre these braces needed?\n\n> +                       git diff-tree -p --root \"$commit\" >\"$dotest/patch\"\n\nIn your previous mail in this thread, this was\n\n+                       git diff-tree --root --binary -m\n--first-parent \"$commit\" >\"$dotest/patch\"\n\nI see why you dropped \"-m --first-parent\"; we should simply never\nreceive such patches to \"git-am --rebasing\". But why isn't --binary\nnecessary?\n\n> diff --git a/git-rebase--am.sh b/git-rebase--am.sh\n>\n> [...]\n\nWhy was this part left out? Did you get it to work using 'git log\n--cherry-pick --right-only'? I'm asking because I did not get it to\nwork with except for the case where $onto=upstream (basically when\n--onto is not given).\n\nMartin\n"},{"id":"178267","messageId":"7vr52140ve.fsf@alter.siamese.dyndns.org","threadId":"28337","inReplyTo":"CAOeW2eG_muSdbWUaPG36=1-Ay6h6-4qHgWPNdjqY5Zpb52XisQ@mail.gmail.com","subject":"Re: Git Bug - diff in commit message.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-25T14:11:33Z","receivedAt":"2011-10-25T14:11:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:\n\n>> And here is a quick hack to do that using \"log --cherry-pick --right-only\".\n>\n> Thanks. I had done something similar. I have now adopted most of your\n> changes into my patches, but  I have a few questions.\n\nIt was a quick hack and not necessarily well thought out.\n\n> Are these braces needed?\n\nProbably not.\n\n> In your previous mail in this thread, this was\n> ...\n> I see why you dropped \"-m --first-parent\"; we should simply never\n> receive such patches to \"git-am --rebasing\". But why isn't --binary\n> necessary?\n\nIt may be; I somehow thought we made it a no-op but didn't check.\n\n> Why was this part left out?\n\nNo particular reason other than I was not looking at the earlier \"how\nabout this\" patch closely. It may be needed, it may not be, I dunno, and I\ndidn't check when I wrote this second quick hack.\n\nThanks.\n"}]}