1 |
commit: 59117bcb1793a6e99b8693eabb18cc891c12f962 |
2 |
Author: Michał Górny <mgorny <AT> gentoo <DOT> org> |
3 |
AuthorDate: Fri Feb 2 22:50:06 2018 +0000 |
4 |
Commit: Michał Górny <mgorny <AT> gentoo <DOT> org> |
5 |
CommitDate: Tue Feb 27 19:06:30 2018 +0000 |
6 |
URL: https://gitweb.gentoo.org/proj/portage.git/commit/?id=59117bcb |
7 |
|
8 |
repoman: Add commit message verification |
9 |
|
10 |
Add a check for common mistakes in commit messages. For now, it is |
11 |
pretty rough and works only for -m/-F. It will be extended to work |
12 |
in the interactive mode in the future. |
13 |
|
14 |
Reviewed-by: Brian Dolbec <dolsen <AT> gentoo.org> |
15 |
|
16 |
repoman/pym/repoman/actions.py | 74 +++++++++++++- |
17 |
repoman/pym/repoman/tests/commit/__init__.py | 2 + |
18 |
repoman/pym/repoman/tests/commit/__test__.py | 1 + |
19 |
repoman/pym/repoman/tests/commit/test_commitmsg.py | 109 +++++++++++++++++++++ |
20 |
repoman/pym/repoman/tests/simple/test_simple.py | 8 +- |
21 |
5 files changed, 189 insertions(+), 5 deletions(-) |
22 |
|
23 |
diff --git a/repoman/pym/repoman/actions.py b/repoman/pym/repoman/actions.py |
24 |
index b76a6e466..57b528312 100644 |
25 |
--- a/repoman/pym/repoman/actions.py |
26 |
+++ b/repoman/pym/repoman/actions.py |
27 |
@@ -119,7 +119,16 @@ class Actions(object): |
28 |
if commitmessage[:9].lower() in ("cat/pkg: ",): |
29 |
commitmessage = self.msg_prefix() + commitmessage[9:] |
30 |
|
31 |
- if not commitmessage or not commitmessage.strip(): |
32 |
+ if commitmessage is not None and commitmessage.strip(): |
33 |
+ res, expl = self.verify_commit_message(commitmessage) |
34 |
+ if not res: |
35 |
+ print(bad("RepoMan does not like your commit message:")) |
36 |
+ print(expl) |
37 |
+ if self.options.force: |
38 |
+ print('(but proceeding due to --force)') |
39 |
+ else: |
40 |
+ sys.exit(1) |
41 |
+ else: |
42 |
commitmessage = self.get_new_commit_message(qa_output) |
43 |
|
44 |
commitmessage = commitmessage.rstrip() |
45 |
@@ -585,3 +594,66 @@ class Actions(object): |
46 |
print("* no commit message? aborting commit.") |
47 |
sys.exit(1) |
48 |
return commitmessage |
49 |
+ |
50 |
+ @staticmethod |
51 |
+ def verify_commit_message(msg): |
52 |
+ """ |
53 |
+ Check whether the message roughly complies with GLEP66. Returns |
54 |
+ (True, None) if it does, (False, <explanation>) if it does not. |
55 |
+ """ |
56 |
+ |
57 |
+ problems = [] |
58 |
+ paras = msg.strip().split('\n\n') |
59 |
+ summary = paras.pop(0) |
60 |
+ |
61 |
+ if ':' not in summary: |
62 |
+ problems.append('summary line must start with a logical unit name, e.g. "cat/pkg:"') |
63 |
+ if '\n' in summary.strip(): |
64 |
+ problems.append('commit message must start with a *single* line of summary, followed by empty line') |
65 |
+ # accept 69 overall or unit+50, in case of very long package names |
66 |
+ elif len(summary.strip()) > 69 and len(summary.split(':', 1)[-1]) > 50: |
67 |
+ problems.append('summary line is too long (max 69 characters)') |
68 |
+ |
69 |
+ multiple_footers = False |
70 |
+ gentoo_bug_used = False |
71 |
+ bug_closes_without_url = False |
72 |
+ body_too_long = False |
73 |
+ |
74 |
+ footer_re = re.compile(r'^[\w-]+:') |
75 |
+ |
76 |
+ found_footer = False |
77 |
+ for p in paras: |
78 |
+ lines = p.splitlines() |
79 |
+ # if all lines look like footer material, we assume it's footer |
80 |
+ # else, we assume it's body text |
81 |
+ if all(footer_re.match(l) for l in lines if l.strip()): |
82 |
+ # multiple footer-like blocks? |
83 |
+ if found_footer: |
84 |
+ multiple_footers = True |
85 |
+ found_footer = True |
86 |
+ for l in lines: |
87 |
+ if l.startswith('Gentoo-Bug'): |
88 |
+ gentoo_bug_used = True |
89 |
+ elif l.startswith('Bug:') or l.startswith('Closes:'): |
90 |
+ if 'http://' not in l and 'https://' not in l: |
91 |
+ bug_closes_without_url = True |
92 |
+ else: |
93 |
+ for l in lines: |
94 |
+ # we recommend wrapping at 72 but accept up to 80; |
95 |
+ # do not complain if we have single word (usually |
96 |
+ # it means very long URL) |
97 |
+ if len(l.strip()) > 80 and len(l.split()) > 1: |
98 |
+ body_too_long = True |
99 |
+ |
100 |
+ if multiple_footers: |
101 |
+ problems.append('multiple footer-style blocks found, please combine them into one') |
102 |
+ if gentoo_bug_used: |
103 |
+ problems.append('please replace Gentoo-Bug with GLEP 66-compliant Bug/Closes') |
104 |
+ if bug_closes_without_url: |
105 |
+ problems.append('Bug/Closes footers require full URL') |
106 |
+ if body_too_long: |
107 |
+ problems.append('body lines should be wrapped at 72 (max 80) characters') |
108 |
+ |
109 |
+ if problems: |
110 |
+ return (False, '\n'.join('- %s' % x for x in problems)) |
111 |
+ return (True, None) |
112 |
|
113 |
diff --git a/repoman/pym/repoman/tests/commit/__init__.py b/repoman/pym/repoman/tests/commit/__init__.py |
114 |
new file mode 100644 |
115 |
index 000000000..d74fd94a7 |
116 |
--- /dev/null |
117 |
+++ b/repoman/pym/repoman/tests/commit/__init__.py |
118 |
@@ -0,0 +1,2 @@ |
119 |
+# Copyright 2011-2018 Gentoo Foundation |
120 |
+# Distributed under the terms of the GNU General Public License v2 |
121 |
|
122 |
diff --git a/repoman/pym/repoman/tests/commit/__test__.py b/repoman/pym/repoman/tests/commit/__test__.py |
123 |
new file mode 100644 |
124 |
index 000000000..8b1378917 |
125 |
--- /dev/null |
126 |
+++ b/repoman/pym/repoman/tests/commit/__test__.py |
127 |
@@ -0,0 +1 @@ |
128 |
+ |
129 |
|
130 |
diff --git a/repoman/pym/repoman/tests/commit/test_commitmsg.py b/repoman/pym/repoman/tests/commit/test_commitmsg.py |
131 |
new file mode 100644 |
132 |
index 000000000..a734c1065 |
133 |
--- /dev/null |
134 |
+++ b/repoman/pym/repoman/tests/commit/test_commitmsg.py |
135 |
@@ -0,0 +1,109 @@ |
136 |
+# Copyright 2011-2018 Gentoo Foundation |
137 |
+# Distributed under the terms of the GNU General Public License v2 |
138 |
+ |
139 |
+from repoman.actions import Actions |
140 |
+from repoman.tests import TestCase |
141 |
+ |
142 |
+ |
143 |
+class CommitMessageVerificationTest(TestCase): |
144 |
+ def assertGood(self, commitmsg): |
145 |
+ res, expl = Actions.verify_commit_message(commitmsg) |
146 |
+ self.assertTrue(res, |
147 |
+ '''Commit message verification failed for: |
148 |
+%s |
149 |
+ |
150 |
+Error: |
151 |
+%s''' % (commitmsg, expl)) |
152 |
+ |
153 |
+ def assertBad(self, commitmsg, reason_re): |
154 |
+ res, expl = Actions.verify_commit_message(commitmsg) |
155 |
+ self.assertFalse(res, |
156 |
+ '''Commit message verification succeeded unexpectedly, for: |
157 |
+%s |
158 |
+ |
159 |
+Expected: /%s/''' % (commitmsg, reason_re)) |
160 |
+ self.assertNotIn('\n', expl.strip(), |
161 |
+ '''Commit message verification returned multiple errors (one expected): |
162 |
+%s |
163 |
+ |
164 |
+Expected: /%s/ |
165 |
+Errors: |
166 |
+%s''' % (commitmsg, reason_re, expl)) |
167 |
+ self.assertRegexpMatches(expl, reason_re, |
168 |
+ '''Commit message verification did not return expected error, for: |
169 |
+%s |
170 |
+ |
171 |
+Expected: /%s/ |
172 |
+Errors: |
173 |
+%s''' % (commitmsg, reason_re, expl)) |
174 |
+ |
175 |
+ def test_summary_only(self): |
176 |
+ self.assertGood('dev-foo/bar: Actually good commit message') |
177 |
+ |
178 |
+ def test_summary_and_body(self): |
179 |
+ self.assertGood('''dev-foo/bar: Good commit message |
180 |
+ |
181 |
+Extended description goes here and is properly wrapped at 72 characters |
182 |
+which is very nice and blah blah. |
183 |
+ |
184 |
+Another paragraph for the sake of having one.''') |
185 |
+ |
186 |
+ def test_summary_and_footer(self): |
187 |
+ self.assertGood('''dev-foo/bar: Good commit message |
188 |
+ |
189 |
+Closes: https://bugs.gentoo.org/NNNNNN''') |
190 |
+ |
191 |
+ def test_summary_body_and_footer(self): |
192 |
+ self.assertGood('''dev-foo/bar: Good commit message |
193 |
+ |
194 |
+Extended description goes here and is properly wrapped at 72 characters |
195 |
+which is very nice and blah blah. |
196 |
+ |
197 |
+Another paragraph for the sake of having one. |
198 |
+ |
199 |
+Closes: https://bugs.gentoo.org/NNNNNN''') |
200 |
+ |
201 |
+ def test_summary_without_unit_name(self): |
202 |
+ self.assertBad('Version bump', r'summary.*logical unit name') |
203 |
+ |
204 |
+ def test_multiline_summary(self): |
205 |
+ self.assertBad('''dev-foo/bar: Commit message with very long summary |
206 |
+that got wrapped because of length''', r'single.*line.*summary') |
207 |
+ |
208 |
+ def test_overlong_summary(self): |
209 |
+ self.assertBad('dev-foo/bar: Commit message with very long summary \ |
210 |
+in a single line that should trigger an explicit error', |
211 |
+ r'summary.*too long') |
212 |
+ |
213 |
+ def test_summary_with_very_long_package_name(self): |
214 |
+ self.assertGood('dev-foo/foo-bar-bar-baz-bar-bar-foo-bar-bar-\ |
215 |
+baz-foo-baz-baz-foo: We do not fail because pkgname was long') |
216 |
+ |
217 |
+ def test_multiple_footers(self): |
218 |
+ self.assertBad('''dev-foo/bar: Good summary |
219 |
+ |
220 |
+Bug: https://bugs.gentoo.org/NNNNNN |
221 |
+ |
222 |
+Closes: https://github.com/gentoo/gentoo/pull/NNNN''', r'multiple footer') |
223 |
+ |
224 |
+ def test_gentoo_bug(self): |
225 |
+ self.assertBad('''dev-foo/bar: Good summary |
226 |
+ |
227 |
+Gentoo-Bug: NNNNNN''', r'Gentoo-Bug') |
228 |
+ |
229 |
+ def test_bug_with_number(self): |
230 |
+ self.assertBad('''dev-foo/bar: Good summary |
231 |
+ |
232 |
+Bug: NNNNNN''', r'Bug.*full URL') |
233 |
+ |
234 |
+ def test_closes_with_number(self): |
235 |
+ self.assertBad('''dev-foo/bar: Good summary |
236 |
+ |
237 |
+Closes: NNNNNN''', r'Closes.*full URL') |
238 |
+ |
239 |
+ def test_body_too_long(self): |
240 |
+ self.assertBad('''dev-foo/bar: Good summary |
241 |
+ |
242 |
+But the body is not wrapped properly and has very long lines that are \ |
243 |
+very hard to read and blah blah blah |
244 |
+blah blah.''', r'body.*wrapped') |
245 |
|
246 |
diff --git a/repoman/pym/repoman/tests/simple/test_simple.py b/repoman/pym/repoman/tests/simple/test_simple.py |
247 |
index a24e0d5a3..3d7a70ad0 100644 |
248 |
--- a/repoman/pym/repoman/tests/simple/test_simple.py |
249 |
+++ b/repoman/pym/repoman/tests/simple/test_simple.py |
250 |
@@ -1,4 +1,4 @@ |
251 |
-# Copyright 2011-2015 Gentoo Foundation |
252 |
+# Copyright 2011-2018 Gentoo Foundation |
253 |
# Distributed under the terms of the GNU General Public License v2 |
254 |
|
255 |
import subprocess |
256 |
@@ -194,13 +194,13 @@ class SimpleRepomanTestCase(TestCase): |
257 |
("", repoman_cmd + ("full", "-d")), |
258 |
("", cp_cmd + (test_ebuild, test_ebuild[:-8] + "2.ebuild")), |
259 |
("", git_cmd + ("add", test_ebuild[:-8] + "2.ebuild")), |
260 |
- ("", repoman_cmd + ("commit", "-m", "bump to version 2")), |
261 |
+ ("", repoman_cmd + ("commit", "-m", "cat/pkg: bump to version 2")), |
262 |
("", cp_cmd + (test_ebuild, test_ebuild[:-8] + "3.ebuild")), |
263 |
("", git_cmd + ("add", test_ebuild[:-8] + "3.ebuild")), |
264 |
- ("dev-libs", repoman_cmd + ("commit", "-m", "bump to version 3")), |
265 |
+ ("dev-libs", repoman_cmd + ("commit", "-m", "cat/pkg: bump to version 3")), |
266 |
("", cp_cmd + (test_ebuild, test_ebuild[:-8] + "4.ebuild")), |
267 |
("", git_cmd + ("add", test_ebuild[:-8] + "4.ebuild")), |
268 |
- ("dev-libs/A", repoman_cmd + ("commit", "-m", "bump to version 4")), |
269 |
+ ("dev-libs/A", repoman_cmd + ("commit", "-m", "cat/pkg: bump to version 4")), |
270 |
) |
271 |
|
272 |
env = { |