[RFC PATCHv3 0/4] am: patch-format

[RFC PATCHv3 0/4] am: patch-format

[Chris Packham]

I've re-ordered things in this round. Hopefully the first 3 patches are
uncontroversial, Junio has expressed reservations about the 4th. I
haven't looked at moving the changes into mailsplit yet or I could try
and update gitk to use something other than diff-tree or I could write
some external filter programs.

Chris Packham (4):
  am: avoid re-directing stdin twice
  t/am: add test for stgit patch format
  t/am: add tests for hg patch format
  am: add gitk patch format

 Documentation/git-am.txt |  3 +-
 git-am.sh                | 38 +++++++++++++++++++++++-
 t/t4150-am.sh            | 76 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 115 insertions(+), 2 deletions(-)

-- 
2.1.0.64.gc343089

[RFC PATCHv3 1/4] am: avoid re-directing stdin twice

[Chris Packham]

In check_patch_format we feed $1 to a block that attempts to determine
the patch format. Since we've already redirected $1 to stdin there is no
need to redirect it again when we invoke tr. This prevents the following
errors when invoking git am

  $ git am patch.patch
  tr: write error: Broken pipe
  tr: write error
  Patch format detection failed.

Cc: Stephen Boyd <bebarino@gmail.com>
Signed-off-by: Chris Packham <judge.packham@gmail.com>
---
Nothing new since http://article.gmane.org/gmane.comp.version-control.git/256425

 git-am.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-am.sh b/git-am.sh
