Add a check for common mistakes in commit messages. For now, it is
pretty rough and works only for -m/-F. It will be extended to work
in the interactive mode in the future.
---
 repoman/pym/repoman/actions.py                     |  74 +++++++++++++-
 repoman/pym/repoman/tests/commit/__init__.py       |   2 +
 repoman/pym/repoman/tests/commit/__test__.py       |   1 +
 repoman/pym/repoman/tests/commit/test_commitmsg.py | 109 +++++++++++++++++++++
 repoman/pym/repoman/tests/simple/test_simple.py    |   8 +-
 5 files changed, 189 insertions(+), 5 deletions(-)
 create mode 100644 repoman/pym/repoman/tests/commit/__init__.py
 create mode 100644 repoman/pym/repoman/tests/commit/__test__.py
 create mode 100644 repoman/pym/repoman/tests/commit/test_commitmsg.py

diff --git a/repoman/pym/repoman/actions.py b/repoman/pym/repoman/actions.py
index b76a6e466..57b528312 100644
--- a/repoman/pym/repoman/actions.py
+++ b/repoman/pym/repoman/actions.py
@@ -119,7 +119,16 @@ class Actions(object):
                        if commitmessage[:9].lower() in ("cat/pkg: ",):
                                commitmessage = self.msg_prefix() + 
commitmessage[9:]
 
