Tarmac

Merge lp:~jml/tarmac/break-up-do-merges into lp:~ubuntuone-hackers/tarmac/trunk

  1. break-up-do-merges
  2. Merge into trunk
Proposed by Jonathan Lange
Status: Merged
Merged at revision: 420
Proposed branch: lp:~jml/tarmac/break-up-do-merges
Merge into: lp:~ubuntuone-hackers/tarmac/trunk
Diff against target: 462 lines (+200/-195)
2 files modified
tarmac/bin/commands.py (+194/-191)
tarmac/tests/test_commands.py (+6/-4)
To merge this branch: bzr merge lp:~jml/tarmac/break-up-do-merges
Reviewer Review Type Date Requested Status
Ubuntu One hackers Pending
Review via email: mp+140894@code.launchpad.net

Description of the change

Builds on james_w's branch of the same name, applying my review comments.

To post a comment you must log in.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tarmac/bin/commands.py'
2--- tarmac/bin/commands.py 2012-12-19 15:54:44 +0000
3+++ tarmac/bin/commands.py 2012-12-20 12:57:59 +0000
4@@ -134,6 +134,182 @@
5 help_commands(self.outf)
6
7
8+def _get_mergable_proposals_for_branch(lp_branch, logger, imply_commit_message=False):
9+ """Return a list of the mergable proposals for the given branch."""
10+ proposals = []
11+ for entry in lp_branch.landing_candidates:
12+ logger.debug("Considering merge proposal: {0}".format(entry.web_link))
13+
14+ if entry.queue_status != u'Approved':
15+ logger.debug(
16+ " Skipping proposal: status is {0}, not "
17+ "'Approved'".format(entry.queue_status))
18+ continue
19+
20+ if (not imply_commit_message and not entry.commit_message):
21+ logger.debug(
22+ " Skipping proposal: proposal has no commit message")
23+ continue
24+
25+ proposals.append(entry)
26+ return proposals
27+
28+
29+def _get_reviews(proposal):
30+ """Get the set of reviews from the proposal."""
31+ votes = [vote for vote in proposal.votes if vote.comment]
32+ if not votes:
33+ return None
34+ return [
35+ '%s;%s' % (vote.reviewer.display_name, vote.comment.vote)
36+ for vote in votes]
37+
38+
39+def set_up(logger, debug=False, http_debug=False):
40+ if debug:
41+ set_up_debug_logging()
42+ logger.debug('Debug logging enabled')
43+ if http_debug:
44+ httplib2.debuglevel = 1
45+ logger.debug('HTTP debugging enabled.')
46+ logger.debug('Loading plugins')
47+ load_plugins()
48+ logger.debug('Plugins loaded')
49+
50+
51+def merge_proposals(target, proposals, logger, config, command):
52+ logger.debug('Firing tarmac_pre_merge hook')
53+ tarmac_hooks.fire('tarmac_pre_merge',
54+ command, target)
55+ try:
56+ statuses = [
57+ merge_proposal(target, proposal, logger, config, command)
58+ for proposal in proposals]
59+ logger.debug('Firing tarmac_post_merge hook')
60+ tarmac_hooks.fire('tarmac_post_merge',
61+ command, target)
62+ finally:
63+ target.cleanup()
64+ return statuses
65+
66+
67+def merge_proposal(target, proposal, logger, config, command):
68+ target.cleanup()
69+ logger.debug(
70+ u'Preparing to merge %(source_branch)s' % {
71+ 'source_branch': proposal.source_branch.web_link})
72+ try:
73+ prerequisite = proposal.prerequisite_branch
74+ if prerequisite:
75+ merges = [x for x in prerequisite.landing_targets
76+ if x.target_branch == target.lp_branch and
77+ x.queue_status != u'Superseded']
78+ if len(merges) == 0:
79+ raise TarmacMergeError(
80+ u'No proposals of prerequisite branch.',
81+ u'No proposals found for merge of %s '
82+ u'into %s.' % (
83+ prerequisite.web_link,
84+ target.lp_branch.web_link))
85+ elif len(merges) > 1:
86+ raise TarmacMergeError(
87+ u'Too many proposals of prerequisite.',
88+ u'More than one proposal found for merge '
89+ u'of %s into %s, which is not Superseded.' % (
90+ prerequisite.web_link,
91+ target.lp_branch.web_link))
92+ elif len(merges) == 1:
93+ if merges[0].queue_status != u'Merged':
94+ raise TarmacMergeError(
95+ u'Prerequisite not yet merged.',
96+ u'The prerequisite %s has not yet been '
97+ u'merged into %s.' % (
98+ prerequisite.web_link,
99+ target.lp_branch.web_link))
100+
101+ if not proposal.reviewed_revid:
102+ raise TarmacMergeError(
103+ u'No approved revision specified.')
104+
105+
106+ source = Branch.create(
107+ proposal.source_branch, config, target=target)
108+
109+ source_bzr_branch = source.get_bzr_branch()
110+ approved = source_bzr_branch.revision_id_to_revno(
111+ str(proposal.reviewed_revid))
112+ tip = source_bzr_branch.revno()
113+
114+ if tip > approved:
115+ message = u'Unapproved changes made after approval'
116+ lp_comment = (
117+ u'There are additional revisions which have not '
118+ u'been approved in review. Please seek review and '
119+ u'approval of these new revisions.')
120+ raise UnapprovedChanges(message, lp_comment)
121+
122+ logger.debug(
123+ 'Merging %(source)s at revision %(revision)s' % {
124+ 'source': proposal.source_branch.web_link,
125+ 'revision': proposal.reviewed_revid})
126+
127+ target.merge(source, str(proposal.reviewed_revid))
128+
129+ logger.debug('Firing tarmac_pre_commit hook')
130+ tarmac_hooks.fire('tarmac_pre_commit',
131+ command, target, source, proposal)
132+
133+ except TarmacMergeError, failure:
134+ logger.warn(
135+ u'Merging %(source)s into %(target)s failed: %(msg)s' %
136+ {'source': proposal.source_branch.web_link,
137+ 'target': proposal.target_branch.web_link,
138+ 'msg': str(failure)})
139+
140+ subject = u'Re: [Merge] %(source)s into %(target)s' % {
141+ "source": proposal.source_branch.display_name,
142+ "target": proposal.target_branch.display_name}
143+
144+ if failure.comment:
145+ comment = failure.comment
146+ else:
147+ comment = str(failure)
148+
149+ proposal.createComment(subject=subject, content=comment)
150+ try:
151+ proposal.setStatus(
152+ status=config.rejected_branch_status)
153+ except AttributeError:
154+ proposal.setStatus(status=u'Needs review')
155+ proposal.lp_save()
156+ return False
157+
158+ except PointlessMerge:
159+ logger.warn(
160+ 'Merging %(source)s into %(target)s would be '
161+ 'pointless.' % {
162+ 'source': proposal.source_branch.web_link,
163+ 'target': proposal.target_branch.web_link})
164+ return False
165+
166+ merge_url = get_review_url(proposal)
167+ revprops = {'merge_url': merge_url}
168+
169+ commit_message = proposal.commit_message
170+ if commit_message is None and config.imply_commit_message:
171+ commit_message = proposal.description
172+
173+ target.commit(commit_message,
174+ revprops=revprops,
175+ authors=source.authors,
176+ reviews=_get_reviews(proposal))
177+
178+ logger.debug('Firing tarmac_post_commit hook')
179+ tarmac_hooks.fire('tarmac_post_commit',
180+ command, target, source, proposal)
181+ return True
182+
183+
184 class cmd_merge(TarmacCommand):
185 '''Automatically merge approved merge proposal branches.'''
186
187@@ -143,7 +319,8 @@
188 options.http_debug_option,
189 options.debug_option,
190 options.imply_commit_message_option,
191- options.one_option]
192+ options.one_option,
193+ ]
194
195 def _do_merges(self, branch_url):
196
197@@ -152,204 +329,29 @@
198 self.logger.info('Not a valid branch: {0}'.format(branch_url))
199 return
200
201- proposals = self._get_mergable_proposals_for_branch(lp_branch)
202+ proposals = _get_mergable_proposals_for_branch(lp_branch, self.logger,
203+ imply_commit_message=self.config.imply_commit_message)
204
205 if not proposals:
206 self.logger.info(
207 'No approved proposals found for %(branch_url)s' % {
208 'branch_url': branch_url})
209- return
210+ return []
211+
212+ if self.config.one:
213+ proposals = proposals[0]
214
215 target = Branch.create(lp_branch, self.config, create_tree=True)
216-
217- self.logger.debug('Firing tarmac_pre_merge hook')
218- tarmac_hooks.fire('tarmac_pre_merge',
219- self, target)
220-
221- try:
222- for proposal in proposals:
223- target.cleanup()
224- self.logger.debug(
225- u'Preparing to merge %(source_branch)s' % {
226- 'source_branch': proposal.source_branch.web_link})
227- try:
228- prerequisite = proposal.prerequisite_branch
229- if prerequisite:
230- merges = [x for x in prerequisite.landing_targets
231- if x.target_branch == target.lp_branch and
232- x.queue_status != u'Superseded']
233- if len(merges) == 0:
234- raise TarmacMergeError(
235- u'No proposals of prerequisite branch.',
236- u'No proposals found for merge of %s '
237- u'into %s.' % (
238- prerequisite.web_link,
239- target.lp_branch.web_link))
240- elif len(merges) > 1:
241- raise TarmacMergeError(
242- u'Too many proposals of prerequisite.',
243- u'More than one proposal found for merge '
244- u'of %s into %s, which is not Superseded.' % (
245- prerequisite.web_link,
246- target.lp_branch.web_link))
247- elif len(merges) == 1:
248- if merges[0].queue_status != u'Merged':
249- raise TarmacMergeError(
250- u'Prerequisite not yet merged.',
251- u'The prerequisite %s has not yet been '
252- u'merged into %s.' % (
253- prerequisite.web_link,
254- target.lp_branch.web_link))
255-
256- if not proposal.reviewed_revid:
257- raise TarmacMergeError(
258- u'No approved revision specified.')
259-
260-
261- source = Branch.create(
262- proposal.source_branch, self.config, target=target)
263-
264- source_bzr_branch = source.get_bzr_branch()
265- approved = source_bzr_branch.revision_id_to_revno(
266- str(proposal.reviewed_revid))
267- tip = source_bzr_branch.revno()
268-
269- if tip > approved:
270- message = u'Unapproved changes made after approval'
271- lp_comment = (
272- u'There are additional revisions which have not '
273- u'been approved in review. Please seek review and '
274- u'approval of these new revisions.')
275- raise UnapprovedChanges(message, lp_comment)
276-
277- self.logger.debug(
278- 'Merging %(source)s at revision %(revision)s' % {
279- 'source': proposal.source_branch.web_link,
280- 'revision': proposal.reviewed_revid})
281-
282- target.merge(source, str(proposal.reviewed_revid))
283-
284- self.logger.debug('Firing tarmac_pre_commit hook')
285- tarmac_hooks.fire('tarmac_pre_commit',
286- self, target, source, proposal)
287-
288- except TarmacMergeError, failure:
289- self.logger.warn(
290- u'Merging %(source)s into %(target)s failed: %(msg)s' %
291- {'source': proposal.source_branch.web_link,
292- 'target': proposal.target_branch.web_link,
293- 'msg': str(failure)})
294-
295- subject = u'Re: [Merge] %(source)s into %(target)s' % {
296- "source": proposal.source_branch.display_name,
297- "target": proposal.target_branch.display_name}
298-
299- if failure.comment:
300- comment = failure.comment
301- else:
302- comment = str(failure)
303-
304- proposal.createComment(subject=subject, content=comment)
305- try:
306- proposal.setStatus(
307- status=self.config.rejected_branch_status)
308- except AttributeError:
309- proposal.setStatus(status=u'Needs review')
310- proposal.lp_save()
311-
312- # If we've been asked to only merge one branch, then exit.
313- if self.config.one:
314- return True
315-
316- continue
317- except PointlessMerge:
318- self.logger.warn(
319- 'Merging %(source)s into %(target)s would be '
320- 'pointless.' % {
321- 'source': proposal.source_branch.web_link,
322- 'target': proposal.target_branch.web_link})
323- continue
324-
325- merge_url = get_review_url(proposal)
326- revprops = {'merge_url': merge_url}
327-
328- commit_message = proposal.commit_message
329- if commit_message is None and self.config.imply_commit_message:
330- commit_message = proposal.description
331-
332- target.commit(commit_message,
333- revprops=revprops,
334- authors=source.authors,
335- reviews=self._get_reviews(proposal))
336-
337- self.logger.debug('Firing tarmac_post_commit hook')
338- tarmac_hooks.fire('tarmac_post_commit',
339- self, target, source, proposal)
340-
341- # If we've been asked to only merge one branch, then exit.
342- if self.config.one:
343- return True
344-
345- # This except is here because we need the else and can't have it
346- # without an except as well.
347- except:
348- raise
349- else:
350- self.logger.debug('Firing tarmac_post_merge hook')
351- tarmac_hooks.fire('tarmac_post_merge',
352- self, target)
353- finally:
354- target.cleanup()
355-
356- def _get_mergable_proposals_for_branch(self, lp_branch):
357- """Return a list of the mergable proposals for the given branch."""
358- proposals = []
359- for entry in lp_branch.landing_candidates:
360- self.logger.debug("Considering merge proposal: {0}".format(entry.web_link))
361-
362- if entry.queue_status != u'Approved':
363- self.logger.debug(
364- " Skipping proposal: status is {0}, not "
365- "'Approved'".format(entry.queue_status))
366- continue
367-
368- if (not self.config.imply_commit_message and
369- not entry.commit_message):
370- self.logger.debug(
371- " Skipping proposal: proposal has no commit message")
372- continue
373-
374- proposals.append(entry)
375- return proposals
376-
377- def _get_reviews(self, proposal):
378- """Get the set of reviews from the proposal."""
379- reviews = []
380- for vote in proposal.votes:
381- if not vote.comment:
382- continue
383- else:
384- reviews.append('%s;%s' % (vote.reviewer.display_name,
385- vote.comment.vote))
386-
387- if len(reviews) == 0:
388- return None
389-
390- return reviews
391+ statuses = merge_proposals(target, proposals, self.logger, self.config, self)
392+ return statuses
393+
394
395 def run(self, branch_url=None, launchpad=None, **kwargs):
396 for key, value in kwargs.iteritems():
397 self.config.set('Tarmac', key, value)
398
399- if self.config.debug:
400- set_up_debug_logging()
401- self.logger.debug('Debug logging enabled')
402- if self.config.http_debug:
403- httplib2.debuglevel = 1
404- self.logger.debug('HTTP debugging enabled.')
405- self.logger.debug('Loading plugins')
406- load_plugins()
407- self.logger.debug('Plugins loaded')
408+ set_up(self.logger, debug=self.config.debug,
409+ http_debug=self.config.http_debug)
410
411 self.launchpad = launchpad
412 if self.launchpad is None:
413@@ -357,23 +359,24 @@
414 self.launchpad = self.get_launchpad_object()
415 self.logger.debug('launchpad object loaded')
416
417+ statuses = []
418+
419 if branch_url:
420 self.logger.debug('%(branch_url)s specified as branch_url' % {
421 'branch_url': branch_url})
422 if not branch_url.startswith('lp:'):
423 raise TarmacCommandError('Branch urls must start with lp:')
424- self._do_merges(branch_url)
425-
426+ statuses.extend(self._do_merges(branch_url))
427 else:
428 for branch in self.config.branches:
429 self.logger.debug(
430 'Merging approved branches against %(branch)s' % {
431 'branch': branch})
432 try:
433- merged = self._do_merges(branch)
434+ statuses.extend(self._do_merges(branch))
435
436 # If we've been asked to only merge one branch, then exit.
437- if merged and self.config.one:
438+ if statuses and self.config.one:
439 break
440 except LockContention:
441 continue
442
443=== modified file 'tarmac/tests/test_commands.py'
444--- tarmac/tests/test_commands.py 2012-12-20 11:08:10 +0000
445+++ tarmac/tests/test_commands.py 2012-12-20 12:57:59 +0000
446@@ -198,10 +198,12 @@
447
448 def test_get_reviews(self):
449 """Test that the _get_reviews method gives the right lists."""
450- self.assertEqual(self.command._get_reviews(self.proposals[0]),
451- [u'Reviewer;Needs Fixing'])
452- self.assertEqual(self.command._get_reviews(self.proposals[1]),
453- [u'Reviewer;Approve', u'Reviewer2;Abstain'])
454+ self.assertEqual(
455+ commands._get_reviews(self.proposals[0]),
456+ [u'Reviewer;Needs Fixing'])
457+ self.assertEqual(
458+ commands._get_reviews(self.proposals[1]),
459+ [u'Reviewer;Approve', u'Reviewer2;Abstain'])
460
461 def test_run_merge_url_substitution(self):
462 """Test that the merge urls get substituted correctly."""

Subscribers

People subscribed via source and target branches