index ee61a77..fade7f8 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -250,7 +250,7 @@ check_patch_format () {
 			# discarding the indented remainder of folded lines,
 			# and see if it looks like that they all begin with the
 			# header field names...
-			tr -d '\015' <"$1" |
+			tr -d '\015' |
 			sed -n -e '/^$/q' -e '/^[ 	]/d' -e p |
 			sane_egrep -v '^[!-9;-~]+:' >/dev/null ||
 			patch_format=mbox
-- 
2.1.0.64.gc343089

Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice

[Johannes Sixt]

Am 05.09.2014 12:06, schrieb Chris Packham:
> In check_patch_format we feed $1 to a block that attempts to determine
> the patch format. Since we've already redirected $1 to stdin there is no
> need to redirect it again when we invoke tr. This prevents the following
> errors when invoking git am
> 
>   $ git am patch.patch
>   tr: write error: Broken pipe
>   tr: write error
>   Patch format detection failed.
> 
> Cc: Stephen Boyd <bebarino@gmail.com>
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
> ---
> Nothing new since http://article.gmane.org/gmane.comp.version-control.git/256425
> 
>  git-am.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/git-am.sh b/git-am.sh
> index ee61a77..fade7f8 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -250,7 +250,7 @@ check_patch_format () {
>  			# discarding the indented remainder of folded lines,
>  			# and see if it looks like that they all begin with the
>  			# header field names...
> -			tr -d '\015' <"$1" |
> +			tr -d '\015' |
>  			sed -n -e '/^$/q' -e '/^[ 	]/d' -e p |
>  			sane_egrep -v '^[!-9;-~]+:' >/dev/null ||
>  			patch_format=mbox
> 

I think this change is wrong. This pipeline checks whether one of the
lines at the top of the file contains something that looks like an email
header. With your change, the first three lines would not be looked at
because they were already consumed earlier.

I wonder why tr (assuming it is *this* instance of tr) dies with a write
error instead of from a SIGPIPE. Is SIGPIPE ignored somewhere and then
the tr invocation inherits this "ignore SIGPIPE" setting?

The only thing your version changes is that tr writes a bit less text
into the pipe. Perhaps its just sufficient that the output fits into the
pipe buffer, and no error occurs anymore? Then the new version is not a
real fix: make the patch text a bit longer, and the error is back.

-- Hannes

Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice

[Chris Packham]

On Sat, Sep 6, 2014 at 8:26 AM, Johannes Sixt <j6t@kdbg.org> wrote:
> Am 05.09.2014 12:06, schrieb Chris Packham:
>> In check_patch_format we feed $1 to a block that attempts to determine
>> the patch format. Since we've already redirected $1 to stdin there is no
>> need to redirect it again when we invoke tr. This prevents the following
>> errors when invoking git am
>>
>>   $ git am patch.patch
>>   tr: write error: Broken pipe
>>   tr: write error
>>   Patch format detection failed.
>>
>> Cc: Stephen Boyd <bebarino@gmail.com>
>> Signed-off-by: Chris Packham <judge.packham@gmail.com>
>> ---
>> Nothing new since http://article.gmane.org/gmane.comp.version-control.git/256425
>>
>>  git-am.sh | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/git-am.sh b/git-am.sh
>> index ee61a77..fade7f8 100755
>> --- a/git-am.sh
>> +++ b/git-am.sh
>> @@ -250,7 +250,7 @@ check_patch_format () {
>>                       # discarding the indented remainder of folded lines,
>>                       # and see if it looks like that they all begin with the
>>                       # header field names...
>> -                     tr -d '\015' <"$1" |
>> +                     tr -d '\015' |
>>                       sed -n -e '/^$/q' -e '/^[       ]/d' -e p |
>>                       sane_egrep -v '^[!-9;-~]+:' >/dev/null ||
>>                       patch_format=mbox
>>
>
> I think this change is wrong. This pipeline checks whether one of the
> lines at the top of the file contains something that looks like an email
> header. With your change, the first three lines would not be looked at
> because they were already consumed earlier.
>
> I wonder why tr (assuming it is *this* instance of tr) dies with a write
> error instead of from a SIGPIPE. Is SIGPIPE ignored somewhere and then
> the tr invocation inherits this "ignore SIGPIPE" setting?
>
> The only thing your version changes is that tr writes a bit less text
> into the pipe. Perhaps its just sufficient that the output fits into the
> pipe buffer, and no error occurs anymore? Then the new version is not a
> real fix: make the patch text a bit longer, and the error is back.
>
> -- Hannes
>

I did notice some oddities when attempting to reproduce this issue.
They would be explained by the output fitting into the buffer. So yes
perhaps this solution has just changed enough so that it no longer
triggers on the particular patch I was testing with. It still seems a
bit funny that we start re-reading the input part way through
processing it.

Perhaps putting the tr outside the whole block would be a better solution?

Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice

[Junio C Hamano]

Chris Packham <judge.packham@gmail.com> writes:

> On Sat, Sep 6, 2014 at 8:26 AM, Johannes Sixt <j6t@kdbg.org> wrote:
>> Am 05.09.2014 12:06, schrieb Chris Packham:
>>> In check_patch_format we feed $1 to a block that attempts to determine
>>> the patch format. Since we've already redirected $1 to stdin there is no
>>> need to redirect it again when we invoke tr. This prevents the following
>>> errors when invoking git am
>>>
>>>   $ git am patch.patch
>>>   tr: write error: Broken pipe
>>>   tr: write error
>>>   Patch format detection failed.
>>> ...
>>
>> I wonder why tr (assuming it is *this* instance of tr) dies with a write
>> error instead of from a SIGPIPE. Is SIGPIPE ignored somewhere and then
>> the tr invocation inherits this "ignore SIGPIPE" setting?
>> ...
> Perhaps putting the tr outside the whole block would be a better solution?

Perhaps fixing the root cause of your (but not other people's) "tr"
failing is the right solution, no?

Also,

>>> -                     tr -d '\015' <"$1" |
>>>                       sed -n -e '/^$/q' -e '/^[       ]/d' -e p |
>>>                       sane_egrep -v '^[!-9;-~]+:' >/dev/null ||
>>>                       patch_format=mbox

as the tr is at an upsteam of this pipeline, it does not really
matter to the outcome if it gives a write-error error message or not
(the downstream sane_egrep would have decided, based on the data it
was given, if the payload was mbox format), so...

An easier workaround may be to update the sed script downstream of
tr.  It stops reading as soon as it finished to save cycles, and tr
should know that it does not have to produce any more output.  For a
broken tr installation, the sed script could be taught to slurp
everything in the message body (without passing it to downstream
sane_egrep, of course), and your "tr" would not see a broken pipe.

But that is still a workaround, not a fix, and an expensive one at
that.

Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Also,
>
>>>> -                     tr -d '\015' <"$1" |
>>>>                       sed -n -e '/^$/q' -e '/^[       ]/d' -e p |
>>>>                       sane_egrep -v '^[!-9;-~]+:' >/dev/null ||
>>>>                       patch_format=mbox
>
> as the tr is at an upsteam of this pipeline, it does not really
> matter to the outcome if it gives a write-error error message or not
> (the downstream sane_egrep would have decided, based on the data it
> was given, if the payload was mbox format), so...
>
> An easier workaround may be to update the sed script downstream of
> tr.  It stops reading as soon as it finished to save cycles, and tr
> should know that it does not have to produce any more output.  For a
> broken tr installation, the sed script could be taught to slurp
> everything in the message body (without passing it to downstream
> sane_egrep, of course), and your "tr" would not see a broken pipe.
>
> But that is still a workaround, not a fix, and an expensive one at
> that.

Redoing what e3f67d30 (am: fix patch format detection for
Thunderbird "Save As" emails, 2010-01-25) tried to do without
wasting a fork and a pipe may be a workable improvement.

I see Stephen who wrote the original "Thunderbird save-as" is
already on the Cc list.  How about doing it this way instead?

 git-am.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index ee61a77..9db3846 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -250,8 +250,7 @@ check_patch_format () {
 			# discarding the indented remainder of folded lines,
 			# and see if it looks like that they all begin with the
 			# header field names...
-			tr -d '\015' <"$1" |
-			sed -n -e '/^$/q' -e '/^[ 	]/d' -e p |
+			sed -n -e '/^$/q' -e '/^\r$/q' -e '/^[ 	]/d' -e p <"$1" |
 			sane_egrep -v '^[!-9;-~]+:' >/dev/null ||
 			patch_format=mbox
 		fi

Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Redoing what e3f67d30 (am: fix patch format detection for
> Thunderbird "Save As" emails, 2010-01-25) tried to do without
> wasting a fork and a pipe may be a workable improvement.
>
> I see Stephen who wrote the original "Thunderbird save-as" is
> already on the Cc list.  How about doing it this way instead?

Not that way, but more like this.

 git-am.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index ee61a77..32e3039 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -250,8 +250,7 @@ check_patch_format () {
 			# discarding the indented remainder of folded lines,
 			# and see if it looks like that they all begin with the
 			# header field names...
-			tr -d '\015' <"$1" |
-			sed -n -e '/^$/q' -e '/^[ 	]/d' -e p |
+			sed -n -e 's/\r$//' -e '/^$/q' -e '/^[ 	]/d' -e p |
 			sane_egrep -v '^[!-9;-~]+:' >/dev/null ||
 			patch_format=mbox
 		fi

Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice

[Stephen Boyd]

(replying from webmail interface)

On Fri, Sep 5, 2014 at 3:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Redoing what e3f67d30 (am: fix patch format detection for
>> Thunderbird "Save As" emails, 2010-01-25) tried to do without
>> wasting a fork and a pipe may be a workable improvement.
>>
>> I see Stephen who wrote the original "Thunderbird save-as" is
>> already on the Cc list.  How about doing it this way instead?
>

It was so long ago I can't even remember writing that patch. But I
googled the thread from 4.5 years ago and I see that you suggested we
use tr because \r is not portable[1].

> Not that way, but more like this.
>
>  git-am.sh | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/git-am.sh b/git-am.sh
> index ee61a77..32e3039 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -250,8 +250,7 @@ check_patch_format () {
>                         # discarding the indented remainder of folded lines,
>                         # and see if it looks like that they all begin with the
>                         # header field names...
> -                       tr -d '\015' <"$1" |
> -                       sed -n -e '/^$/q' -e '/^[       ]/d' -e p |
> +                       sed -n -e 's/\r$//' -e '/^$/q' -e '/^[  ]/d' -e p |
>                         sane_egrep -v '^[!-9;-~]+:' >/dev/null ||
>                         patch_format=mbox
>                 fi

[1] http://git.661346.n2.nabble.com/PATCH-am-fix-patch-format-detection-for-Thunderbird-quot-Save-As-quot-emails-td4184273.html

Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice

[Junio C Hamano]

Stephen Boyd <bebarino@gmail.com> writes:

>>> I see Stephen who wrote the original "Thunderbird save-as" is
>>> already on the Cc list.  How about doing it this way instead?
>
> It was so long ago I can't even remember writing that patch. But I
> googled the thread from 4.5 years ago and I see that you suggested we
> use tr because \r is not portable[1].

Hmph.  That's unfortunate that this may be one of those things that
even though it is in POSIX the real world prevents us from using it.

I wonder if things changed over the past four years, though.  Can
folks on OSX or BSD do a quick check?

Thanks.

Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice

[Torsten Bögershausen]

On 2014-09-06 09.34, Junio C Hamano wrote:
> Stephen Boyd <bebarino@gmail.com> writes:
> 
>>>> I see Stephen who wrote the original "Thunderbird save-as" is
>>>> already on the Cc list.  How about doing it this way instead?
>>
>> It was so long ago I can't even remember writing that patch. But I
>> googled the thread from 4.5 years ago and I see that you suggested we
>> use tr because \r is not portable[1].
> 
> Hmph.  That's unfortunate that this may be one of those things that
> even though it is in POSIX the real world prevents us from using it.
> 
> I wonder if things changed over the past four years, though.  Can
> folks on OSX or BSD do a quick check?
>
I may have missed the discussion, does this help?
"\r" can be used with tr, but not with sed:


tb@macosx:/tmp> cat ./xx.sh 
#!/bin/sh
which tr
printf "AB\rCD\n" | tr 'A\r\n\BCD' 'aRNbcd' | xxd
printf "E\rE" | tr -d '\r' | xxd
which sed
printf "AB\rCD\n" | sed -e  's/\r/R/g' | xxd
printf "E\rE" | sed -e 's/\r//g' | xxd

tb@macosx:/tmp> ./xx.sh 
/usr/bin/tr
0000000: 6162 5263 644e                           abRcdN
0000000: 4545                                     EE
/usr/bin/sed
0000000: 4142 0d43 440a                           AB.CD.
0000000: 450d 450a                                E.E.
tb@macosx:/tmp> 

[RFC PATCHv3 2/4] t/am: add test for stgit patch format

[Chris Packham]

This adds a tests which exercise the detection of the stgit format.
There is a current know breakage in that the code that deals with stgit
in split_patches can't handle reading from stdin.

Signed-off-by: Chris Packham <judge.packham@gmail.com>
---
 t/t4150-am.sh | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 5edb79a..dbe3475 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -103,6 +103,15 @@ test_expect_success setup '
 		echo "X-Fake-Field: Line Three" &&
 		git format-patch --stdout first | sed -e "1d"
 	} > patch1-ws.eml &&
+	{
+		echo "From: $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>" &&
+		echo &&
+		cat msg &&
+		echo &&
+		echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" &&
+		echo "---" &&
+		git diff-tree --stat -p second | sed -e "1d"
+	} > patch1-stgit.eml &&
 
 	sed -n -e "3,\$p" msg >file &&
 	git add file &&
@@ -186,6 +195,24 @@ test_expect_success 'am applies patch e-mail with preceding whitespace' '
 	test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)"
 '
 
+test_expect_success 'am applies patch generated by stgit' '
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout first &&
+	git am patch1-stgit.eml &&
+	test_path_is_missing .git/rebase-apply &&
+	git diff --exit-code second
+'
+
+test_expect_failure 'am applies patch using --patch-format=stgit' '
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout first &&
+	git am --patch-format=stgit <patch1-stgit.eml &&
+	test_path_is_missing .git/rebase-apply &&
+	git diff --exit-code second
+'
+
 test_expect_success 'setup: new author and committer' '
 	GIT_AUTHOR_NAME="Another Thor" &&
 	GIT_AUTHOR_EMAIL="a.thor@example.com" &&
-- 
2.1.0.64.gc343089

[RFC PATCHv3 3/4] t/am: add tests for hg patch format

[Chris Packham]

This adds a tests which exercise the detection of the hg format.  As
with stgit there is a current know breakage in where split_patches can't
handle reading from stdin with these patch formats.

Cc: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Signed-off-by: Chris Packham <judge.packham@gmail.com>
---
Note. I don't have access to a mercurial repository (plus I know next to
nothing about it) so the patch I've generated for the test was created
by looking at the format detection code. If someone can show me an
actual example of what mercurial produces that'd be helpful.

 t/t4150-am.sh | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index dbe3475..8ee81cf 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -112,6 +112,14 @@ test_expect_success setup '
 		echo "---" &&
 		git diff-tree --stat -p second | sed -e "1d"
 	} > patch1-stgit.eml &&
+	{
+		echo "# HG changeset patch"
+		echo "# User $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>"
+		echo &&
+		cat msg &&
+		echo "---" &&
+		git diff-tree --stat -p second | sed -e "1d"
+	} > patch1-hg.eml &&
 
 	sed -n -e "3,\$p" msg >file &&
 	git add file &&
@@ -213,6 +221,24 @@ test_expect_failure 'am applies patch using --patch-format=stgit' '
 	git diff --exit-code second
 '
 
+test_expect_success 'am applies patch generated by hg' '
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout first &&
+	git am patch1-hg.eml &&
+	test_path_is_missing .git/rebase-apply &&
+	git diff --exit-code second
+'
+
+test_expect_failure 'am applies patch using --patch-format=hg' '
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout first &&
+	git am --patch-format=hg <patch1-hg.eml &&
+	test_path_is_missing .git/rebase-apply &&
+	git diff --exit-code second
+'
+
 test_expect_success 'setup: new author and committer' '
 	GIT_AUTHOR_NAME="Another Thor" &&
 	GIT_AUTHOR_EMAIL="a.thor@example.com" &&
-- 
2.1.0.64.gc343089

[RFC PATCHv3 4/4] am: add gitk patch format

[Chris Packham]

Patches created using gitk's "write commit to file" functionality (which
uses 'git diff-tree -p --pretty' under the hood) need some massaging in
order to apply cleanly. This consists of dropping the 'commit' line
automatically determining the subject and removing leading whitespace.

Signed-off-by: Chris Packham <judge.packham@gmail.com>
---
This hasn't materially changed from the version Junio expressed
reservations about[1]. It solves my immediate problem but perhaps this
(as well as stgit and hg) belong as external filters in a pipeline
before git am. Or maybe mailsplit should absorb the functionality?

[1] - http://article.gmane.org/gmane.comp.version-control.git/256426

 Documentation/git-am.txt |  3 ++-
 git-am.sh                | 36 ++++++++++++++++++++++++++++++++++++
 t/t4150-am.sh            | 23 +++++++++++++++++++++++
 3 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 9adce37..b59d2b3 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -101,7 +101,8 @@ default.   You can use `--no-utf8` to override this.
 	By default the command will try to detect the patch format
 	automatically. This option allows the user to bypass the automatic
 	detection and specify the patch format that the patch(es) should be
-	interpreted as. Valid formats are mbox, stgit, stgit-series and hg.
+	interpreted as. Valid formats are mbox, stgit, stgit-series, hg and
+	gitk.
 
 -i::
 --interactive::
diff --git a/git-am.sh b/git-am.sh
index fade7f8..5d69c89 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -227,6 +227,9 @@ check_patch_format () {
 		"# HG changeset patch")
 			patch_format=hg
 			;;
+		'commit '*)
+			patch_format=gitk
+			;;
 		*)
 			# if the second line is empty and the third is
 			# a From, Author or Date entry, this is very
