public inbox for gentoo-portage-dev@lists.gentoo.org
 help / color / mirror / Atom feed
From: "Michał Górny" <mgorny@gentoo.org>
To: gentoo-portage-dev@lists.gentoo.org
Cc: "Michał Górny" <mgorny@gentoo.org>
Subject: [gentoo-portage-dev] [PATCH v2 1/3] repoman: Add commit message verification
Date: Sat, 17 Feb 2018 13:56:46 +0100	[thread overview]
Message-ID: <20180217125648.1697-1-mgorny@gentoo.org> (raw)

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/actions.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 = 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
+
+	footer_re = 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, <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
+
+		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(cls.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/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.1



             reply	other threads:[~2018-02-17 12:57 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-17 12:56 Michał Górny [this message]
2018-02-17 12:56 ` [gentoo-portage-dev] [PATCH v2 2/3] repoman: Remove support for getting messages from stdin Michał Górny
2018-02-17 12:56 ` [gentoo-portage-dev] [PATCH v2 3/3] repoman: Verify commit messages when using EDITOR Michał Górny
2018-02-17 13:18 ` [gentoo-portage-dev] [PATCH v2 1/3] repoman: Add commit message verification M. J. Everitt
2018-02-18 16:28   ` Brian Dolbec
2018-02-18 16:35     ` M. J. Everitt
2018-02-18 17:11 ` Brian Dolbec
2018-02-18 18:47   ` Michał Górny
2018-02-19 14:27 ` Michał Górny

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180217125648.1697-1-mgorny@gentoo.org \
    --to=mgorny@gentoo.org \
    --cc=gentoo-portage-dev@lists.gentoo.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox