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