Tarmac

Merge lp:~james-w/tarmac/branch-tree-properties into lp:tarmac

  1. branch-tree-properties
  2. Merge into main
Proposed by James Westby
Status: Rejected
Rejected by: dobey
Proposed branch: lp:~james-w/tarmac/branch-tree-properties
Merge into: lp:tarmac
Prerequisite: lp:~james-w/tarmac/lock-contention
Diff against target: 274 lines (+80/-53)
2 files modified
tarmac/bin/commands.py (+7/-5)
tarmac/branch.py (+73/-48)
To merge this branch: bzr merge lp:~james-w/tarmac/branch-tree-properties
Reviewer Review Type Date Requested Status
dobey Disapprove
Paul Hummer Approve
Review via email: mp+144805@code.launchpad.net

Commit message

Don't keep an open branch/tree around for too long but reopen it instead.

Description of the change

Don't keep an open branch/tree around for too long but reopen it instead.

I believe this was to avoid failing if LP closes the connection during the
tests.

(from the u1 fork)

To post a comment you must log in.
Revision history for this message
Paul Hummer (rockstar) :
review: Approve
Revision history for this message
Paul Hummer (rockstar) wrote :

The prerequisite lp:~james-w/tarmac/lock-contention has not yet been merged into lp:tarmac.

Revision history for this message
Paul Hummer (rockstar) wrote :

Attempt to merge into lp:tarmac failed due to conflicts:

text conflict in tarmac/branch.py

Revision history for this message
dobey (dobey) wrote :

Please create bugs for the issues, include tests, and split the changes into a branch or two which do not have unnecessary dependencies on other unrelated changes.

review: Disapprove

Unmerged revisions

406. By James Westby

Merge trunk.

405. By James Westby

Merged lock-contention into branch-tree-properties.

404. By James Westby

