threads / patch / 39631

v2, 5 partsam: improve test coverage and touch up foreign patch parsing

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

## tl;dr

7 messages between Jun 15, 2015 and Jun 15, 2015. Diffs are folded; open one to read it.

replies: 6people: 2as markdown or json

Paul Tan· Jun 15, 2015, 11:08 UTC · lore
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
Paul Tan· Jun 15, 2015, 11:08 UTC · re: Paul Tan · lore

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

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(+)
Show changes to t/t4150-am.sh +22 −0
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
Paul Tan· Jun 15, 2015, 11:08 UTC · re: Paul Tan · lore

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

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(-)
Show changes to 2 files +12 −1

git-am.sh, t/t4150-am.sh

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
Paul Tan· Jun 15, 2015, 11:08 UTC · re: Paul Tan · lore

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

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(+)
Show changes to t/t4150-am.sh +17 −0
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
Paul Tan· Jun 15, 2015, 11:08 UTC · re: Paul Tan · lore

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

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(-)
Show changes to 2 files +26 −3

git-am.sh, t/t4150-am.sh

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
Paul Tan· Jun 15, 2015, 11:08 UTC · re: Paul Tan · lore

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

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(-)
Show changes to 2 files +12 −1

git-am.sh, t/t4150-am.sh

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

← back to recent threads