threads / rfc / 37495

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

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

## tl;dr

13 messages between Sep 5, 2014 and Sep 6, 2014.

replies: 12people: 5as markdown or json

Chris Packham· Sep 5, 2014, 10:06 UTC · lore

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
Chris Packham· Sep 5, 2014, 10:06 UTC · re: Chris Packham · lore

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

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
Johannes Sixt· Sep 5, 2014, 20:26 UTC · re: Chris Packham · lore

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

Am 05.09.2014 12:06, schrieb Chris Packham:
Show 32 quoted lines
> 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
Chris Packham· Sep 5, 2014, 21:31 UTC · re: Johannes Sixt · lore

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

On Sat, Sep 6, 2014 at 8:26 AM, Johannes Sixt <j6t@kdbg.org> wrote:
Show 50 quoted lines
> 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?
Junio C Hamano· Sep 5, 2014, 22:00 UTC · re: Chris Packham · lore

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

Chris Packham <judge.packham@gmail.com> writes:
Show 18 quoted lines
> 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.

Junio C Hamano· Sep 5, 2014, 22:16 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <gitster@pobox.com> writes:
Show 21 quoted lines
> 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
Junio C Hamano· Sep 5, 2014, 22:25 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <gitster@pobox.com> writes:
Show 6 quoted lines
> 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
Stephen Boyd· Sep 5, 2014, 23:18 UTC · re: Junio C Hamano · lore

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

(replying from webmail interface)
On Fri, Sep 5, 2014 at 3:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 9 quoted lines
> 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].

Show 19 quoted lines
> 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
Junio C Hamano· Sep 6, 2014, 07:34 UTC · re: Stephen Boyd · lore

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

Stephen Boyd <bebarino@gmail.com> writes:
Show 6 quoted lines
>>> 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.
Torsten Bögershausen· Sep 6, 2014, 12:46 UTC · re: Junio C Hamano · lore

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

On 2014-09-06 09.34, Junio C Hamano wrote:
Show 15 quoted lines
> 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>

Chris Packham· Sep 5, 2014, 10:06 UTC · re: Chris Packham · lore

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

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
Chris Packham· Sep 5, 2014, 10:06 UTC · re: Chris Packham · lore

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

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
Chris Packham· Sep 5, 2014, 10:06 UTC · re: Chris Packham · lore

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

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

← back to recent threads