Skip to contentSkip to conversation

Special strings in commit messages

PATCH New Florian Weimer 2 participants 4 messages Oct 8, 2017 – Nov 18, 2017

lore
Share & tools
lore
#0 lore raw

I have a commit which looks like this:

$ git cat-file commit 4ca76eb7b47724c2444dfea7890fa8db4edd5762
tree c845be47a0653624b1984d0dc1a0b485b527811d
parent 9eee98638ef06149e17f94afaa357e3a9e296e69
author Florian Weimer <[email protected]> 1507481682 +0200
committer Florian Weimer <[email protected]> 1507481682 +0200

19: glibc-fedora-nis-rh188246.patch

From baba5d9461d4e8a581ac26fe4412ad783ffc73e7 Mon Sep 17 00:00:00 2001
From: Jakub Jelinek <[email protected]>
Date: Mon, 1 May 2006 08:02:53 +0000
Subject: [PATCH] Enable SETENT_BATCH_READ nis/nss option by default

* Mon May 1 2006 Jakub Jelinek <[email protected]> 2.4.90-4
- SETENT_BATCH_READ /etc/default/nss option for speeding up some usages of NIS+ (#188246)

This commit causes git rebase to fail, with this error:

fatal: could not parse .git/rebase-apply/0008

At this point, .git/rebase-apply/0008 contains this:

“
From baba5d9461d4e8a581ac26fe4412ad783ffc73e7 Mon Sep 17 00:00:00 2001
From: Jakub Jelinek <[email protected]>
Date: Mon, 1 May 2006 08:02:53 +0000
Subject: [PATCH] Enable SETENT_BATCH_READ nis/nss option by default

* Mon May 1 2006 Jakub Jelinek <[email protected]> 2.4.90-4
- SETENT_BATCH_READ /etc/default/nss option for speeding up some usages of NIS+ (#188246)

---
 nis/nss | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
nis/nss +0 -0
index 0ac6774a1ff29f012efaec9c4be1fcc3b83da7e8..d720e719267db5f741b67e7b98e4052e503c4333 100644

”

Followed by the diff. The preceding patch, .git/rebase-apply/0007, is:

“
[fweimer@oldenburg glibc-patches]$ cat .git/rebase-apply/0007 From 4ca76eb7b47724c2444dfea7890fa8db4edd5762 Mon Sep 17 00:00:00 2001
From: Florian Weimer <[email protected]>
Date: Sun, 8 Oct 2017 18:54:42 +0200
Subject: 19: glibc-fedora-nis-rh188246.patch

”

Based on strace output, something in git rebase calls git mailsplit, and it probably sees the "\nFrom " string and treats it as a start of a new mail message, and things go downhill from there.

I will escape "\nFrom " in commit messages (probably as "\n.From " or maybe "\n>From ", plus escaping for "\n."/"\n>" to make the encoding reversible), but I wonder if there is something else I need to escape while I'm at it.

Thanks,
Florian

#1 lore raw
Quoted 7 lines · Florian Weimer

Based on strace output, something in git rebase calls git mailsplit, and it probably sees the "\nFrom " string and treats it as a start of a new mail message, and things go downhill from there.

I will escape "\nFrom " in commit messages (probably as "\n.From " or maybe "\n>From ", plus escaping for "\n."/"\n>" to make the encoding reversible), but I wonder if there is something else I need to escape while I'm at it.

I suppose it's safe to start using mboxrd internally when there's little danger of mixing different git versions.

Totally untested (but passes "make test"), can you try this?

-----8<------
Subject: [PATCH] rebase: use mboxrd format to avoid split errors

The mboxrd format allows the use of embedded "From " lines in commit messages without being misinterpreted by mailsplit

---
 git-rebase--am.sh | 2 ++
 1 file changed, 2 insertions(+)
git-rebase--am.sh +2 -0
index 6e64d40d6f..14c50782e0 100644
@@ -53,6 +53,7 @@ else
 
 	git format-patch -k --stdout --full-index --cherry-pick --right-only \
 		--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \
+		--pretty=mboxrd \
 		$git_format_patch_opt \
 		"$revisions" ${restrict_revision+^$restrict_revision} \
 		>"$GIT_DIR/rebased-patches"
@@ -83,6 +84,7 @@ else
 	fi
 
 	git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" \
+		--patch-format=mboxrd \
 		$allow_rerere_autoupdate \
 		${gpg_sign_opt:+"$gpg_sign_opt"} <"$GIT_DIR/rebased-patches"
 	ret=$?
Signature · 1 line
EW
#2 lore raw
Quoted 13 lines · Eric Wong
git-rebase--am.sh +1 -0
index 6e64d40d6f..14c50782e0 100644
@@ -53,6 +53,7 @@ else
 
  	git format-patch -k --stdout --full-index --cherry-pick --right-only \
  		--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \
+		--pretty=mboxrd \
  		$git_format_patch_opt \
  		"$revisions" ${restrict_revision+^$restrict_revision} \
  		>"$GIT_DIR/rebased-patches"

git-rebase--am.sh else · line 90

Quoted 7 lines
git-rebase--am.sh +1 -0
@@ -83,6 +84,7 @@ else
  	fi
 
  	git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" \
+		--patch-format=mboxrd \
  		$allow_rerere_autoupdate \
  		${gpg_sign_opt:+"$gpg_sign_opt"} <"$GIT_DIR/rebased-patches"
  	ret=$?

My context is slightly different, but I added the mboxrd options manually to both commands, and it fixes my test case.

I'm still wondering if I have to be on the lookout for similar issues with different strings.

Thanks,
Florian

#3 lore raw

PATCH v2 1/1 rebase: use mboxrd format to avoid split errors

Sorry, I forgot about this for a while :x

Quoted 2 lines · Florian Weimer

git-rebase--am.sh

Quoted 1 line · Eric Wong
git-rebase--am.sh +0 -0

<snip, identical change below>

Quoted 5 lines

My context is slightly different, but I added the mboxrd options manually to both commands, and it fixes my test case.

I'm still wondering if I have to be on the lookout for similar issues with different strings.

I don't think theres other issues with different strings, "From " lines are the one known ambiguity problem with mbox and thus the reason mboxrd was created.

Below is an updated patch which adds a test case:

---------8<---------
Subject: [PATCH] rebase: use mboxrd format to avoid split errors

The mboxrd format allows the use of embedded "From " lines in commit messages without being misinterpreted by mailsplit

---
 git-rebase--am.sh |  2 ++
 t/t3400-rebase.sh | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+)
git-rebase--am.sh +2 -0
index 6e64d40d6f..14c50782e0 100644
@@ -53,6 +53,7 @@ else
 
 	git format-patch -k --stdout --full-index --cherry-pick --right-only \
 		--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \
+		--pretty=mboxrd \
 		$git_format_patch_opt \
 		"$revisions" ${restrict_revision+^$restrict_revision} \
 		>"$GIT_DIR/rebased-patches"
@@ -83,6 +84,7 @@ else
 	fi
 
 	git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" \
+		--patch-format=mboxrd \
 		$allow_rerere_autoupdate \
 		${gpg_sign_opt:+"$gpg_sign_opt"} <"$GIT_DIR/rebased-patches"
 	ret=$?
t/t3400-rebase.sh +22 -0
index f5fd15e559..8ac58d5ea5 100755
@@ -255,4 +255,26 @@ test_expect_success 'rebase commit with an ancient timestamp' '
 	grep "author .* 34567 +0600$" actual
 '
 
+test_expect_success 'rebase with "From " line in commit message' '
+	git checkout -b preserve-from master~1 &&
+	cat >From_.msg <<EOF &&
+Somebody embedded an mbox in a commit message
+
+This is from so-and-so:
+
+From a@b Mon Sep 17 00:00:00 2001
+From: John Doe <[email protected]>
+Date: Sat, 11 Nov 2017 00:00:00 +0000
+Subject: not this message
+
+something
+EOF
+	>From_ &&
+	git add From_ &&
+	git commit -F From_.msg &&
+	git rebase master &&
+	git log -1 --pretty=format:%B >out &&
+	test_cmp From_.msg out
+'
+
 test_done
Signature · 1 line
EW