bzr Upload plugin

Merge lp://staging/~garyvdm/bzr-upload/check_not_diverged into lp://staging/bzr-upload

  1. check_not_diverged
  2. Merge into trunk
Proposed by Gary van der Merwe
Status: Superseded
Proposed branch: lp://staging/~garyvdm/bzr-upload/check_not_diverged
Merge into: lp://staging/bzr-upload
Diff against target: 297 lines
3 files modified
NEWS (+5/-0)
__init__.py (+101/-55)
tests/test_upload.py (+35/-0)
To merge this branch: bzr merge lp://staging/~garyvdm/bzr-upload/check_not_diverged
Reviewer Review Type Date Requested Status
Vincent Ladeuil Needs Information
Review via email: mp+12555@code.staging.launchpad.net

This proposal has been superseded by a proposal from 2009-10-02.

To post a comment you must log in.
Revision history for this message
Gary van der Merwe (garyvdm) wrote :

This makes bzr-upload check that you are not upload diverged branches. This can be ignored by running bzr upload --overwrite.

Some notes:
* We need to better explain how to resolve the error. This may mean the user does a merge, or runs with --overwrite. There are some complexities with this though. How does the user figure out which branch he needs to merge with. And if he does want to --overwrite, it is quite likely that they don't have the previously uploaded revision in their repo. We should probably detect this, and let them know how the can get the revision into their repo, with out changing their branch tip (bzr pull, ignore the diverged error.) So in all, explaining to the user how to resolve this is quite complex.

* By design, this change does not apply to the auto hook. I has good reasons, but I'll explain tommorow, cause I'm quite tired now.

Revision history for this message
Vincent Ladeuil (vila) wrote :

At first glance, there are a couple of issues:

- you cache the uploaded revid but I don't think we have code paths
  complex enough to need it, so I'd prefer to not do it,

- why do yo suddenly need to lock_read() the branch ? It makes the
  patch hard to read and does not seem related.

- assertUploadRevid in the tests would be better named assertRevidUploaded
  or assertIsUploadedRevid

- both of your tests starts with the same call, that's what setUps are for :)

review: Needs Information
lp://staging/~garyvdm/bzr-upload/check_not_diverged updated
69. By Gary van der Merwe

Some small changes to the tests as per review.

70. By Gary van der Merwe

Fix formating in DivergedError.

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2009-09-05 12:33:03 +0000
3+++ NEWS 2009-10-02 08:03:11 +0000
4@@ -28,6 +28,11 @@
5 defaults to '.bzr-upload.revid'.
6 (Vincent Ladeuil, #423331)
7
8+* Upload now checks that the revision we are uploading is a descendent
9+ from the revision that was uploaded, and hence that the branchs that
10+ they were uploaded from have not diverged. This can be ignored by passing
11+ the --overwrite option. (Gary van der Merwe)
12+
13
14 Bug Fixes
15 *********
16
17=== modified file '__init__.py'
18--- __init__.py 2009-09-05 12:33:03 +0000
19+++ __init__.py 2009-10-02 08:03:11 +0000
20@@ -39,6 +39,15 @@
21
22 bzr upload --full sftp://user@host/location/on/webserver
23
24+This command only works on the revision beening uploaded is a decendent of the
25+revision that was previously uploaded, and that they are hence from branches
26+that have not diverged. Branches are considered diverged if the destination
27+branch's most recent commit is one that has not been merged (directly or
28+indirectly) by the source branch.
29+
30+If branches have diverged, you can use 'bzr upload --overwrite' to replace
31+the other branch completely, discarding its unmerged changes.
32+
33
34 Automatically Uploading
35 -----------------------
36@@ -153,6 +162,7 @@
37 osutils,
38 urlutils,
39 workingtree,
40+ revision
41 )
42 """)
43
44@@ -243,15 +253,23 @@
45 self.quiet = quiet
46 self._pending_deletions = []
47 self._pending_renames = []
48+ self._uploaded_revid = None
49
50 def set_uploaded_revid(self, rev_id):
51 # XXX: Add tests for concurrent updates, etc.
52 revid_path = get_upload_revid_location(self.branch)
53 self.to_transport.put_bytes(revid_path, rev_id)
54+ self._uploaded_revid = rev_id
55
56 def get_uploaded_revid(self):
57- revid_path = get_upload_revid_location(self.branch)
58- return self.to_transport.get_bytes(revid_path)
59+ if self._uploaded_revid is None:
60+ revid_path = get_upload_revid_location(self.branch)
61+ try:
62+ self._uploaded_revid = self.to_transport.get_bytes(revid_path)
63+ except errors.NoSuchFile:
64+ # We have not upload to here.
65+ self._uploaded_revid = revision.NULL_REVISION
66+ return self._uploaded_revid
67
68 def upload_file(self, relpath, id, mode=None):
69 if mode is None:
70@@ -388,9 +406,9 @@
71 def upload_tree(self):
72 # If we can't find the revid file on the remote location, upload the
73 # full tree instead
74- try:
75- rev_id = self.get_uploaded_revid()
76- except errors.NoSuchFile:
77+ rev_id = self.get_uploaded_revid()
78+
79+ if rev_id == revision.NULL_REVISION:
80 if not self.quiet:
81 self.outf.write('No uploaded revision id found,'
82 ' switching to full upload\n')
83@@ -483,6 +501,7 @@
84 takes_options = [
85 'revision',
86 'remember',
87+ 'overwrite',
88 option.Option('full', 'Upload the full working tree.'),
89 option.Option('quiet', 'Do not output what is being done.',
90 short_name='q'),
91@@ -498,7 +517,7 @@
92 ]
93
94 def run(self, location=None, full=False, revision=None, remember=None,
95- directory=None, quiet=False, auto=None
96+ directory=None, quiet=False, auto=None, overwrite=False
97 ):
98 if directory is None:
99 directory = u'.'
100@@ -509,57 +528,73 @@
101
102 (wt, branch,
103 relpath) = bzrdir.BzrDir.open_containing_tree_or_branch(directory)
104-
105+
106 if wt:
107- changes = wt.changes_from(wt.basis_tree())
108-
109- if revision is None and changes.has_changed():
110- raise errors.UncommittedChanges(wt)
111-
112- if location is None:
113- stored_loc = get_upload_location(branch)
114- if stored_loc is None:
115- raise errors.BzrCommandError('No upload location'
116- ' known or specified.')
117- else:
118- # FIXME: Not currently tested
119- display_url = urlutils.unescape_for_display(stored_loc,
120- self.outf.encoding)
121- self.outf.write("Using saved location: %s\n" % display_url)
122- location = stored_loc
123-
124- to_transport = transport.get_transport(location)
125-
126- # Check that we are not uploading to a existing working tree.
127+ wt.lock_read()
128+ else:
129+ branch.lock_read()
130 try:
131- to_bzr_dir = bzrdir.BzrDir.open_from_transport(to_transport)
132- has_wt = to_bzr_dir.has_workingtree()
133- except errors.NotBranchError:
134- has_wt = False
135- except errors.NotLocalUrl:
136- # The exception raised is a bit weird... but that's life.
137- has_wt = True
138-
139- if has_wt:
140- raise CannotUploadToWorkingTreeError(url=location)
141-
142- if revision is None:
143- rev_id = branch.last_revision()
144- else:
145- if len(revision) != 1:
146- raise errors.BzrCommandError(
147- 'bzr upload --revision takes exactly 1 argument')
148- rev_id = revision[0].in_history(branch).rev_id
149-
150- tree = branch.repository.revision_tree(rev_id)
151-
152- uploader = BzrUploader(branch, to_transport, self.outf, tree,
153- rev_id, quiet=quiet)
154-
155- if full:
156- uploader.upload_full_tree()
157- else:
158- uploader.upload_tree()
159+ if wt:
160+ changes = wt.changes_from(wt.basis_tree())
161+
162+ if revision is None and changes.has_changed():
163+ raise errors.UncommittedChanges(wt)
164+
165+ if location is None:
166+ stored_loc = get_upload_location(branch)
167+ if stored_loc is None:
168+ raise errors.BzrCommandError('No upload location'
169+ ' known or specified.')
170+ else:
171+ # FIXME: Not currently tested
172+ display_url = urlutils.unescape_for_display(stored_loc,
173+ self.outf.encoding)
174+ self.outf.write("Using saved location: %s\n" % display_url)
175+ location = stored_loc
176+
177+ to_transport = transport.get_transport(location)
178+
179+ # Check that we are not uploading to a existing working tree.
180+ try:
181+ to_bzr_dir = bzrdir.BzrDir.open_from_transport(to_transport)
182+ has_wt = to_bzr_dir.has_workingtree()
183+ except errors.NotBranchError:
184+ has_wt = False
185+ except errors.NotLocalUrl:
186+ # The exception raised is a bit weird... but that's life.
187+ has_wt = True
188+
189+ if has_wt:
190+ raise CannotUploadToWorkingTreeError(url=location)
191+
192+ if revision is None:
193+ rev_id = branch.last_revision()
194+ else:
195+ if len(revision) != 1:
196+ raise errors.BzrCommandError(
197+ 'bzr upload --revision takes exactly 1 argument')
198+ rev_id = revision[0].in_history(branch).rev_id
199+
200+ tree = branch.repository.revision_tree(rev_id)
201+
202+ uploader = BzrUploader(branch, to_transport, self.outf, tree,
203+ rev_id, quiet=quiet)
204+
205+ if not overwrite:
206+ prev_uploaded_rev_id = uploader.get_uploaded_revid()
207+ graph = branch.repository.get_graph()
208+ if not graph.is_ancestor(prev_uploaded_rev_id, rev_id):
209+ raise DivergedError(rev_id, prev_uploaded_rev_id)
210+
211+ if full:
212+ uploader.upload_full_tree()
213+ else:
214+ uploader.upload_tree()
215+ finally:
216+ if wt:
217+ wt.unlock()
218+ else:
219+ branch.unlock()
220
221 # We uploaded successfully, remember it
222 if get_upload_location(branch) is None or remember:
223@@ -583,6 +618,17 @@
224 else:
225 auto_hook_available = False
226
227+class DivergedError(errors.BzrError):
228+
229+ _fmt = ("The revision upload to this location is from a branch that is "
230+ "diverged from this branch: %(upload_revid) \n")
231+
232+ # We should probably tell the user to use merge on the diverged branch,
233+ # but how do we explan which branch they need to merge from!
234+
235+ def __init__(self, revid, upload_revid):
236+ self.revid = revid
237+ self.upload_revid = upload_revid
238
239 def load_tests(basic_tests, module, loader):
240 # This module shouldn't define any tests but I don't know how to report
241
242=== modified file 'tests/test_upload.py'
243--- tests/test_upload.py 2009-09-02 22:31:06 +0000
244+++ tests/test_upload.py 2009-10-02 08:03:11 +0000
245@@ -29,6 +29,7 @@
246 tests,
247 transport,
248 workingtree,
249+ uncommit,
250 )
251 from bzrlib.smart import server as smart_server
252
253@@ -43,6 +44,7 @@
254 cmd_upload,
255 get_upload_auto,
256 CannotUploadToWorkingTreeError,
257+ DivergedError,
258 )
259
260
261@@ -617,3 +619,36 @@
262 self.do_full_upload(directory=remote_branch_url)
263 self.assertUpFileEqual('foo', 'hello')
264
265+class TestUploadDiverged(tests.TestCaseWithTransport,
266+ UploadUtilsMixin):
267+
268+ def setUp(self):
269+ super(TestUploadDiverged, self).setUp()
270+ self.diverged_tree = self.create_diverged_tree_and_upload_location()
271+
272+ def create_diverged_tree_and_upload_location(self):
273+ tree_a = self.make_branch_and_tree('tree_a')
274+ tree_a.commit('message 1', rev_id='rev1')
275+ tree_a.commit('message 2', rev_id='rev2a')
276+ tree_b = tree_a.bzrdir.sprout('tree_b').open_workingtree()
277+ uncommit.uncommit(tree_b.branch, tree=tree_b)
278+ tree_b.commit('message 2', rev_id='rev2b')
279+
280+ # upload tree a
281+ self.do_full_upload(directory=tree_a.basedir)
282+ return tree_b
283+
284+ def assertRevidUploaded(self, revid):
285+ t = self.get_transport(self.upload_dir)
286+ uploaded_revid = t.get_bytes('.bzr-upload.revid')
287+ self.assertEqual(revid, uploaded_revid)
288+
289+ def test_cant_upload_diverged(self):
290+ self.assertRaises(DivergedError, self.do_incremental_upload,
291+ directory=self.diverged_tree.basedir)
292+ self.assertRevidUploaded('rev2a')
293+
294+ def test_upload_diverged_with_overwrite(self):
295+ self.do_incremental_upload(directory=self.diverged_tree.basedir,
296+ overwrite=True)
297+ self.assertRevidUploaded('rev2b')

Subscribers

People subscribed via source and target branches

to status/vote changes: