[PATCH v2 0/5] am: improve test coverage and touch up foreign patch parsing

[PATCH v2 0/5] am: improve test coverage and touch up foreign patch parsing

[Paul Tan]

This is a re-roll of [v1].

Previous versions:

[v1] http://thread.gmane.org/gmane.comp.version-control.git/271053

git-am is able to parse StGit and mercurial patches. However, there are no
regression tests in the test suite for these patch formats, and there are some
small bugs as well:

* the mercurial and stgit patch parsers does not support reading from stdin

* the mercurial patch parser parsed the patch date wrongly and git-am is thus
  unable to reconstruct the exact commit.

Some patches are based on Chris' patch series[1], which I've credited accordingly.

[1] http://thread.gmane.org/gmane.comp.version-control.git/256502


Paul Tan (5):
  t4150: test applying StGit patch
  am: teach StGit patch parser how to read from stdin
  t4150: test applying StGit series
  am: use gmtime() to parse mercurial patch date
  am: teach mercurial patch parser how to read from stdin

 git-am.sh     | 12 +++++----
 t/t4150-am.sh | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+), 5 deletions(-)

-- 
2.1.4

[PATCH v2 1/5] t4150: test applying StGit patch

[Paul Tan]

By default, an StGit patch separates the subject from the commit message
and headers as follows:

	$subject

	From: $author_name <$author_email>

	$message
	---
	$diffstats

We test git-am's ability to detect such a patch as an StGit patch, and
its ability to be able to extract the commit author, date and message
from such a patch.

Based-on-patch-by: Chris Packham <judge.packham@gmail.com>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 t/t4150-am.sh | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 306e6f3..0ead529 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -104,6 +104,18 @@ test_expect_success setup '
 		echo "X-Fake-Field: Line Three" &&
 		git format-patch --stdout first | sed -e "1d"
 	} > patch1-ws.eml &&
+	{
+		sed -ne "1p" msg &&
+		echo &&
+		echo "From: $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>" &&
+		echo "Date: $GIT_AUTHOR_DATE" &&
+		echo &&
+		sed -e "1,2d" msg &&
+		echo &&
+		echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" &&
+		echo "---" &&
+		git diff-tree --no-commit-id --stat -p second
+	} >patch1-stgit.eml &&
 
 	sed -n -e "3,\$p" msg >file &&
 	git add file &&
@@ -187,6 +199,16 @@ 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 stgit patch' '
+	rm -fr .git/rebase-apply &&
+	git checkout -f first &&
+	git am patch1-stgit.eml &&
+	test_path_is_missing .git/rebase-apply &&
+	git diff --exit-code second &&
+	test_cmp_rev second HEAD &&
+	test_cmp_rev second^ HEAD^
+'
+
 test_expect_success 'setup: new author and committer' '
 	GIT_AUTHOR_NAME="Another Thor" &&
 	GIT_AUTHOR_EMAIL="a.thor@example.com" &&
-- 
2.1.4

[PATCH v2 2/5] am: teach StGit patch parser how to read from stdin

[Paul Tan]

git-mailsplit, which splits mbox patches, will read the patch from stdin
when the filename is "-" or there are no files listed on the
command-line.

To be consistent with this behavior, teach the StGit patch parser to
read from stdin if the filename is "-" or no files are listed on the
command-line.

Based-on-patch-by: Chris Packham <judge.packham@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---

Notes:
    v2
    
    * Just pass the filename directly to perl. Hmm, I think we should add a
      "--" in front so that filenames that start with a dash won't be
      interpreted as a command-line switch by perl?

 git-am.sh     |  3 ++-
 t/t4150-am.sh | 10 ++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/git-am.sh b/git-am.sh
index 761befb..5ea2e4d 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -297,6 +297,7 @@ split_patches () {
 		;;
 	stgit)
 		this=0
+		test 0 -eq "$#" && set -- -
 		for stgit in "$@"
 		do
 			this=$(expr "$this" + 1)
@@ -318,7 +319,7 @@ split_patches () {
 					print "Subject: ", $_ ;
 					$subject = 1;
 				}
-			' < "$stgit" > "$dotest/$msgnum" || clean_abort
+			' -- "$stgit" >"$dotest/$msgnum" || clean_abort
 		done
 		echo "$this" > "$dotest/last"
 		this=
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 0ead529..51962e4 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -209,6 +209,16 @@ test_expect_success 'am applies stgit patch' '
 	test_cmp_rev second^ HEAD^
 '
 
+test_expect_success 'am --patch-format=stgit applies stgit patch' '
+	rm -fr .git/rebase-apply &&
+	git checkout -f first &&
+	git am --patch-format=stgit <patch1-stgit.eml &&
+	test_path_is_missing .git/rebase-apply &&
+	git diff --exit-code second &&
+	test_cmp_rev second HEAD &&
+	test_cmp_rev second^ HEAD^
+'
+
 test_expect_success 'setup: new author and committer' '
 	GIT_AUTHOR_NAME="Another Thor" &&
 	GIT_AUTHOR_EMAIL="a.thor@example.com" &&
-- 
2.1.4

[PATCH v2 3/5] t4150: test applying StGit series

[Paul Tan]

A StGit series is a directory containing a "series" file which begins
with the line:

	# This series applies on GIT commit XXXXX

where XXXXX is the commit ID that the patch series applies on. Every
following line names a patch in the directory to be applied.

Test that git-am, when given this "series" file, is able to detect it as
an StGit series and apply all the patches in the series.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 t/t4150-am.sh | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 51962e4..7aad8f8 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -116,6 +116,13 @@ test_expect_success setup '
 		echo "---" &&
 		git diff-tree --no-commit-id --stat -p second
 	} >patch1-stgit.eml &&
+	mkdir stgit-series &&
+	cp patch1-stgit.eml stgit-series/patch &&
+	{
+		echo "# This series applies on GIT commit $(git rev-parse first)" &&
+		echo "patch"
+	} >stgit-series/series &&
+
 
 	sed -n -e "3,\$p" msg >file &&
 	git add file &&
@@ -219,6 +226,16 @@ test_expect_success 'am --patch-format=stgit applies stgit patch' '
 	test_cmp_rev second^ HEAD^
 '
 
+test_expect_success 'am applies stgit series' '
+	rm -fr .git/rebase-apply &&
+	git checkout -f first &&
+	git am stgit-series/series &&
+	test_path_is_missing .git/rebase-apply &&
+	git diff --exit-code second &&
+	test_cmp_rev second HEAD &&
+	test_cmp_rev second^ HEAD^
+'
+
 test_expect_success 'setup: new author and committer' '
 	GIT_AUTHOR_NAME="Another Thor" &&
 	GIT_AUTHOR_EMAIL="a.thor@example.com" &&
-- 
2.1.4

[PATCH v2 4/5] am: use gmtime() to parse mercurial patch date

[Paul Tan]

An example of the line in a mercurial patch that specifies the date of
the commit would be:

	# Date 1433753301 25200

where the first number is the number of seconds since the unix epoch (in
UTC), and the second number is the offset of the timezone, in second s
west of UTC (negative if the timezone is east of UTC).

git-am uses localtime() to break down the first number into its
components (year, month, day, hours, minutes, seconds etc.). However,
the returned components are relative to the user's time zone. As a
result, if the user's time zone does not match the time zone specified
in the patch, the resulting commit will have the wrong author date.

