{"thread":{"id":"46934","subject":"Special strings in commit messages","startedAt":"2017-10-08T18:24:45Z","lastAt":"2017-11-18T01:10:12Z","messageCount":4,"participants":["Florian Weimer","Eric Wong"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"329993","messageId":"f54bea17-c245-c644-f974-ca2ac84901c6@redhat.com","threadId":"46934","inReplyTo":null,"subject":"Special strings in commit messages","fromName":"Florian Weimer","fromEmail":"fweimer@redhat.com","sentAt":"2017-10-08T18:24:37Z","receivedAt":"2017-10-08T18:24:45Z","isPatch":false,"sender":{"key":"fweimer@redhat.com","avatar":null},"body":"I have a commit which looks like this:\n\n$ git cat-file commit 4ca76eb7b47724c2444dfea7890fa8db4edd5762\ntree c845be47a0653624b1984d0dc1a0b485b527811d\nparent 9eee98638ef06149e17f94afaa357e3a9e296e69\nauthor Florian Weimer <fweimer@redhat.com> 1507481682 +0200\ncommitter Florian Weimer <fweimer@redhat.com> 1507481682 +0200\n\n19: glibc-fedora-nis-rh188246.patch\n\n From baba5d9461d4e8a581ac26fe4412ad783ffc73e7 Mon Sep 17 00:00:00 2001\nFrom: Jakub Jelinek <jakub@redhat.com>\nDate: Mon, 1 May 2006 08:02:53 +0000\nSubject: [PATCH] Enable SETENT_BATCH_READ nis/nss option by default\n\n* Mon May  1 2006 Jakub Jelinek <jakub@redhat.com> 2.4.90-4\n- SETENT_BATCH_READ /etc/default/nss option for speeding up\n   some usages of NIS+ (#188246)\n\n\nThis commit causes git rebase to fail, with this error:\n\nfatal: could not parse .git/rebase-apply/0008\n\nAt this point, .git/rebase-apply/0008 contains this:\n\n“\n From baba5d9461d4e8a581ac26fe4412ad783ffc73e7 Mon Sep 17 00:00:00 2001\nFrom: Jakub Jelinek <jakub@redhat.com>\nDate: Mon, 1 May 2006 08:02:53 +0000\nSubject: [PATCH] Enable SETENT_BATCH_READ nis/nss option by default\n\n* Mon May  1 2006 Jakub Jelinek <jakub@redhat.com> 2.4.90-4\n- SETENT_BATCH_READ /etc/default/nss option for speeding up\n   some usages of NIS+ (#188246)\n---\n  nis/nss | 2 +-\n  1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/nis/nss b/nis/nss\nindex \n0ac6774a1ff29f012efaec9c4be1fcc3b83da7e8..d720e719267db5f741b67e7b98e4052e503c4333 \n100644\n”\n\nFollowed by the diff.  The preceding patch, .git/rebase-apply/0007, is:\n\n“\n[fweimer@oldenburg glibc-patches]$ cat .git/rebase-apply/0007\n From 4ca76eb7b47724c2444dfea7890fa8db4edd5762 Mon Sep 17 00:00:00 2001\nFrom: Florian Weimer <fweimer@redhat.com>\nDate: Sun, 8 Oct 2017 18:54:42 +0200\nSubject: 19: glibc-fedora-nis-rh188246.patch\n\n\n”\n\nBased on strace output, something in git rebase calls git mailsplit, and \nit probably sees the \"\\nFrom \" string and treats it as a start of a new \nmail message, and things go downhill from there.\n\nI will escape \"\\nFrom \" in commit messages (probably as \"\\n.From \" or \nmaybe \"\\n>From \", plus escaping for \"\\n.\"/\"\\n>\" to make the encoding \nreversible), but I wonder if there is something else I need to escape \nwhile I'm at it.\n\nThanks,\nFlorian\n"},{"id":"330003","messageId":"20171008213053.GA8568@starla","threadId":"46934","inReplyTo":"f54bea17-c245-c644-f974-ca2ac84901c6@redhat.com","subject":"Re: Special strings in commit messages","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2017-10-08T21:30:53Z","receivedAt":"2017-10-08T21:31:10Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Florian Weimer <fweimer@redhat.com> wrote:\n> Based on strace output, something in git rebase calls git mailsplit, and it\n> probably sees the \"\\nFrom \" string and treats it as a start of a new mail\n> message, and things go downhill from there.\n> \n> I will escape \"\\nFrom \" in commit messages (probably as \"\\n.From \" or maybe\n> \"\\n>From \", plus escaping for \"\\n.\"/\"\\n>\" to make the encoding reversible),\n> but I wonder if there is something else I need to escape while I'm at it.\n\nI suppose it's safe to start using mboxrd internally when\nthere's little danger of mixing different git versions.\n\nTotally untested (but passes \"make test\"), can you try this?\n\n-----8<------\nSubject: [PATCH] rebase: use mboxrd format to avoid split errors\n\nThe mboxrd format allows the use of embedded \"From \" lines in\ncommit messages without being misinterpreted by mailsplit\n\nReported-by: Florian Weimer <fweimer@redhat.com>\nSigned-off-by: Eric Wong <e@80x24.org>\n---\n git-rebase--am.sh | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/git-rebase--am.sh b/git-rebase--am.sh\nindex 6e64d40d6f..14c50782e0 100644\n--- a/git-rebase--am.sh\n+++ b/git-rebase--am.sh\n@@ -53,6 +53,7 @@ else\n \n \tgit format-patch -k --stdout --full-index --cherry-pick --right-only \\\n \t\t--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \\\n+\t\t--pretty=mboxrd \\\n \t\t$git_format_patch_opt \\\n \t\t\"$revisions\" ${restrict_revision+^$restrict_revision} \\\n \t\t>\"$GIT_DIR/rebased-patches\"\n@@ -83,6 +84,7 @@ else\n \tfi\n \n \tgit am $git_am_opt --rebasing --resolvemsg=\"$resolvemsg\" \\\n+\t\t--patch-format=mboxrd \\\n \t\t$allow_rerere_autoupdate \\\n \t\t${gpg_sign_opt:+\"$gpg_sign_opt\"} <\"$GIT_DIR/rebased-patches\"\n \tret=$?\n-- \nEW\n"},{"id":"330037","messageId":"6a036339-d57d-4cc8-acfc-4e1786763a80@redhat.com","threadId":"46934","inReplyTo":"20171008213053.GA8568@starla","subject":"Re: Special strings in commit messages","fromName":"Florian Weimer","fromEmail":"fweimer@redhat.com","sentAt":"2017-10-09T13:38:13Z","receivedAt":"2017-10-09T13:38:21Z","isPatch":false,"sender":{"key":"fweimer@redhat.com","avatar":null},"body":"On 10/08/2017 11:30 PM, Eric Wong wrote:\n> diff --git a/git-rebase--am.sh b/git-rebase--am.sh\n> index 6e64d40d6f..14c50782e0 100644\n> --- a/git-rebase--am.sh\n> +++ b/git-rebase--am.sh\n> @@ -53,6 +53,7 @@ else\n>   \n>   \tgit format-patch -k --stdout --full-index --cherry-pick --right-only \\\n>   \t\t--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \\\n> +\t\t--pretty=mboxrd \\\n>   \t\t$git_format_patch_opt \\\n>   \t\t\"$revisions\" ${restrict_revision+^$restrict_revision} \\\n>   \t\t>\"$GIT_DIR/rebased-patches\"\n> @@ -83,6 +84,7 @@ else\n>   \tfi\n>   \n>   \tgit am $git_am_opt --rebasing --resolvemsg=\"$resolvemsg\" \\\n> +\t\t--patch-format=mboxrd \\\n>   \t\t$allow_rerere_autoupdate \\\n>   \t\t${gpg_sign_opt:+\"$gpg_sign_opt\"} <\"$GIT_DIR/rebased-patches\"\n>   \tret=$?\n\nMy context is slightly different, but I added the mboxrd options \nmanually to both commands, and it fixes my test case.\n\nI'm still wondering if I have to be on the lookout for similar issues \nwith different strings.\n\nThanks,\nFlorian\n"},{"id":"332782","messageId":"20171118010116.GA17169@starla","threadId":"46934","inReplyTo":"6a036339-d57d-4cc8-acfc-4e1786763a80@redhat.com","subject":"[PATCH v2] rebase: use mboxrd format to avoid split errors","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2017-11-18T01:01:16Z","receivedAt":"2017-11-18T01:10:12Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Sorry, I forgot about this for a while :x\n\nFlorian Weimer <fweimer@redhat.com> wrote:\n> On 10/08/2017 11:30 PM, Eric Wong wrote:\n> >diff --git a/git-rebase--am.sh b/git-rebase--am.sh\n   <snip, identical change below>\n\n> My context is slightly different, but I added the mboxrd options manually to\n> both commands, and it fixes my test case.\n> \n> I'm still wondering if I have to be on the lookout for similar issues with\n> different strings.\n\nI don't think theres other issues with different strings,\n\"From \" lines are the one known ambiguity problem with mbox and\nthus the reason mboxrd was created.\n\nBelow is an updated patch which adds a test case:\n\n---------8<---------\nSubject: [PATCH] rebase: use mboxrd format to avoid split errors\n\nThe mboxrd format allows the use of embedded \"From \" lines in\ncommit messages without being misinterpreted by mailsplit\n\nReported-by: Florian Weimer <fweimer@redhat.com>\nSigned-off-by: Eric Wong <e@80x24.org>\n---\n git-rebase--am.sh |  2 ++\n t/t3400-rebase.sh | 22 ++++++++++++++++++++++\n 2 files changed, 24 insertions(+)\n\ndiff --git a/git-rebase--am.sh b/git-rebase--am.sh\nindex 6e64d40d6f..14c50782e0 100644\n--- a/git-rebase--am.sh\n+++ b/git-rebase--am.sh\n@@ -53,6 +53,7 @@ else\n \n \tgit format-patch -k --stdout --full-index --cherry-pick --right-only \\\n \t\t--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \\\n+\t\t--pretty=mboxrd \\\n \t\t$git_format_patch_opt \\\n \t\t\"$revisions\" ${restrict_revision+^$restrict_revision} \\\n \t\t>\"$GIT_DIR/rebased-patches\"\n@@ -83,6 +84,7 @@ else\n \tfi\n \n \tgit am $git_am_opt --rebasing --resolvemsg=\"$resolvemsg\" \\\n+\t\t--patch-format=mboxrd \\\n \t\t$allow_rerere_autoupdate \\\n \t\t${gpg_sign_opt:+\"$gpg_sign_opt\"} <\"$GIT_DIR/rebased-patches\"\n \tret=$?\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex f5fd15e559..8ac58d5ea5 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -255,4 +255,26 @@ test_expect_success 'rebase commit with an ancient timestamp' '\n \tgrep \"author .* 34567 +0600$\" actual\n '\n \n+test_expect_success 'rebase with \"From \" line in commit message' '\n+\tgit checkout -b preserve-from master~1 &&\n+\tcat >From_.msg <<EOF &&\n+Somebody embedded an mbox in a commit message\n+\n+This is from so-and-so:\n+\n+From a@b Mon Sep 17 00:00:00 2001\n+From: John Doe <nobody@example.com>\n+Date: Sat, 11 Nov 2017 00:00:00 +0000\n+Subject: not this message\n+\n+something\n+EOF\n+\t>From_ &&\n+\tgit add From_ &&\n+\tgit commit -F From_.msg &&\n+\tgit rebase master &&\n+\tgit log -1 --pretty=format:%B >out &&\n+\ttest_cmp From_.msg out\n+'\n+\n test_done\n-- \nEW\n"}]}