Use properties to get the branch and tree, rather than assigning at instaniation.

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 2013-01-31 20:03:27 +0000
3+++ tarmac/bin/commands.py 2013-03-13 17:29:24 +0000
4@@ -210,9 +210,10 @@
5 source = Branch.create(
6 proposal.source_branch, self.config, target=target)
7
8- approved = source.bzr_branch.revision_id_to_revno(
9+ source_bzr_branch = source.get_bzr_branch()
10+ approved = source_bzr_branch.revision_id_to_revno(
11 str(proposal.reviewed_revid))
12- tip = source.bzr_branch.revno()
13+ tip = source_bzr_branch.revno()
14
15 if tip > approved:
16 message = u'Unapproved changes made after approval'
17@@ -276,10 +277,11 @@
18 commit_message = proposal.commit_message
19 if commit_message is None and self.config.imply_commit_message:
20 commit_message = proposal.description
21+
22 target.commit(commit_message,
23- revprops=revprops,
24- authors=source.authors,
25- reviews=self._get_reviews(proposal))
26+ revprops=revprops,
27+ authors=source.authors,
28+ reviews=self._get_reviews(proposal))
29
30 self.logger.debug('Firing tarmac_post_commit hook')
31 tarmac_hooks.fire('tarmac_post_commit',
32
33=== modified file 'tarmac/branch.py'
34--- tarmac/branch.py 2013-02-04 21:07:51 +0000
35+++ tarmac/branch.py 2013-03-13 17:29:24 +0000
36@@ -37,7 +37,6 @@
37
38 def __init__(self, lp_branch, config=False, target=None):
39 self.lp_branch = lp_branch
40- self.bzr_branch = bzr_branch.Branch.open(self.lp_branch.bzr_identity)
41 if config:
42 self.config = BranchConfig(lp_branch.bzr_identity, config)
43 else:
44@@ -45,15 +44,27 @@
45
46 self.target = target
47 self.logger = logging.getLogger('tarmac')
48+ self.temp_tree_dir = None
49+
50+ def get_bzr_branch(self):
51+ return bzr_branch.Branch.open(self.lp_branch.bzr_identity)
52+
53+ # For backwards compatibility
54+ bzr_branch = property(get_bzr_branch)
55+
56+ def get_tree(self):
57+ if self.temp_tree_dir is not None:
58+ return WorkingTree.open(self.temp_tree_dir)
59+ if os.path.exists(self.config.tree_dir):
60+ return WorkingTree.open(self.config.tree_dir)
61+
62+ # For backwards compatibility
63+ tree = property(get_tree)
64
65 def __del__(self):
66 """Do some potenetially necessary cleanup during deletion."""
67- try:
68- # If we were using a temp directory, then remove it
69+ if self.temp_tree_dir is not None:
70 shutil.rmtree(self.temp_tree_dir)
71- except AttributeError:
72- # Not using a tempdir
73- pass
74
75 @classmethod
76 def create(cls, lp_branch, config, create_tree=False, target=None):
77@@ -62,50 +73,59 @@
78 clazz.create_tree()
79 return clazz
80
81+
82 def create_tree(self):
83 '''Create the dir and working tree.'''
84+ bzr_branch = self.get_bzr_branch()
85 try:
86- self.logger.debug(
87- 'Using tree in %(tree_dir)s' % {
88- 'tree_dir': self.config.tree_dir})
89- if os.path.exists(self.config.tree_dir):
90- self.tree = WorkingTree.open(self.config.tree_dir)
91- else:
92+ tree = self.get_tree()
93+ if tree is None:
94 self.logger.debug('Tree does not exist. Creating dir')
95 # Create the path up to but not including tree_dir if it does
96 # not exist.
97 parent_dir = os.path.dirname(self.config.tree_dir)
98 if not os.path.exists(parent_dir):
99 os.makedirs(parent_dir)
100- self.tree = self.bzr_branch.create_checkout(
101- self.config.tree_dir, lightweight=True)
102+ tree = bzr_branch.create_checkout(
103+ self.config.tree_dir, lightweight=False)
104+ self.logger.debug(
105+ 'Using tree in %(tree_dir)s' % {
106+ 'tree_dir': self.config.tree_dir})
107 except AttributeError:
108 # Store this so we can rmtree later
109 self.temp_tree_dir = tempfile.mkdtemp()
110 self.logger.debug(
111 'Using temp dir at %(tree_dir)s' % {
112 'tree_dir': self.temp_tree_dir})
113- self.tree = self.bzr_branch.create_checkout(self.temp_tree_dir)
114+ tree = bzr_branch.create_checkout(self.temp_tree_dir)
115
116 self.cleanup()
117
118 def cleanup(self):
119 '''Remove the working tree from the temp dir.'''
120- assert self.tree
121- self.tree.revert()
122- for filename in [self.tree.abspath(f) for f in self.unmanaged_files]:
123+ tree = self.get_tree()
124+ assert tree
125+ self.logger.info("Running cleanup in %s." % (
126+ self.lp_branch.bzr_identity))
127+ tree.revert()
128+ self.logger.info("Reverted changes in %s." % (
129+ self.lp_branch.bzr_identity))
130+ for filename in [tree.abspath(f) for f in self.unmanaged_files]:
131 if os.path.isdir(filename) and not os.path.islink(filename):
132 shutil.rmtree(filename)
133 else:
134 os.remove(filename)
135
136- self.tree.update()
137+ self.logger.info("Successfully removed extra files from %s." % (
138+ self.lp_branch.bzr_identity))
139+ tree.update()
140
141 def merge(self, branch, revid=None):
142 '''Merge from another tarmac.branch.Branch instance.'''
143- assert self.tree
144- conflict_list = self.tree.merge_from_branch(
145- branch.bzr_branch, to_revision=revid)
146+ tree = self.get_tree()
147+ assert tree
148+ conflict_list = tree.merge_from_branch(
149+ branch.get_bzr_branch(), to_revision=revid)
150 if conflict_list:
151 message = u'Conflicts merging branch.'
152 lp_comment = (
153@@ -119,26 +139,30 @@
154 def unmanaged_files(self):
155 """Get the list of ignored and unknown files in the tree."""
156 unmanaged = []
157+ tree = self.get_tree()
158 try:
159- self.tree.lock_read()
160- unmanaged = [x for x in self.tree.unknowns()]
161- unmanaged.extend([x[0] for x in self.tree.ignored_files()])
162+ tree.lock_read()
163+ unmanaged = [x for x in tree.unknowns()]
164+ unmanaged.extend([x[0] for x in tree.ignored_files()])
165 finally:
166- self.tree.unlock()
167+ tree.unlock()
168 return unmanaged
169
170 @property
171 def conflicts(self):
172 '''Print the conflicts.'''
173- assert self.tree.conflicts()
174+ tree = self.get_tree()
175+ assert tree
176 conflicts = []
177- for conflict in self.tree.conflicts():
178+ for conflict in tree.conflicts():
179 conflicts.append(
180 u'%s in %s' % (conflict.typestring, conflict.path))
181 return '\n'.join(conflicts)
182
183 def commit(self, commit_message, revprops=None, **kwargs):
184 '''Commit changes.'''
185+ tree = self.get_tree()
186+ assert tree
187 if not revprops:
188 revprops = {}
189
190@@ -155,8 +179,8 @@
191 'review identity or vote.')
192 revprops['reviews'] = '\n'.join(reviews)
193
194- self.tree.commit(commit_message, committer='Tarmac',
195- revprops=revprops, authors=authors)
196+ tree.commit(commit_message, committer='Tarmac',
197+ revprops=revprops, authors=authors)
198
199 @property
200 def landing_candidates(self):
201@@ -167,19 +191,21 @@
202 def authors(self):
203 author_list = []
204
205+ bzr_branch = self.get_bzr_branch()
206 if self.target:
207+ target_bzr_branch = self.target.get_bzr_branch()
208 try:
209- self.bzr_branch.lock_read()
210- self.target.bzr_branch.lock_read()
211+ bzr_branch.lock_read()
212+ target_bzr_branch.lock_read()
213
214- graph = self.bzr_branch.repository.get_graph(
215- self.target.bzr_branch.repository)
216+ graph = bzr_branch.repository.get_graph(
217+ target_bzr_branch.repository)
218
219 unique_ids = graph.find_unique_ancestors(
220- self.bzr_branch.last_revision(),
221- [self.target.bzr_branch.last_revision()])
222+ bzr_branch.last_revision(),
223+ [target_bzr_branch.last_revision()])
224
225- revs = self.bzr_branch.repository.get_revisions(unique_ids)
226+ revs = bzr_branch.repository.get_revisions(unique_ids)
227 for rev in revs:
228 apparent_authors = rev.get_apparent_authors()
229 for author in apparent_authors:
230@@ -188,12 +214,12 @@
231 author_list.append(author)
232
233 finally:
234- self.target.bzr_branch.unlock()
235- self.bzr_branch.unlock()
236+ target_bzr_branch.unlock()
237+ bzr_branch.unlock()
238 else:
239- last_rev = self.bzr_branch.last_revision()
240+ last_rev = bzr_branch.last_revision()
241 if last_rev != 'null:':
242- rev = self.bzr_branch.repository.get_revision(last_rev)
243+ rev = bzr_branch.repository.get_revision(last_rev)
244 apparent_authors = rev.get_apparent_authors()
245 author_list.extend(
246 [a.replace('\n', '') for a in apparent_authors])
247@@ -204,21 +230,20 @@
248 def fixed_bugs(self):
249 """Return the list of bugs fixed by the branch."""
250 bugs_list = []
251-
252+ bzr_branch = self.get_bzr_branch()
253 try:
254- self.bzr_branch.lock_read()
255- oldrevid = self.bzr_branch.get_rev_id(self.lp_branch.revision_count)
256- for rev_info in self.bzr_branch.iter_merge_sorted_revisions(
257+ bzr_branch.lock_read()
258+ oldrevid = bzr_branch.get_rev_id(self.lp_branch.revision_count)
259+ for rev_info in bzr_branch.iter_merge_sorted_revisions(
260 stop_revision_id=oldrevid):
261 try:
262- rev = self.bzr_branch.repository.get_revision(rev_info[0])
263+ rev = bzr_branch.repository.get_revision(rev_info[0])
264 for bug in rev.iter_bugs():
265 if bug[0].startswith('https://launchpad.net/bugs/'):
266 bugs_list.append(bug[0].replace(
267 'https://launchpad.net/bugs/', ''))
268 except NoSuchRevision:
269 continue
270-
271 finally:
272- self.bzr_branch.unlock()
273+ bzr_branch.unlock()
274 return bugs_list

Subscribers

People subscribed via source and target branches