Merge lp:~james-w/tarmac/exit-status into lp:~ubuntuone-hackers/tarmac/trunk
- exit-status
- Merge into trunk
Proposed by
James Westby
Status: | Superseded |
---|---|
Proposed branch: | lp:~james-w/tarmac/exit-status |
Merge into: | lp:~ubuntuone-hackers/tarmac/trunk |
Diff against target: |
547 lines (+240/-196) 6 files modified
bin/tarmac (+3/-1) tarmac/bin/__init__.py (+1/-1) tarmac/bin/commands.py (+209/-191) tarmac/bin/registry.py (+2/-2) tarmac/plugins/command.py (+1/-1) tarmac/plugins/commit.py (+24/-0) |
To merge this branch: | bzr merge lp:~james-w/tarmac/exit-status |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Ubuntu One hackers | Pending | ||
Review via email: mp+140549@code.launchpad.net |
This proposal has been superseded by a proposal from 2012-12-18.
Commit message
Description of the change
Hi,
This allows the commands to set an exit status for the whole
tarmac script. We need it so that we can communicate some status
information to jenkins.
Thanks,
James
To post a comment you must log in.
lp:~james-w/tarmac/exit-status
updated
- 421. By James Westby
-
Merged break-up-do-merges into exit-status.
- 422. By James Westby
-
Merged break-up-do-merges into exit-status.
- 423. By James Westby
-
Merged break-up-do-merges into exit-status.
- 424. By James Westby
-
Merged break-up-do-merges into exit-status.
Unmerged revisions
- 424. By James Westby
-
Merged break-up-do-merges into exit-status.
- 423. By James Westby
-
Merged break-up-do-merges into exit-status.
- 422. By James Westby
-
Merged break-up-do-merges into exit-status.
- 421. By James Westby
-
Merged break-up-do-merges into exit-status.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'bin/tarmac' |
2 | --- bin/tarmac 2010-06-17 17:23:37 +0000 |
3 | +++ bin/tarmac 2012-12-18 21:22:38 +0000 |
4 | @@ -2,5 +2,7 @@ |
5 | # vim:filetype=python |
6 | '''Main tarmac script.''' |
7 | |
8 | +import sys |
9 | + |
10 | from tarmac.bin import main |
11 | -main() |
12 | +sys.exit(main()) |
13 | |
14 | === modified file 'tarmac/bin/__init__.py' |
15 | --- tarmac/bin/__init__.py 2010-09-02 15:18:07 +0000 |
16 | +++ tarmac/bin/__init__.py 2012-12-18 21:22:38 +0000 |
17 | @@ -27,4 +27,4 @@ |
18 | args = sys.argv[1:] |
19 | if not args: |
20 | args = ['help'] |
21 | - registry.run(args) |
22 | + return registry.run(args) |
23 | |
24 | === modified file 'tarmac/bin/commands.py' |
25 | --- tarmac/bin/commands.py 2012-09-05 18:08:50 +0000 |
26 | +++ tarmac/bin/commands.py 2012-12-18 21:22:38 +0000 |
27 | @@ -134,6 +134,194 @@ |
28 | help_commands(self.outf) |
29 | |
30 | |
31 | +def _get_mergable_proposals_for_branch(lp_branch, logger, imply_commit_message=False): |
32 | + """Return a list of the mergable proposals for the given branch.""" |
33 | + proposals = [] |
34 | + for entry in lp_branch.landing_candidates: |
35 | + logger.debug("Considering merge proposal: {0}".format(entry.web_link)) |
36 | + |
37 | + if entry.queue_status != u'Approved': |
38 | + logger.debug( |
39 | + " Skipping proposal: status is {0}, not " |
40 | + "'Approved'".format(entry.queue_status)) |
41 | + continue |
42 | + |
43 | + if (not imply_commit_message and not entry.commit_message): |
44 | + logger.debug( |
45 | + " Skipping proposal: proposal has no commit message") |
46 | + continue |
47 | + |
48 | + proposals.append(entry) |
49 | + return proposals |
50 | + |
51 | + |
52 | +def _get_reviews(proposal): |
53 | + """Get the set of reviews from the proposal.""" |
54 | + reviews = [] |
55 | + for vote in proposal.votes: |
56 | + if not vote.comment: |
57 | + continue |
58 | + else: |
59 | + reviews.append('%s;%s' % (vote.reviewer.display_name, |
60 | + vote.comment.vote)) |
61 | + |
62 | + if len(reviews) == 0: |
63 | + return None |
64 | + |
65 | + return reviews |
66 | + |
67 | + |
68 | +def setup(logger, debug=False, http_debug=False): |
69 | + if debug: |
70 | + set_up_debug_logging() |
71 | + logger.debug('Debug logging enabled') |
72 | + if http_debug: |
73 | + httplib2.debuglevel = 1 |
74 | + logger.debug('HTTP debugging enabled.') |
75 | + logger.debug('Loading plugins') |
76 | + load_plugins() |
77 | + logger.debug('Plugins loaded') |
78 | + |
79 | + |
80 | +def merge_proposals(target, proposals, logger, config, command): |
81 | + logger.debug('Firing tarmac_pre_merge hook') |
82 | + tarmac_hooks.fire('tarmac_pre_merge', |
83 | + command, target) |
84 | + |
85 | + statuses = [] |
86 | + try: |
87 | + for proposal in proposals: |
88 | + status = merge_proposal(target, proposal, logger, |
89 | + config, command) |
90 | + statuses.append(status) |
91 | + # This except is here because we need the else and can't have it |
92 | + # without an except as well. |
93 | + except: |
94 | + raise |
95 | + else: |
96 | + logger.debug('Firing tarmac_post_merge hook') |
97 | + tarmac_hooks.fire('tarmac_post_merge', |
98 | + command, target) |
99 | + finally: |
100 | + target.cleanup() |
101 | + return statuses |
102 | + |
103 | + |
104 | +def merge_proposal(target, proposal, logger, config, command): |
105 | + target.cleanup() |
106 | + logger.debug( |
107 | + u'Preparing to merge %(source_branch)s' % { |
108 | + 'source_branch': proposal.source_branch.web_link}) |
109 | + try: |
110 | + prerequisite = proposal.prerequisite_branch |
111 | + if prerequisite: |
112 | + merges = [x for x in prerequisite.landing_targets |
113 | + if x.target_branch == target.lp_branch and |
114 | + x.queue_status != u'Superseded'] |
115 | + if len(merges) == 0: |
116 | + raise TarmacMergeError( |
117 | + u'No proposals of prerequisite branch.', |
118 | + u'No proposals found for merge of %s ' |
119 | + u'into %s.' % ( |
120 | + prerequisite.web_link, |
121 | + target.lp_branch.web_link)) |
122 | + elif len(merges) > 1: |
123 | + raise TarmacMergeError( |
124 | + u'Too many proposals of prerequisite.', |
125 | + u'More than one proposal found for merge ' |
126 | + u'of %s into %s, which is not Superseded.' % ( |
127 | + prerequisite.web_link, |
128 | + target.lp_branch.web_link)) |
129 | + elif len(merges) == 1: |
130 | + if merges[0].queue_status != u'Merged': |
131 | + raise TarmacMergeError( |
132 | + u'Prerequisite not yet merged.', |
133 | + u'The prerequisite %s has not yet been ' |
134 | + u'merged into %s.' % ( |
135 | + prerequisite.web_link, |
136 | + target.lp_branch.web_link)) |
137 | + |
138 | + if not proposal.reviewed_revid: |
139 | + raise TarmacMergeError( |
140 | + u'No approved revision specified.') |
141 | + |
142 | + |
143 | + source = Branch.create( |
144 | + proposal.source_branch, config, target=target) |
145 | + |
146 | + source_bzr_branch = source.get_bzr_branch() |
147 | + approved = source_bzr_branch.revision_id_to_revno( |
148 | + str(proposal.reviewed_revid)) |
149 | + tip = source_bzr_branch.revno() |
150 | + |
151 | + if tip > approved: |
152 | + message = u'Unapproved changes made after approval' |
153 | + lp_comment = ( |
154 | + u'There are additional revisions which have not ' |
155 | + u'been approved in review. Please seek review and ' |
156 | + u'approval of these new revisions.') |
157 | + raise UnapprovedChanges(message, lp_comment) |
158 | + |
159 | + logger.debug( |
160 | + 'Merging %(source)s at revision %(revision)s' % { |
161 | + 'source': proposal.source_branch.web_link, |
162 | + 'revision': proposal.reviewed_revid}) |
163 | + |
164 | + target.merge(source, str(proposal.reviewed_revid)) |
165 | + |
166 | + logger.debug('Firing tarmac_pre_commit hook') |
167 | + tarmac_hooks.fire('tarmac_pre_commit', |
168 | + command, target, source, proposal) |
169 | + |
170 | + except TarmacMergeError, failure: |
171 | + logger.warn( |
172 | + u'Merging %(source)s into %(target)s failed: %(msg)s' % |
173 | + {'source': proposal.source_branch.web_link, |
174 | + 'target': proposal.target_branch.web_link, |
175 | + 'msg': str(failure)}) |
176 | + |
177 | + subject = u'Re: [Merge] %(source)s into %(target)s' % { |
178 | + "source": proposal.source_branch.display_name, |
179 | + "target": proposal.target_branch.display_name} |
180 | + |
181 | + if failure.comment: |
182 | + comment = failure.comment |
183 | + else: |
184 | + comment = str(failure) |
185 | + |
186 | + proposal.createComment(subject=subject, content=comment) |
187 | + try: |
188 | + proposal.setStatus( |
189 | + status=config.rejected_branch_status) |
190 | + except AttributeError: |
191 | + proposal.setStatus(status=u'Needs review') |
192 | + proposal.lp_save() |
193 | + return False |
194 | + |
195 | + except PointlessMerge: |
196 | + logger.warn( |
197 | + 'Merging %(source)s into %(target)s would be ' |
198 | + 'pointless.' % { |
199 | + 'source': proposal.source_branch.web_link, |
200 | + 'target': proposal.target_branch.web_link}) |
201 | + return False |
202 | + |
203 | + merge_url = get_review_url(proposal) |
204 | + revprops = {'merge_url': merge_url} |
205 | + |
206 | + commit_message = proposal.commit_message |
207 | + if commit_message is None and config.imply_commit_message: |
208 | + commit_message = proposal.description |
209 | + |
210 | + target.commit(commit_message, |
211 | + reviews=_get_reviews(proposal)) |
212 | + |
213 | + logger.debug('Firing tarmac_post_commit hook') |
214 | + tarmac_hooks.fire('tarmac_post_commit', |
215 | + command, target, source, proposal) |
216 | + return True |
217 | + |
218 | + |
219 | class cmd_merge(TarmacCommand): |
220 | '''Automatically merge approved merge proposal branches.''' |
221 | |
222 | @@ -143,7 +331,8 @@ |
223 | options.http_debug_option, |
224 | options.debug_option, |
225 | options.imply_commit_message_option, |
226 | - options.one_option] |
227 | + options.one_option, |
228 | + ] |
229 | |
230 | def _do_merges(self, branch_url): |
231 | |
232 | @@ -152,204 +341,29 @@ |
233 | self.logger.info('Not a valid branch: {0}'.format(branch_url)) |
234 | return |
235 | |
236 | - proposals = self._get_mergable_proposals_for_branch(lp_branch) |
237 | + proposals = _get_mergable_proposals_for_branch(lp_branch, self.logger, |
238 | + imply_commit_message=self.config.imply_commit_message) |
239 | |
240 | if not proposals: |
241 | self.logger.info( |
242 | 'No approved proposals found for %(branch_url)s' % { |
243 | 'branch_url': branch_url}) |
244 | - return |
245 | + return [] |
246 | + |
247 | + if self.config.one: |
248 | + proposals = proposals[0] |
249 | |
250 | target = Branch.create(lp_branch, self.config, create_tree=True) |
251 | - |
252 | - self.logger.debug('Firing tarmac_pre_merge hook') |
253 | - tarmac_hooks.fire('tarmac_pre_merge', |
254 | - self, target) |
255 | - |
256 | - try: |
257 | - for proposal in proposals: |
258 | - target.cleanup() |
259 | - self.logger.debug( |
260 | - u'Preparing to merge %(source_branch)s' % { |
261 | - 'source_branch': proposal.source_branch.web_link}) |
262 | - try: |
263 | - prerequisite = proposal.prerequisite_branch |
264 | - if prerequisite: |
265 | - merges = [x for x in prerequisite.landing_targets |
266 | - if x.target_branch == target.lp_branch and |
267 | - x.queue_status != u'Superseded'] |
268 | - if len(merges) == 0: |
269 | - raise TarmacMergeError( |
270 | - u'No proposals of prerequisite branch.', |
271 | - u'No proposals found for merge of %s ' |
272 | - u'into %s.' % ( |
273 | - prerequisite.web_link, |
274 | - target.lp_branch.web_link)) |
275 | - elif len(merges) > 1: |
276 | - raise TarmacMergeError( |
277 | - u'Too many proposals of prerequisite.', |
278 | - u'More than one proposal found for merge ' |
279 | - u'of %s into %s, which is not Superseded.' % ( |
280 | - prerequisite.web_link, |
281 | - target.lp_branch.web_link)) |
282 | - elif len(merges) == 1: |
283 | - if merges[0].queue_status != u'Merged': |
284 | - raise TarmacMergeError( |
285 | - u'Prerequisite not yet merged.', |
286 | - u'The prerequisite %s has not yet been ' |
287 | - u'merged into %s.' % ( |
288 | - prerequisite.web_link, |
289 | - target.lp_branch.web_link)) |
290 | - |
291 | - if not proposal.reviewed_revid: |
292 | - raise TarmacMergeError( |
293 | - u'No approved revision specified.') |
294 | - |
295 | - |
296 | - source = Branch.create( |
297 | - proposal.source_branch, self.config, target=target) |
298 | - |
299 | - source_bzr_branch = source.get_bzr_branch() |
300 | - approved = source_bzr_branch.revision_id_to_revno( |
301 | - str(proposal.reviewed_revid)) |
302 | - tip = source_bzr_branch.revno() |
303 | - |
304 | - if tip > approved: |
305 | - message = u'Unapproved changes made after approval' |
306 | - lp_comment = ( |
307 | - u'There are additional revisions which have not ' |
308 | - u'been approved in review. Please seek review and ' |
309 | - u'approval of these new revisions.') |
310 | - raise UnapprovedChanges(message, lp_comment) |
311 | - |
312 | - self.logger.debug( |
313 | - 'Merging %(source)s at revision %(revision)s' % { |
314 | - 'source': proposal.source_branch.web_link, |
315 | - 'revision': proposal.reviewed_revid}) |
316 | - |
317 | - target.merge(source, str(proposal.reviewed_revid)) |
318 | - |
319 | - self.logger.debug('Firing tarmac_pre_commit hook') |
320 | - tarmac_hooks.fire('tarmac_pre_commit', |
321 | - self, target, source, proposal) |
322 | - |
323 | - except TarmacMergeError, failure: |
324 | - self.logger.warn( |
325 | - u'Merging %(source)s into %(target)s failed: %(msg)s' % |
326 | - {'source': proposal.source_branch.web_link, |
327 | - 'target': proposal.target_branch.web_link, |
328 | - 'msg': str(failure)}) |
329 | - |
330 | - subject = u'Re: [Merge] %(source)s into %(target)s' % { |
331 | - "source": proposal.source_branch.display_name, |
332 | - "target": proposal.target_branch.display_name} |
333 | - |
334 | - if failure.comment: |
335 | - comment = failure.comment |
336 | - else: |
337 | - comment = str(failure) |
338 | - |
339 | - proposal.createComment(subject=subject, content=comment) |
340 | - try: |
341 | - proposal.setStatus( |
342 | - status=self.config.rejected_branch_status) |
343 | - except AttributeError: |
344 | - proposal.setStatus(status=u'Needs review') |
345 | - proposal.lp_save() |
346 | - |
347 | - # If we've been asked to only merge one branch, then exit. |
348 | - if self.config.one: |
349 | - return True |
350 | - |
351 | - continue |
352 | - except PointlessMerge: |
353 | - self.logger.warn( |
354 | - 'Merging %(source)s into %(target)s would be ' |
355 | - 'pointless.' % { |
356 | - 'source': proposal.source_branch.web_link, |
357 | - 'target': proposal.target_branch.web_link}) |
358 | - continue |
359 | - |
360 | - merge_url = get_review_url(proposal) |
361 | - revprops = {'merge_url': merge_url} |
362 | - |
363 | - commit_message = proposal.commit_message |
364 | - if commit_message is None and self.config.imply_commit_message: |
365 | - commit_message = proposal.description |
366 | - |
367 | - target.commit(commit_message, |
368 | - revprops=revprops, |
369 | - authors=source.authors, |
370 | - reviews=self._get_reviews(proposal)) |
371 | - |
372 | - self.logger.debug('Firing tarmac_post_commit hook') |
373 | - tarmac_hooks.fire('tarmac_post_commit', |
374 | - self, target, source, proposal) |
375 | - |
376 | - # If we've been asked to only merge one branch, then exit. |
377 | - if self.config.one: |
378 | - return True |
379 | - |
380 | - # This except is here because we need the else and can't have it |
381 | - # without an except as well. |
382 | - except: |
383 | - raise |
384 | - else: |
385 | - self.logger.debug('Firing tarmac_post_merge hook') |
386 | - tarmac_hooks.fire('tarmac_post_merge', |
387 | - self, target) |
388 | - finally: |
389 | - target.cleanup() |
390 | - |
391 | - def _get_mergable_proposals_for_branch(self, lp_branch): |
392 | - """Return a list of the mergable proposals for the given branch.""" |
393 | - proposals = [] |
394 | - for entry in lp_branch.landing_candidates: |
395 | - self.logger.debug("Considering merge proposal: {0}".format(entry.web_link)) |
396 | - |
397 | - if entry.queue_status != u'Approved': |
398 | - self.logger.debug( |
399 | - " Skipping proposal: status is {0}, not " |
400 | - "'Approved'".format(entry.queue_status)) |
401 | - continue |
402 | - |
403 | - if (not self.config.imply_commit_message and |
404 | - not entry.commit_message): |
405 | - self.logger.debug( |
406 | - " Skipping proposal: proposal has no commit message") |
407 | - continue |
408 | - |
409 | - proposals.append(entry) |
410 | - return proposals |
411 | - |
412 | - def _get_reviews(self, proposal): |
413 | - """Get the set of reviews from the proposal.""" |
414 | - reviews = [] |
415 | - for vote in proposal.votes: |
416 | - if not vote.comment: |
417 | - continue |
418 | - else: |
419 | - reviews.append('%s;%s' % (vote.reviewer.display_name, |
420 | - vote.comment.vote)) |
421 | - |
422 | - if len(reviews) == 0: |
423 | - return None |
424 | - |
425 | - return reviews |
426 | + statuses = merge_proposals(target, proposals, self.logger, self.config, self) |
427 | + return statuses |
428 | + |
429 | |
430 | def run(self, branch_url=None, launchpad=None, **kwargs): |
431 | for key, value in kwargs.iteritems(): |
432 | self.config.set('Tarmac', key, value) |
433 | |
434 | - if self.config.debug: |
435 | - set_up_debug_logging() |
436 | - self.logger.debug('Debug logging enabled') |
437 | - if self.config.http_debug: |
438 | - httplib2.debuglevel = 1 |
439 | - self.logger.debug('HTTP debugging enabled.') |
440 | - self.logger.debug('Loading plugins') |
441 | - load_plugins() |
442 | - self.logger.debug('Plugins loaded') |
443 | + setup(self.logger, debug=self.config.debug, |
444 | + http_debug=self.config.http_debug) |
445 | |
446 | self.launchpad = launchpad |
447 | if self.launchpad is None: |
448 | @@ -357,23 +371,24 @@ |
449 | self.launchpad = self.get_launchpad_object() |
450 | self.logger.debug('launchpad object loaded') |
451 | |
452 | + statuses = [] |
453 | + |
454 | if branch_url: |
455 | self.logger.debug('%(branch_url)s specified as branch_url' % { |
456 | 'branch_url': branch_url}) |
457 | if not branch_url.startswith('lp:'): |
458 | raise TarmacCommandError('Branch urls must start with lp:') |
459 | - self._do_merges(branch_url) |
460 | - |
461 | + statuses.extend(self._do_merges(branch_url)) |
462 | else: |
463 | for branch in self.config.branches: |
464 | self.logger.debug( |
465 | 'Merging approved branches against %(branch)s' % { |
466 | 'branch': branch}) |
467 | try: |
468 | - merged = self._do_merges(branch) |
469 | + statuses.extend(self._do_merges(branch)) |
470 | |
471 | # If we've been asked to only merge one branch, then exit. |
472 | - if merged and self.config.one: |
473 | + if statuses and self.config.one: |
474 | break |
475 | except LockContention: |
476 | continue |
477 | @@ -382,3 +397,6 @@ |
478 | 'An error occurred trying to merge %s: %s', |
479 | branch, error) |
480 | raise |
481 | + if len(filter(lambda x: x is False, statuses)) > 0: |
482 | + return 2 |
483 | + return 0 |
484 | |
485 | === modified file 'tarmac/bin/registry.py' |
486 | --- tarmac/bin/registry.py 2010-10-25 20:20:18 +0000 |
487 | +++ tarmac/bin/registry.py 2012-12-18 21:22:38 +0000 |
488 | @@ -45,7 +45,7 @@ |
489 | |
490 | def _run(self, args): |
491 | '''Execute the command.''' |
492 | - run_bzr(args) |
493 | + return run_bzr(args) |
494 | |
495 | def install_hooks(self): |
496 | '''Use the bzrlib Command support for running commands.''' |
497 | @@ -57,7 +57,7 @@ |
498 | def run(self, args): |
499 | '''Execute the command.''' |
500 | try: |
501 | - self._run(args) |
502 | + return self._run(args) |
503 | except BzrCommandError, e: |
504 | sys.exit('tarmac: ERROR: ' + str(e)) |
505 | |
506 | |
507 | === modified file 'tarmac/plugins/command.py' |
508 | --- tarmac/plugins/command.py 2012-03-06 19:55:15 +0000 |
509 | +++ tarmac/plugins/command.py 2012-12-18 21:22:38 +0000 |
510 | @@ -185,7 +185,7 @@ |
511 | u'%(output)s') % { |
512 | 'source': self.proposal.source_branch.display_name, |
513 | 'target': self.proposal.target_branch.display_name, |
514 | - 'output': u'\n'.join([stdout_value, stderr_value]), |
515 | + 'output': u'\n'.join([stdout_value.decode('utf-8', 'ignore'), stderr_value.decode('utf-8', 'ignore')]), |
516 | } |
517 | raise VerifyCommandFailed(message, comment) |
518 | |
519 | |
520 | === added file 'tarmac/plugins/commit.py' |
521 | --- tarmac/plugins/commit.py 1970-01-01 00:00:00 +0000 |
522 | +++ tarmac/plugins/commit.py 2012-12-18 21:22:38 +0000 |
523 | @@ -0,0 +1,24 @@ |
524 | +import re |
525 | + |
526 | +from tarmac.hooks import tarmac_hooks |
527 | +from tarmac.plugins import TarmacPlugin |
528 | + |
529 | + |
530 | +class CommitMessage(TarmacPlugin): |
531 | + |
532 | + def run(self, command, target, source, proposal): |
533 | + if not re.match(r'^\[.+?\].*$', proposal.commit_message): |
534 | + params = ["[r=%s]" % proposal.reviewer.name] |
535 | + for bug in proposal.source_branch.linked_bugs: |
536 | + params.append("[bug=%s]" % bug.id) |
537 | + |
538 | + proposal.commit_message = ("%s " % ",".join(params) + |
539 | + proposal.commit_message) |
540 | + self.logger.debug("Setting commit_message to %s", |
541 | + proposal.commit_message) |
542 | + else: |
543 | + self.logger.debug("%s has already right commit message", |
544 | + proposal) |
545 | + |
546 | +tarmac_hooks['tarmac_pre_commit'].hook(CommitMessage(), |
547 | + 'Commit messsage template editor.') |