-               if not commitmessage or not commitmessage.strip():
+               if commitmessage is not None and commitmessage.strip():
+                       res, expl = self.verify_commit_message(commitmessage)
+                       if not res:
+                               print(bad("RepoMan does not like your commit 
message:"))
+                               print(expl)
+                               if self.options.force:
+                                       print('(but proceeding due to --force)')
+                               else:
+                                       sys.exit(1)
+               else:
                        commitmessage = self.get_new_commit_message(qa_output)
 
                commitmessage = commitmessage.rstrip()
@@ -585,3 +594,66 @@ class Actions(object):
                        print("* no commit message?  aborting commit.")
                        sys.exit(1)
                return commitmessage
+
+       @staticmethod
+       def verify_commit_message(msg):
+               """
+               Check whether the message roughly complies with GLEP66. Returns
+               (True, None) if it does, (False, <explanation>) if it does not.
+               """
+
+               problems = []
+               paras = msg.strip().split('\n\n')
+               summary = paras.pop(0)
+
+               if ':' not in summary:
+                       problems.append('summary line must start with a logical 
unit name, e.g. "cat/pkg:"')
+               if '\n' in summary.strip():
+                       problems.append('commit message must start with a 
*single* line of summary, followed by empty line')
+               # accept 69 overall or unit+50, in case of very long package 
names
+               elif len(summary.strip()) > 69 and len(summary.split(':', 
1)[-1]) > 50:
+                       problems.append('summary line is too long (max 69 
characters)')
+
+               multiple_footers = False
+               gentoo_bug_used = False
+               bug_closes_without_url = False
+               body_too_long = False
+
+               footer_re = re.compile(r'^[\w-]+:')
+
+               found_footer = False
+               for p in paras:
+                       lines = p.splitlines()
+                       # if all lines look like footer material, we assume 
it's footer
+                       # else, we assume it's body text
+                       if all(footer_re.match(l) for l in lines if l.strip()):
+                               # multiple footer-like blocks?
+                               if found_footer:
+                                       multiple_footers = True
+                               found_footer = True
+                               for l in lines:
+                                       if l.startswith('Gentoo-Bug'):
+                                               gentoo_bug_used = True
+                                       elif l.startswith('Bug:') or 
l.startswith('Closes:'):
+                                               if 'http://' not in l and 
'https://' not in l:
+                                                       bug_closes_without_url 
= True
+                       else:
+                               for l in lines:
+                                       # we recommend wrapping at 72 but 
accept up to 80;
+                                       # do not complain if we have single 
word (usually
+                                       # it means very long URL)
+                                       if len(l.strip()) > 80 and 
len(l.split()) > 1:
+                                               body_too_long = True
+
+               if multiple_footers:
+                       problems.append('multiple footer-style blocks found, 
please combine them into one')
+               if gentoo_bug_used:
+                       problems.append('please replace Gentoo-Bug with GLEP 
66-compliant Bug/Closes')
+               if bug_closes_without_url:
+                       problems.append('Bug/Closes footers require full URL')
+               if body_too_long:
+                       problems.append('body lines should be wrapped at 72 
(max 80) characters')
+
+               if problems:
+                       return (False, '\n'.join('- %s' % x for x in problems))
+               return (True, None)
diff --git a/repoman/pym/repoman/tests/commit/__init__.py 
b/repoman/pym/repoman/tests/commit/__init__.py
new file mode 100644
index 000000000..d74fd94a7
--- /dev/null
+++ b/repoman/pym/repoman/tests/commit/__init__.py
@@ -0,0 +1,2 @@
+# Copyright 2011-2018 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
diff --git a/repoman/pym/repoman/tests/commit/__test__.py 
b/repoman/pym/repoman/tests/commit/__test__.py
new file mode 100644
index 000000000..8b1378917
--- /dev/null
+++ b/repoman/pym/repoman/tests/commit/__test__.py
@@ -0,0 +1 @@
+
diff --git a/repoman/pym/repoman/tests/commit/test_commitmsg.py 
b/repoman/pym/repoman/tests/commit/test_commitmsg.py
new file mode 100644
index 000000000..a734c1065
--- /dev/null
+++ b/repoman/pym/repoman/tests/commit/test_commitmsg.py
@@ -0,0 +1,109 @@
+# Copyright 2011-2018 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
+
+from repoman.actions import Actions
+from repoman.tests import TestCase
+
+
+class CommitMessageVerificationTest(TestCase):
+       def assertGood(self, commitmsg):
+               res, expl = Actions.verify_commit_message(commitmsg)
+               self.assertTrue(res,
+                               '''Commit message verification failed for:
+%s
+
+Error:
+%s''' % (commitmsg, expl))
+
+       def assertBad(self, commitmsg, reason_re):
+               res, expl = Actions.verify_commit_message(commitmsg)
+               self.assertFalse(res,
+                               '''Commit message verification succeeded 
unexpectedly, for:
+%s
+
+Expected: /%s/''' % (commitmsg, reason_re))
+               self.assertNotIn('\n', expl.strip(),
+                               '''Commit message verification returned 
multiple errors (one expected):
+%s
+
+Expected: /%s/
+Errors:
+%s''' % (commitmsg, reason_re, expl))
+               self.assertRegexpMatches(expl, reason_re,
+                               '''Commit message verification did not return 
expected error, for:
+%s
+
+Expected: /%s/
+Errors:
+%s''' % (commitmsg, reason_re, expl))
+
+       def test_summary_only(self):
+               self.assertGood('dev-foo/bar: Actually good commit message')
+
+       def test_summary_and_body(self):
+               self.assertGood('''dev-foo/bar: Good commit message
+
+Extended description goes here and is properly wrapped at 72 characters
+which is very nice and blah blah.
+
+Another paragraph for the sake of having one.''')
+
+       def test_summary_and_footer(self):
+               self.assertGood('''dev-foo/bar: Good commit message
+
+Closes: https://bugs.gentoo.org/NNNNNN''')
+
+       def test_summary_body_and_footer(self):
+               self.assertGood('''dev-foo/bar: Good commit message
+
+Extended description goes here and is properly wrapped at 72 characters
+which is very nice and blah blah.
+
+Another paragraph for the sake of having one.
+
+Closes: https://bugs.gentoo.org/NNNNNN''')
+
+       def test_summary_without_unit_name(self):
+               self.assertBad('Version bump', r'summary.*logical unit name')
+
+       def test_multiline_summary(self):
+               self.assertBad('''dev-foo/bar: Commit message with very long 
summary
+that got wrapped because of length''', r'single.*line.*summary')
+
+       def test_overlong_summary(self):
+               self.assertBad('dev-foo/bar: Commit message with very long 
summary \
+in a single line that should trigger an explicit error',
+                               r'summary.*too long')
+
+       def test_summary_with_very_long_package_name(self):
+               self.assertGood('dev-foo/foo-bar-bar-baz-bar-bar-foo-bar-bar-\
+baz-foo-baz-baz-foo: We do not fail because pkgname was long')
+
+       def test_multiple_footers(self):
+               self.assertBad('''dev-foo/bar: Good summary
+
+Bug: https://bugs.gentoo.org/NNNNNN
+
+Closes: https://github.com/gentoo/gentoo/pull/NNNN''', r'multiple footer')
+
+       def test_gentoo_bug(self):
+               self.assertBad('''dev-foo/bar: Good summary
+
+Gentoo-Bug: NNNNNN''', r'Gentoo-Bug')
+
+       def test_bug_with_number(self):
+               self.assertBad('''dev-foo/bar: Good summary
+
+Bug: NNNNNN''', r'Bug.*full URL')
+
+       def test_closes_with_number(self):
+               self.assertBad('''dev-foo/bar: Good summary
+
+Closes: NNNNNN''', r'Closes.*full URL')
+
+       def test_body_too_long(self):
+               self.assertBad('''dev-foo/bar: Good summary
+
+But the body is not wrapped properly and has very long lines that are \
+very hard to read and blah blah blah
+blah blah.''', r'body.*wrapped')
diff --git a/repoman/pym/repoman/tests/simple/test_simple.py 
b/repoman/pym/repoman/tests/simple/test_simple.py
index a24e0d5a3..3d7a70ad0 100644
--- a/repoman/pym/repoman/tests/simple/test_simple.py
+++ b/repoman/pym/repoman/tests/simple/test_simple.py
@@ -1,4 +1,4 @@
-# Copyright 2011-2015 Gentoo Foundation
+# Copyright 2011-2018 Gentoo Foundation
 # Distributed under the terms of the GNU General Public License v2
 
 import subprocess
@@ -194,13 +194,13 @@ class SimpleRepomanTestCase(TestCase):
                        ("", repoman_cmd + ("full", "-d")),
                        ("", cp_cmd + (test_ebuild, test_ebuild[:-8] + 
"2.ebuild")),
                        ("", git_cmd + ("add", test_ebuild[:-8] + "2.ebuild")),
-                       ("", repoman_cmd + ("commit", "-m", "bump to version 
2")),
+                       ("", repoman_cmd + ("commit", "-m", "cat/pkg: bump to 
version 2")),
                        ("", cp_cmd + (test_ebuild, test_ebuild[:-8] + 
"3.ebuild")),
                        ("", git_cmd + ("add", test_ebuild[:-8] + "3.ebuild")),
-                       ("dev-libs", repoman_cmd + ("commit", "-m", "bump to 
version 3")),
+                       ("dev-libs", repoman_cmd + ("commit", "-m", "cat/pkg: 
bump to version 3")),
                        ("", cp_cmd + (test_ebuild, test_ebuild[:-8] + 
"4.ebuild")),
                        ("", git_cmd + ("add", test_ebuild[:-8] + "4.ebuild")),
-                       ("dev-libs/A", repoman_cmd + ("commit", "-m", "bump to 
version 4")),
+                       ("dev-libs/A", repoman_cmd + ("commit", "-m", "cat/pkg: 
bump to version 4")),
                )
 
                env = {
-- 
2.16.2


Reply via email to