From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by finch.gentoo.org (Postfix) with ESMTPS id AC93E138331 for ; Tue, 27 Feb 2018 19:07:08 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id DBCC4E0966; Tue, 27 Feb 2018 19:07:07 +0000 (UTC) Received: from smtp.gentoo.org (mail.gentoo.org [IPv6:2001:470:ea4a:1:5054:ff:fec7:86e4]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id AA41DE0966 for ; Tue, 27 Feb 2018 19:07:07 +0000 (UTC) Received: from oystercatcher.gentoo.org (oystercatcher.gentoo.org [148.251.78.52]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.gentoo.org (Postfix) with ESMTPS id 89435335C2A for ; Tue, 27 Feb 2018 19:07:06 +0000 (UTC) Received: from localhost.localdomain (localhost [IPv6:::1]) by oystercatcher.gentoo.org (Postfix) with ESMTP id 131A91EA for ; Tue, 27 Feb 2018 19:07:05 +0000 (UTC) From: "Michał Górny" To: gentoo-commits@lists.gentoo.org Content-Transfer-Encoding: 8bit Content-type: text/plain; charset=UTF-8 Reply-To: gentoo-dev@lists.gentoo.org, "Michał Górny" Message-ID: <1519758390.59117bcb1793a6e99b8693eabb18cc891c12f962.mgorny@gentoo> Subject: [gentoo-commits] proj/portage:master commit in: repoman/pym/repoman/tests/simple/, repoman/pym/repoman/, ... X-VCS-Repository: proj/portage X-VCS-Files: repoman/pym/repoman/actions.py repoman/pym/repoman/tests/commit/__init__.py repoman/pym/repoman/tests/commit/__test__.py repoman/pym/repoman/tests/commit/test_commitmsg.py repoman/pym/repoman/tests/simple/test_simple.py X-VCS-Directories: repoman/pym/repoman/ repoman/pym/repoman/tests/commit/ repoman/pym/repoman/tests/simple/ X-VCS-Committer: mgorny X-VCS-Committer-Name: Michał Górny X-VCS-Revision: 59117bcb1793a6e99b8693eabb18cc891c12f962 X-VCS-Branch: master Date: Tue, 27 Feb 2018 19:07:05 +0000 (UTC) Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-commits@lists.gentoo.org X-Archives-Salt: ce74dc85-a56c-40c0-a1e8-cadf8498bc3d X-Archives-Hash: 2bb361bc7555b40d4421ba419b770f84 commit: 59117bcb1793a6e99b8693eabb18cc891c12f962 Author: Michał Górny gentoo org> AuthorDate: Fri Feb 2 22:50:06 2018 +0000 Commit: Michał Górny gentoo org> CommitDate: Tue Feb 27 19:06:30 2018 +0000 URL: https://gitweb.gentoo.org/proj/portage.git/commit/?id=59117bcb repoman: Add commit message verification 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. Reviewed-by: Brian Dolbec gentoo.org> 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(-) 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, ) 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 = {