@@ -357,6 +360,39 @@ split_patches () {
 		this=
 		msgnum=
 		;;
+	gitk)
+		# These patches are generates with 'git diff-tree -p --pretty'
+		# we discard the 'commit' line, after that the first line not
+		# starting with 'Author:' or 'Date:' is the subject. We also
+		# need to strip leading whitespace from the message body.
+		this=0
+		for gitk in "$@"
+		do
+			this=$(expr "$this" + 1)
+			msgnum=$(printf "%0${prec}d" $this)
+			@@PERL@@ -ne 'BEGIN { $subject = 0; $diff = 0; }
+				if (!$diff) { s/^    // ; }
+				if ($subject > 1) { print ; }
+				elsif (/^commit\s.*$/) { next ; }
+				elsif (/^\s+$/) { next ; }
+				elsif (/^Author:/) { s/Author/From/ ; print ; }
+				elsif (/^Date:/) { print ;}
+				elsif (/^diff --git/) { $diff = 1 ; print ;}
+				elsif ($subject) {
+					$subject = 2 ;
+					print "\n" ;
+					print ;
+				} else {
+					print "Subject: ", $_ ;
+					$subject = 1;
+				}
+			' <"$gitk" >"$dotest/$msgnum" || clean_abort
+
+		done
+		echo "$this" >"$dotest/last"
+		this=
+		msgnum=
+		;;
 	*)
 		if test -n "$patch_format"
 		then
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 8ee81cf..5d4f7be 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -120,6 +120,7 @@ test_expect_success setup '
 		echo "---" &&
 		git diff-tree --stat -p second | sed -e "1d"
 	} > patch1-hg.eml &&
+	git diff-tree -p --pretty second >patch1-gitk.eml &&
 
 	sed -n -e "3,\$p" msg >file &&
 	git add file &&
@@ -239,6 +240,28 @@ test_expect_failure 'am applies patch using --patch-format=hg' '
 	git diff --exit-code second
 '
 
+test_expect_success 'am applies patch generated by gitk' '
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout first &&
+	git am patch1-gitk.eml &&
+	test_path_is_missing .git/rebase-apply &&
+	git diff --exit-code second &&
+	test "$(git rev-parse second)" = "$(git rev-parse HEAD)" &&
+	test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)"
+'
+
+test_expect_failure 'am applies patch using --patch-format=gitk' '
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout first &&
+	git am --patch-format=gitk <patch1-gitk.eml &&
+	test_path_is_missing .git/rebase-apply &&
+	git diff --exit-code second &&
+	test "$(git rev-parse second)" = "$(git rev-parse HEAD)" &&
+	test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)"
+'
+
 test_expect_success 'setup: new author and committer' '
 	GIT_AUTHOR_NAME="Another Thor" &&
 	GIT_AUTHOR_EMAIL="a.thor@example.com" &&
-- 
2.1.0.64.gc343089