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 3366B1382C5 for ; Sat, 17 Feb 2018 13:18:16 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 1B973E09C6; Sat, 17 Feb 2018 13:18:15 +0000 (UTC) Received: from avasout06.plus.net (avasout06.plus.net [212.159.14.18]) (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 B4BF6E09C2 for ; Sat, 17 Feb 2018 13:18:14 +0000 (UTC) Received: from [192.168.6.147] ([212.159.46.162]) by smtp with ESMTP id n2NXehOnCVrdRn2NYeAc9L; Sat, 17 Feb 2018 13:18:12 +0000 X-CM-Score: 0.00 X-CNFS-Analysis: v=2.3 cv=XZynMrx5 c=1 sm=1 tr=0 a=RuViaDnnNG9rfPLW4VJocg==:117 a=RuViaDnnNG9rfPLW4VJocg==:17 a=13zjGPudsaEWiJwPRgMA:9 a=KqbVwreXT10MYGepIsQA:9 a=QEXdDO2ut3YA:10 a=Gqhr8QsC8kYaPk6uWp4A:9 a=ONNS8QRKHyMA:10 Subject: Re: [gentoo-portage-dev] [PATCH v2 1/3] repoman: Add commit message verification To: gentoo-portage-dev@lists.gentoo.org References: <20180217125648.1697-1-mgorny@gentoo.org> From: "M. J. Everitt" Openpgp: id=BA266E0525CFAB101523351B4C30334F93C22371 Message-ID: <750976a1-5cfc-4dd8-eed0-5b464240ec59@iee.org> Date: Sat, 17 Feb 2018 13:18:03 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-portage-dev@lists.gentoo.org Reply-to: gentoo-portage-dev@lists.gentoo.org MIME-Version: 1.0 In-Reply-To: <20180217125648.1697-1-mgorny@gentoo.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="HpvRd7p5YBOfCnemoaRRiFT8KBV4z6s0d" X-CMAE-Envelope: MS4wfNaTzC3/DTKNdUQ19noUEl24tS+37RT+42CNSvsLvUCQi4XyOMmPKXNiFQCayXWRw1PQe5CV7cyQmh44X+kzEkQGIUS9a2qM5JOZeB81KXiPacG9CMpk CnxGM/elrKfcFzxH0UklOIG1I8J1OL+OPA2KfLRq9Csup4jnhRyvJgtsF2ih/RCnHsOpoOgSmjVllQlvbC+pEOzacQSkim9Mg/Y= X-Archives-Salt: d6cffc4b-e002-4809-9ee5-418fa5ef71f5 X-Archives-Hash: 419fb2b3982c3113d21cc9dd486a7540 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --HpvRd7p5YBOfCnemoaRRiFT8KBV4z6s0d Content-Type: multipart/mixed; boundary="84q19o094SAi1EzivhNu0Ae0H6932VESu"; protected-headers="v1" From: "M. J. Everitt" To: gentoo-portage-dev@lists.gentoo.org Message-ID: <750976a1-5cfc-4dd8-eed0-5b464240ec59@iee.org> Subject: Re: [gentoo-portage-dev] [PATCH v2 1/3] repoman: Add commit message verification References: <20180217125648.1697-1-mgorny@gentoo.org> In-Reply-To: <20180217125648.1697-1-mgorny@gentoo.org> --84q19o094SAi1EzivhNu0Ae0H6932VESu Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Content-Language: en-GB On 17/02/18 12:56, Micha=C5=82 G=C3=B3rny wrote: > 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/simple/test_simple.py | 8 +-- > 4 files changed, 80 insertions(+), 5 deletions(-) > create mode 100644 repoman/pym/repoman/tests/commit/__init__.py > create mode 100644 repoman/pym/repoman/tests/commit/__test__.py > > diff --git a/repoman/pym/repoman/actions.py b/repoman/pym/repoman/actio= ns.py > index b76a6e466..91603865c 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 =3D self.msg_prefix() + commitmessage[9:] > =20 > - if not commitmessage or not commitmessage.strip(): > + if commitmessage is not None and commitmessage.strip(): > + res, expl =3D 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 =3D self.get_new_commit_message(qa_output) > =20 > commitmessage =3D commitmessage.rstrip() > @@ -585,3 +594,66 @@ class Actions(object): > print("* no commit message? aborting commit.") > sys.exit(1) > return commitmessage > + > + footer_re =3D re.compile(r'^[\w-]+:') > + > + @classmethod > + def verify_commit_message(cls, msg): > + """ > + Check whether the message roughly complies with GLEP66. Returns > + (True, None) if it does, (False, ) if it does not. > + """ > + > + problems =3D [] > + paras =3D msg.strip().split('\n\n') > + summary =3D 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 =3D False > + gentoo_bug_used =3D False > + bug_closes_without_url =3D False > + body_too_long =3D False > + > + found_footer =3D False > + for p in paras: > + lines =3D p.splitlines() > + # if all lines look like footer material, we assume it's footer > + # else, we assume it's body text > + if all(cls.footer_re.match(l) for l in lines if l.strip()): > + # multiple footer-like blocks? > + if found_footer: > + multiple_footers =3D True > + found_footer =3D True > + for l in lines: > + if l.startswith('Gentoo-Bug'): > + gentoo_bug_used =3D True > + elif l.startswith('Bug:') or l.startswith('Closes:'): > + if 'http://' not in l and 'https://' not in l: > + bug_closes_without_url =3D 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 =3D 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 B= ug/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) charac= ters') > + > + 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/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 > =20 > 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 versi= on 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 ver= sion 4")), > ) > =20 > env =3D { Might I suggest breaking out checks into a separate module? I think that hard-coding it all is likely to become a pain as time goes on, more checks get added, etc=C2=A0 ... --84q19o094SAi1EzivhNu0Ae0H6932VESu-- --HpvRd7p5YBOfCnemoaRRiFT8KBV4z6s0d Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJaiCuSAAoJEGPnxnn01DHdSdMP/2O/5TFuZ3fyObLJ09ePb+cE 9NpCiQRI5Vqr6cPiHLDmboIB2f5M2HaVWiMuILSOPKOx5PgtYE84eL6/zgY7HC3r 5A5ftz0u8cHigVccAGLMx1TfYjjYn/ec/+ryV0sYcEZt1VuPkGEtvCFTGSFor9ws 3ps7L8tCn5VF+yGipHkZUu5Q7+QDh74zoNAPWneMdEf2OL6Snpna81CUZIDaHbls Qte5dmUnNFNH9wLmkBwAhEdpibyKWnL5xcy3lDnBfqhc1oCejctHTXnlDVPFJIrA UqZt0CY7LXBWxAHjG/aU0IQg1O9XBPw4IUwpli6C6LT7vpKgYlLRmh3h6aVnkvUK IFswWyXTUzBppojzimfdT2h12bK71HjD9shPx+OFtZFk+RvzDXASDQjnpB2eG0uW ZpRr3Y+D5z4vKjStmXfaW+vIzs0HKVHwptBcuRT6ck13XFlWdNhug9ne17MHgSIg cABLXDBa9EKIbTCL+OgcCsHqBpHAQvf5ZRCKmDUznB6xf0zWNefl3yphRCk11uKS RxlArp9OV3y0JX9rmYmJx+m+LflGhwBn0I90QlLH7wYTAY1X7HE2e3mxjN4xgZGU V9f8zCW3a858wemWSBsedgi2KGMxJymAehYLP5n+pzHpj0tEJOhJrBuUz6WWvpVh Zyw2KPBZhTpa1xWkZNXl =/dA6 -----END PGP SIGNATURE----- --HpvRd7p5YBOfCnemoaRRiFT8KBV4z6s0d--