Fix this by using gmtime() instead, which uses UTC instead of the user's
time zone.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 git-am.sh     |  6 +++---
 t/t4150-am.sh | 23 +++++++++++++++++++++++
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 5ea2e4d..f0b6c16 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -343,11 +343,11 @@ split_patches () {
 				elsif (/^\# User /) { s/\# User/From:/ ; print ; }
 				elsif (/^\# Date /) {
 					my ($hashsign, $str, $time, $tz) = split ;
-					$tz = sprintf "%+05d", (0-$tz)/36;
+					$tz_str = sprintf "%+05d", (0-$tz)/36;
 					print "Date: " .
 					      strftime("%a, %d %b %Y %H:%M:%S ",
-						       localtime($time))
-					      . "$tz\n";
+						       gmtime($time-$tz))
+					      . "$tz_str\n";
 				} elsif (/^\# /) { next ; }
 				else {
 					print "\n", $_ ;
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 7aad8f8..4beb4b3 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -122,6 +122,19 @@ test_expect_success setup '
 		echo "# This series applies on GIT commit $(git rev-parse first)" &&
 		echo "patch"
 	} >stgit-series/series &&
+	{
+		echo "# HG changeset patch" &&
+		echo "# User $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>" &&
+		echo "# Date $test_tick 25200" &&
+		echo "#      $(git show --pretty="%aD" -s second)" &&
+		echo "# Node ID $_z40" &&
+		echo "# Parent  $_z40" &&
+		cat msg &&
+		echo &&
+		echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" &&
+		echo &&
+		git diff-tree --no-commit-id -p second
+	} >patch1-hg.eml &&
 
 
 	sed -n -e "3,\$p" msg >file &&
@@ -236,6 +249,16 @@ test_expect_success 'am applies stgit series' '
 	test_cmp_rev second^ HEAD^
 '
 
+test_expect_success 'am applies hg patch' '
+	rm -fr .git/rebase-apply &&
+	git checkout -f first &&
+	git am patch1-hg.eml &&
+	test_path_is_missing .git/rebase-apply &&
+	git diff --exit-code second &&
+	test_cmp_rev second HEAD &&
+	test_cmp_rev second^ HEAD^
+'
+
 test_expect_success 'setup: new author and committer' '
 	GIT_AUTHOR_NAME="Another Thor" &&
 	GIT_AUTHOR_EMAIL="a.thor@example.com" &&
-- 
2.1.4

[PATCH v2 5/5] am: teach mercurial patch parser how to read from stdin

[Paul Tan]

git-mailsplit, which splits mbox patches, will read the patch from stdin
when the filename is "-" or there are no files listed on the
command-line.

To be consistent with this behavior, teach the mercurial patch parser to
read from stdin if the filename is "-" or no files are listed on the
command-line.

Based-on-patch-by: Chris Packham <judge.packham@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---

Notes:
    v2
    
    * Pass the filename directly to perl instead.

 git-am.sh     |  3 ++-
 t/t4150-am.sh | 10 ++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/git-am.sh b/git-am.sh
index f0b6c16..a8d33ef 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -327,6 +327,7 @@ split_patches () {
 		;;
 	hg)
 		this=0
+		test 0 -eq "$#" && set -- -
 		for hg in "$@"
 		do
 			this=$(( $this + 1 ))
@@ -353,7 +354,7 @@ split_patches () {
 					print "\n", $_ ;
 					$subject = 1;
 				}
-			' <"$hg" >"$dotest/$msgnum" || clean_abort
+			' -- "$hg" >"$dotest/$msgnum" || clean_abort
 		done
 		echo "$this" >"$dotest/last"
 		this=
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 4beb4b3..3ebafd9 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -259,6 +259,16 @@ test_expect_success 'am applies hg patch' '
 	test_cmp_rev second^ HEAD^
 '
 
+test_expect_success 'am --patch-format=hg applies hg patch' '
+	rm -fr .git/rebase-apply &&
+	git checkout -f first &&
+	git am --patch-format=hg <patch1-hg.eml &&
+	test_path_is_missing .git/rebase-apply &&
+	git diff --exit-code second &&
+	test_cmp_rev second HEAD &&
+	test_cmp_rev second^ HEAD^
+'
+
 test_expect_success 'setup: new author and committer' '
 	GIT_AUTHOR_NAME="Another Thor" &&
 	GIT_AUTHOR_EMAIL="a.thor@example.com" &&
-- 
2.1.4

Re: [PATCH v2 0/5] am: improve test coverage and touch up foreign patch parsing

[Junio C Hamano]

This round looks good to me.

Thanks all.