[RFC] parser: Detect series markers Mercurial produces
Stephen Finucane
stephen at that.guru
Tue Jan 24 09:28:33 AEDT 2017
From: Andrew Shadura <andrew.shadura at collabora.co.uk>
Unlike Git, Mercurial produces email subject lines of the
following format: "[PATCH M of N] ...". '(?:/| of )' pattern
matches both 'M/N' and 'M of N' formats.
Signed-off-by: Andrew Shadura <andrew.shadura at collabora.co.uk>
Signed-off-by: Stephen Finucane <stephen at that.guru>
---
So I've added the tests but it would appear your change isn't doing
what you expect. I suspect this is because Patchwork tokenizes all
subject prefixes, meaning this:
>>> x = '[PATCH 1 of 2]'
becomes this:
['PATCH', '1', 'of', '2']
and we don't have a '1 of 2' header to parse.
You're probably going to need to rework the 'clean_subject' function to
special-case this header via the regex. I'd do it, but I just don't have
bandwidth right now :(). Could you take a look?
Stephen
---
.gitignore | 2 +-
patchwork/parser.py | 4 +-
patchwork/tests/series/mercurial-cover-letter.mbox | 194 +++++++++++++++++++++
.../tests/series/mercurial-no-cover-letter.mbox | 154 ++++++++++++++++
patchwork/tests/test_parser.py | 2 +
patchwork/tests/test_series.py | 51 +++++-
6 files changed, 403 insertions(+), 4 deletions(-)
create mode 100644 patchwork/tests/series/mercurial-cover-letter.mbox
create mode 100644 patchwork/tests/series/mercurial-no-cover-letter.mbox
diff --git a/.gitignore b/.gitignore
index fd52c8c..c113cff 100644
--- a/.gitignore
+++ b/.gitignore
@@ -17,7 +17,7 @@ TAGS
# patch files
*.patch
*.diff
-*.mbox
+/*.mbox
# cscope files
cscope.*
diff --git a/patchwork/parser.py b/patchwork/parser.py
index 7e2067e..3ef3948 100644
--- a/patchwork/parser.py
+++ b/patchwork/parser.py
@@ -300,7 +300,7 @@ def _find_matching_prefix(subject_prefixes, regex):
def parse_series_marker(subject_prefixes):
"""Extract series markers from subject.
- Extract the markers of multi-patches series, i.e. 'x/n', from the
+ Extract the markers of multi-patches series, e.g. 'x/n', from the
provided subject series.
Args:
@@ -311,7 +311,7 @@ def parse_series_marker(subject_prefixes):
(x, n) if markers found, else (None, None)
"""
- regex = re.compile('^([0-9]+)/([0-9]+)$')
+ regex = re.compile(r'^([0-9]+)(?:/| of )([0-9]+)$')
m = _find_matching_prefix(subject_prefixes, regex)
if m:
return (int(m.group(1)), int(m.group(2)))
diff --git a/patchwork/tests/series/mercurial-cover-letter.mbox b/patchwork/tests/series/mercurial-cover-letter.mbox
new file mode 100644
index 0000000..34cd6f8
--- /dev/null
+++ b/patchwork/tests/series/mercurial-cover-letter.mbox
@@ -0,0 +1,194 @@
+From andrew at shadura.me Mon Jan 23 08:18:28 2017
+Return-Path: <andrew at shadura.me>
+Content-Type: text/plain; charset="us-ascii"
+MIME-Version: 1.0
+Subject: [PATCH 0 of 2] Sample Mercurial patches
+Message-Id: <patchbomb.1485159385 at nuevo>
+User-Agent: Mercurial-patchbomb/3.9.1
+Date: Mon, 23 Jan 2017 09:16:25 +0100
+From: Andrew Shadura <andrew at shadura.me>
+To: Stephen Finucane <stephen at that.guru>
+Cc: andrew.shadura at collabora.co.uk
+Content-Transfer-Encoding: 8bit
+
+Hi,
+
+On 22/01/17 22:28, Stephen Finucane wrote:
+> On Sun, 2017-01-22 at 01:44 +0100, Andrew Shadura wrote:
+>> Unlike Git, Mercurial produces email subject lines of the
+>> following format: "[PATCH M of N] ...". '(?:/| of )' pattern
+>> matches both 'M/N' and 'M of N' formats.
+>>
+>> Signed-off-by: Andrew Shadura <andrew.shadura at collabora.co.uk>
+>
+> This looks a-OK to me, but I would like to add an additional test case
+> in patchwork/tests/test_series.py to prevent this regressing if we
+> could. I can write the tests but I need some mbox files to validate
+> against. Could you supply me with some sample, Mercurial-produced patch
+> emails for this (email me directly)? A two patch series, with and
+> without a cover letter, would suffice (look at the files in
+> patchwork/tests/series for examples).
+
+Sure, here you go.
+
+P.S. Sorry for (possibly) spamming the mailing list by multiple patch submissions.
+It seems the list is moderated, right?
+
+From andrew at shadura.me Mon Jan 23 08:18:29 2017
+Return-Path: <andrew at shadura.me>
+Content-Type: text/plain; charset="us-ascii"
+MIME-Version: 1.0
+Subject: [PATCH 1 of 2] contrib: fix check-commit to not reject commits
+ from `hg sign` and `hg tag`
+X-Mercurial-Node: 2fb3ae89e4e1992e93826c0e3f04e4f965edb0ab
+X-Mercurial-Series-Index: 1
+X-Mercurial-Series-Total: 2
+Message-Id: <2fb3ae89e4e1992e9382.1485159386 at nuevo>
+X-Mercurial-Series-Id: <2fb3ae89e4e1992e9382.1485159386 at nuevo>
+In-Reply-To: <patchbomb.1485159385 at nuevo>
+References: <patchbomb.1485159385 at nuevo>
+User-Agent: Mercurial-patchbomb/3.9.1
+Date: Mon, 23 Jan 2017 09:16:26 +0100
+From: Andrew Shadura <andrew at shadura.me>
+To: Stephen Finucane <stephen at that.guru>
+Cc: andrew.shadura at collabora.co.uk
+Content-Transfer-Encoding: 8bit
+
+# HG changeset patch
+# User Augie Fackler <augie at google.com>
+# Date 1484800475 18000
+# Wed Jan 18 23:34:35 2017 -0500
+# Branch stable
+# Node ID 2fb3ae89e4e1992e93826c0e3f04e4f965edb0ab
+# Parent 94af7d0c812fe7d3a5651191685ca43e1a331814
+contrib: fix check-commit to not reject commits from `hg sign` and `hg tag`
+
+I'm tired of having a spurious red build every time we do a
+release. Fix it once and for all.
+
+diff --git a/contrib/check-commit b/contrib/check-commit
+--- a/contrib/check-commit
++++ b/contrib/check-commit
+@@ -59,6 +59,10 @@ def checkcommit(commit, node=None):
+ exitcode = 0
+ printed = node is None
+ hits = []
++ signtag = (afterheader +
++ r'Added (tag [^ ]+|signature) for changeset [a-f0-9]{12}')
++ if re.search(signtag, commit):
++ return 0
+ for exp, msg in errors:
+ for m in re.finditer(exp, commit):
+ end = m.end()
+diff --git a/tests/test-contrib-check-commit.t b/tests/test-contrib-check-commit.t
+--- a/tests/test-contrib-check-commit.t
++++ b/tests/test-contrib-check-commit.t
+@@ -30,6 +30,30 @@ A fine patch:
+ > EOF
+ $ cat patch-with-long-header.diff | $TESTDIR/../contrib/check-commit
+
++This would normally be against the rules, but it's okay because that's
++what tagging and signing looks like:
++
++ $ cat > creates-a-tag.diff << EOF
++ > # HG changeset patch
++ > # User Augie Fackler <raf at durin42.com>
++ > # Date 1484787778 18000
++ > # Wed Jan 18 20:02:58 2017 -0500
++ > # Branch stable
++ > # Node ID c177635e4acf52923bc3aa9f72a5b1ad1197b173
++ > # Parent a1dd2c0c479e0550040542e392e87bc91262517e
++ > Added tag 4.1-rc for changeset a1dd2c0c479e
++ >
++ > diff --git a/.hgtags b/.hgtags
++ > --- a/.hgtags
++ > +++ b/.hgtags
++ > @@ -150,3 +150,4 @@ 438173c415874f6ac653efc1099dec9c9150e90f
++ > eab27446995210c334c3d06f1a659e3b9b5da769 4.0
++ > b3b1ae98f6a0e14c1e1ba806a6c18e193b6dae5c 4.0.1
++ > e69874dc1f4e142746ff3df91e678a09c6fc208c 4.0.2
++ > +a1dd2c0c479e0550040542e392e87bc91262517e 4.1-rc
++ > EOF
++ $ $TESTDIR/../contrib/check-commit < creates-a-tag.diff
++
+ A patch with lots of errors:
+
+ $ cat > patch-with-long-header.diff << EOF
+
+From andrew at shadura.me Mon Jan 23 08:18:35 2017
+Return-Path: <andrew at shadura.me>
+Content-Type: text/plain; charset="us-ascii"
+MIME-Version: 1.0
+Subject: [PATCH 2 of 2] tests: work around FreeBSD's unzip having slightly
+ different output
+X-Mercurial-Node: b3d2e8cce78c04dbf6f20f0846b6ea8b622983c4
+X-Mercurial-Series-Index: 2
+X-Mercurial-Series-Total: 2
+Message-Id: <b3d2e8cce78c04dbf6f2.1485159387 at nuevo>
+X-Mercurial-Series-Id: <2fb3ae89e4e1992e9382.1485159386 at nuevo>
+In-Reply-To: <patchbomb.1485159385 at nuevo>
+References: <patchbomb.1485159385 at nuevo>
+User-Agent: Mercurial-patchbomb/3.9.1
+Date: Mon, 23 Jan 2017 09:16:27 +0100
+From: Andrew Shadura <andrew at shadura.me>
+To: Stephen Finucane <stephen at that.guru>
+Cc: andrew.shadura at collabora.co.uk
+Content-Transfer-Encoding: 8bit
+
+# HG changeset patch
+# User Augie Fackler <augie at google.com>
+# Date 1484801021 18000
+# Wed Jan 18 23:43:41 2017 -0500
+# Branch stable
+# Node ID b3d2e8cce78c04dbf6f20f0846b6ea8b622983c4
+# Parent 2fb3ae89e4e1992e93826c0e3f04e4f965edb0ab
+tests: work around FreeBSD's unzip having slightly different output
+
+According to man 1 unzip, this unzip appeared in FreeBSD 8.0. It's
+what comes as /usr/bin/unzip, so we may as well cater to it since it's
+easy.
+
+diff --git a/tests/test-archive.t b/tests/test-archive.t
+--- a/tests/test-archive.t
++++ b/tests/test-archive.t
+@@ -327,14 +327,11 @@ old file -- date clamped to 1980
+ $ hg add old
+ $ hg commit -m old
+ $ hg archive ../old.zip
+- $ unzip -l ../old.zip
++ $ unzip -l ../old.zip | grep -v -- ----- | egrep -v files$
+ Archive: ../old.zip
+ \s*Length.* (re)
+- *-----* (glob)
+ *172*80*00:00*old/.hg_archival.txt (glob)
+ *0*80*00:00*old/old (glob)
+- *-----* (glob)
+- \s*172\s+2 files (re)
+
+ show an error when a provided pattern matches no files
+
+diff --git a/tests/test-subrepo-recursion.t b/tests/test-subrepo-recursion.t
+--- a/tests/test-subrepo-recursion.t
++++ b/tests/test-subrepo-recursion.t
+@@ -321,10 +321,9 @@ Test archiving to zip file (unzip output
+
+ (unzip date formating is unstable, we do not care about it and glob it out)
+
+- $ unzip -l ../archive.zip
++ $ unzip -l ../archive.zip | grep -v -- ----- | egrep -v files$
+ Archive: ../archive.zip
+ Length [ ]* Date [ ]* Time [ ]* Name (re)
+- [\- ]* (re)
+ 172 [0-9:\- ]* .hg_archival.txt (re)
+ 10 [0-9:\- ]* .hgsub (re)
+ 45 [0-9:\- ]* .hgsubstate (re)
+@@ -333,8 +332,6 @@ Test archiving to zip file (unzip output
+ 45 [0-9:\- ]* foo/.hgsubstate (re)
+ 9 [0-9:\- ]* foo/y.txt (re)
+ 9 [0-9:\- ]* foo/bar/z.txt (re)
+- [\- ]* (re)
+- 303 [ ]* 8 files (re)
+
+ Test archiving a revision that references a subrepo that is not yet
+ cloned:
+
diff --git a/patchwork/tests/series/mercurial-no-cover-letter.mbox b/patchwork/tests/series/mercurial-no-cover-letter.mbox
new file mode 100644
index 0000000..9006ab8
--- /dev/null
+++ b/patchwork/tests/series/mercurial-no-cover-letter.mbox
@@ -0,0 +1,154 @@
+From andrew at shadura.me Mon Jan 23 08:19:08 2017
+Return-Path: <andrew at shadura.me>
+Content-Type: text/plain; charset="us-ascii"
+MIME-Version: 1.0
+Subject: [PATCH 1 of 2] contrib: fix check-commit to not reject commits
+ from `hg sign` and `hg tag`
+X-Mercurial-Node: 2fb3ae89e4e1992e93826c0e3f04e4f965edb0ab
+X-Mercurial-Series-Index: 1
+X-Mercurial-Series-Total: 2
+Message-Id: <2fb3ae89e4e1992e9382.1485159499 at nuevo>
+X-Mercurial-Series-Id: <2fb3ae89e4e1992e9382.1485159499 at nuevo>
+User-Agent: Mercurial-patchbomb/3.9.1
+Date: Mon, 23 Jan 2017 09:18:19 +0100
+From: Andrew Shadura <andrew at shadura.me>
+To: Stephen Finucane <stephen at that.guru>
+Content-Transfer-Encoding: 8bit
+
+# HG changeset patch
+# User Augie Fackler <augie at google.com>
+# Date 1484800475 18000
+# Wed Jan 18 23:34:35 2017 -0500
+# Branch stable
+# Node ID 2fb3ae89e4e1992e93826c0e3f04e4f965edb0ab
+# Parent 94af7d0c812fe7d3a5651191685ca43e1a331814
+contrib: fix check-commit to not reject commits from `hg sign` and `hg tag`
+
+I'm tired of having a spurious red build every time we do a
+release. Fix it once and for all.
+
+diff --git a/contrib/check-commit b/contrib/check-commit
+--- a/contrib/check-commit
++++ b/contrib/check-commit
+@@ -59,6 +59,10 @@ def checkcommit(commit, node=None):
+ exitcode = 0
+ printed = node is None
+ hits = []
++ signtag = (afterheader +
++ r'Added (tag [^ ]+|signature) for changeset [a-f0-9]{12}')
++ if re.search(signtag, commit):
++ return 0
+ for exp, msg in errors:
+ for m in re.finditer(exp, commit):
+ end = m.end()
+diff --git a/tests/test-contrib-check-commit.t b/tests/test-contrib-check-commit.t
+--- a/tests/test-contrib-check-commit.t
++++ b/tests/test-contrib-check-commit.t
+@@ -30,6 +30,30 @@ A fine patch:
+ > EOF
+ $ cat patch-with-long-header.diff | $TESTDIR/../contrib/check-commit
+
++This would normally be against the rules, but it's okay because that's
++what tagging and signing looks like:
++
++ $ cat > creates-a-tag.diff << EOF
++ > # HG changeset patch
++ > # User Augie Fackler <raf at durin42.com>
++ > # Date 1484787778 18000
++ > # Wed Jan 18 20:02:58 2017 -0500
++ > # Branch stable
++ > # Node ID c177635e4acf52923bc3aa9f72a5b1ad1197b173
++ > # Parent a1dd2c0c479e0550040542e392e87bc91262517e
++ > Added tag 4.1-rc for changeset a1dd2c0c479e
++ >
++ > diff --git a/.hgtags b/.hgtags
++ > --- a/.hgtags
++ > +++ b/.hgtags
++ > @@ -150,3 +150,4 @@ 438173c415874f6ac653efc1099dec9c9150e90f
++ > eab27446995210c334c3d06f1a659e3b9b5da769 4.0
++ > b3b1ae98f6a0e14c1e1ba806a6c18e193b6dae5c 4.0.1
++ > e69874dc1f4e142746ff3df91e678a09c6fc208c 4.0.2
++ > +a1dd2c0c479e0550040542e392e87bc91262517e 4.1-rc
++ > EOF
++ $ $TESTDIR/../contrib/check-commit < creates-a-tag.diff
++
+ A patch with lots of errors:
+
+ $ cat > patch-with-long-header.diff << EOF
+
+From andrew at shadura.me Mon Jan 23 08:19:08 2017
+Return-Path: <andrew at shadura.me>
+Content-Type: text/plain; charset="us-ascii"
+MIME-Version: 1.0
+Subject: [PATCH 2 of 2] tests: work around FreeBSD's unzip having slightly
+ different output
+X-Mercurial-Node: b3d2e8cce78c04dbf6f20f0846b6ea8b622983c4
+X-Mercurial-Series-Index: 2
+X-Mercurial-Series-Total: 2
+Message-Id: <b3d2e8cce78c04dbf6f2.1485159500 at nuevo>
+X-Mercurial-Series-Id: <2fb3ae89e4e1992e9382.1485159499 at nuevo>
+In-Reply-To: <2fb3ae89e4e1992e9382.1485159499 at nuevo>
+References: <2fb3ae89e4e1992e9382.1485159499 at nuevo>
+User-Agent: Mercurial-patchbomb/3.9.1
+Date: Mon, 23 Jan 2017 09:18:20 +0100
+From: Andrew Shadura <andrew at shadura.me>
+To: Stephen Finucane <stephen at that.guru>
+Content-Transfer-Encoding: 8bit
+
+# HG changeset patch
+# User Augie Fackler <augie at google.com>
+# Date 1484801021 18000
+# Wed Jan 18 23:43:41 2017 -0500
+# Branch stable
+# Node ID b3d2e8cce78c04dbf6f20f0846b6ea8b622983c4
+# Parent 2fb3ae89e4e1992e93826c0e3f04e4f965edb0ab
+tests: work around FreeBSD's unzip having slightly different output
+
+According to man 1 unzip, this unzip appeared in FreeBSD 8.0. It's
+what comes as /usr/bin/unzip, so we may as well cater to it since it's
+easy.
+
+diff --git a/tests/test-archive.t b/tests/test-archive.t
+--- a/tests/test-archive.t
++++ b/tests/test-archive.t
+@@ -327,14 +327,11 @@ old file -- date clamped to 1980
+ $ hg add old
+ $ hg commit -m old
+ $ hg archive ../old.zip
+- $ unzip -l ../old.zip
++ $ unzip -l ../old.zip | grep -v -- ----- | egrep -v files$
+ Archive: ../old.zip
+ \s*Length.* (re)
+- *-----* (glob)
+ *172*80*00:00*old/.hg_archival.txt (glob)
+ *0*80*00:00*old/old (glob)
+- *-----* (glob)
+- \s*172\s+2 files (re)
+
+ show an error when a provided pattern matches no files
+
+diff --git a/tests/test-subrepo-recursion.t b/tests/test-subrepo-recursion.t
+--- a/tests/test-subrepo-recursion.t
++++ b/tests/test-subrepo-recursion.t
+@@ -321,10 +321,9 @@ Test archiving to zip file (unzip output
+
+ (unzip date formating is unstable, we do not care about it and glob it out)
+
+- $ unzip -l ../archive.zip
++ $ unzip -l ../archive.zip | grep -v -- ----- | egrep -v files$
+ Archive: ../archive.zip
+ Length [ ]* Date [ ]* Time [ ]* Name (re)
+- [\- ]* (re)
+ 172 [0-9:\- ]* .hg_archival.txt (re)
+ 10 [0-9:\- ]* .hgsub (re)
+ 45 [0-9:\- ]* .hgsubstate (re)
+@@ -333,8 +332,6 @@ Test archiving to zip file (unzip output
+ 45 [0-9:\- ]* foo/.hgsubstate (re)
+ 9 [0-9:\- ]* foo/y.txt (re)
+ 9 [0-9:\- ]* foo/bar/z.txt (re)
+- [\- ]* (re)
+- 303 [ ]* 8 files (re)
+
+ Test archiving a revision that references a subrepo that is not yet
+ cloned:
+
diff --git a/patchwork/tests/test_parser.py b/patchwork/tests/test_parser.py
index 8d0eba0..4b14231 100644
--- a/patchwork/tests/test_parser.py
+++ b/patchwork/tests/test_parser.py
@@ -807,6 +807,8 @@ class SubjectTest(TestCase):
self.assertEqual(parse_series_marker(['bar']), (None, None))
self.assertEqual(parse_series_marker(['bar', '1/2']), (1, 2))
self.assertEqual(parse_series_marker(['bar', '0/12']), (0, 12))
+ self.assertEqual(parse_series_marker(['bar', '1 of 2']), (1, 2))
+ self.assertEqual(parse_series_marker(['bar', '0 of 12']), (0, 12))
def test_version(self):
self.assertEqual(parse_version('', []), 1)
diff --git a/patchwork/tests/test_series.py b/patchwork/tests/test_series.py
index 7377d91..1512d85 100644
--- a/patchwork/tests/test_series.py
+++ b/patchwork/tests/test_series.py
@@ -92,7 +92,10 @@ class _BaseTestCase(TestCase):
class BaseSeriesTest(_BaseTestCase):
- """Tests for a series without any revisions."""
+ """Tests for a series without any revisions.
+
+ All patches are generated using git-send-email(1).
+ """
def test_cover_letter(self):
"""Series with a cover letter.
@@ -171,6 +174,8 @@ class BaseSeriesTest(_BaseTestCase):
class RevisedSeriesTest(_BaseTestCase):
"""Tests for a series plus a single revision.
+ All patches are generated using git-send-email(1).
+
NOTE(stephenfin): In each sample mbox, it is necessary to ensure
the first series is placed before the revision. If not, they will
not be parsed correctly. This is OK as in practice it would very
@@ -375,6 +380,50 @@ class RevisedSeriesTest(_BaseTestCase):
self.assertSerialized(patches, [2, 2])
+class MercurialSeriesTest(_BaseTestCase):
+ """Tests for a series without any revisions.
+
+ All patches are generated using hg(1) email, provided via the Patchbomb
+ extension.
+ """
+
+ def test_cover_letter(self):
+ """Series with a cover letter.
+
+ Parse a series with a cover letter and two patches.
+
+ Input:
+
+ - [PATCH 0 of 2] Sample Mercurial patches
+ - [PATCH 1 of 2] contrib: fix check-commit to not reject
+ commits from `hg sign` and `hg tag`
+ - [PATCH 2 of 2] tests: work around FreeBSD's unzip having
+ slightly different output
+ """
+ covers, patches, comments = self._parse_mbox(
+ 'mercurial-cover-letter.mbox', [1, 2, 0])
+
+ self.assertSerialized(patches, [2])
+ self.assertSerialized(covers, [1])
+
+ def test_no_cover_letter(self):
+ """Series without a cover letter.
+
+ Parse a series with two patches but no cover letter.
+
+ Input:
+
+ - [PATCH 1 of 2] contrib: fix check-commit to not reject
+ commits from `hg sign` and `hg tag`
+ - [PATCH 2 of 2] tests: work around FreeBSD's unzip having
+ slightly different output
+ """
+ _, patches, _ = self._parse_mbox(
+ 'mercurial-no-cover-letter.mbox', [0, 2, 0])
+
+ self.assertSerialized(patches, [2])
+
+
class SeriesNameTestCase(TestCase):
def setUp(self):
--
2.9.3
More information about the Patchwork
mailing list