Bazaar

Merge lp:~mbp/bzr/45719-update-r into lp:bzr

  1. 45719-update-r
  2. Merge into bzr.dev
Proposed by Martin Pool
Status: Merged
Approved by: John A Meinel
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~mbp/bzr/45719-update-r
Merge into: lp:bzr
Diff against target: 361 lines (+176/-37)
5 files modified
NEWS (+13/-5)
bzrlib/builtins.py (+41/-15)
bzrlib/tests/blackbox/test_update.py (+82/-6)
bzrlib/tests/per_workingtree/test_workingtree.py (+14/-0)
bzrlib/workingtree.py (+26/-11)
To merge this branch: bzr merge lp:~mbp/bzr/45719-update-r
Reviewer Review Type Date Requested Status
John A Meinel Approve
Review via email: mp+16476@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Martin Pool (mbp) wrote :

This adds a 'bzr update -r' option. If the branch is bound to another, it always updates from the master branch first. This updated form of the patch addresses John's review comments in <https://bugs.edge.launchpad.net/bzr/+bug/45719/comments/20>.

This is an updated version of what was one of the longest-standing merges, and it's for a five-digit bug too <https://bugs.edge.launchpad.net/bzr/+bug/45719>.

Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
> Martin Pool has proposed merging lp:~mbp/bzr/45719-update-r into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
> Related bugs:
> #45719 update command cannot take a revision number
> https://bugs.launchpad.net/bugs/45719
>
>
> This adds a 'bzr update -r' option. If the branch is bound to another, it always updates from the master branch first. This updated form of the patch addresses John's review comments in <https://bugs.edge.launchpad.net/bzr/+bug/45719/comments/20>.
>
> This is an updated version of what was one of the longest-standing merges, and it's for a five-digit bug too <https://bugs.edge.launchpad.net/bzr/+bug/45719>.
>

v- does 'change_reporter' need to be inside the try/except ? I think it
just got refactored into there.

+ try:
+ change_reporter = delta._ChangeReporter(
+ unversioned_filter=tree.is_ignored,
+ view_info=view_info)
+ conflicts = tree.update(
+ change_reporter,
+ possible_transports=possible_transports,
+ revision=rev,
+ old_tip=old_tip)
+ except errors.NoSuchRevision, e:
+ raise errors.BzrCommandError(
+ "branch has no revision %s\n"
+ "bzr update --revision only works"
+ " for a revision in the branch
history"
+ % (e.revision))

+ if revision is not None:
+ rev = revision[0].as_revision_id(branch)
+ else:
+ rev = branch.last_revision()

^- It might be clearer as 'rev_id' or 'revision_id = '. We have some
ambiguous naming (is it a Revision, revision_id, or RevSpec...) However,
this is really minor.

 review: approve
 merge: approve

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAksw26oACgkQJdeBCYSNAAO00gCeOtH7vUpSUdkD8xTtoyOgod/g
sdIAn3ABG2q0RkqjrPIhgsm9lv5V5Ofs
=IrMf
-----END PGP SIGNATURE-----

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2009-12-23 05:04:12 +0000
3+++ NEWS 2009-12-23 06:33:19 +0000
4@@ -23,11 +23,10 @@
5 ``locations.conf`` or ``branch.conf``.
6 (Ted Gould, Matthew Fuller, Vincent Ladeuil)
7
8-* ``bzrlib.tests.permute_for_extension`` is a helper that simplifies
9- running all tests in the current module, once against a pure python
10- implementation, and once against an extension (pyrex/C) implementation.
11- It can be used to dramatically simplify the implementation of
12- ``load_tests``. (John Arbash Meinel)
13+* ``bzr update`` now takes a ``--revision`` argument. This lets you
14+ change the revision of the working tree to any revision in the
15+ ancestry of the current or master branch. (Matthieu Moy, Mark Hammond,
16+ Martin Pool, #45719)
17
18 Bug Fixes
19 *********
20@@ -60,6 +59,9 @@
21 CamelCase. For the features that were more likely to be used, we added a
22 deprecation thunk, but not all. (John Arbash Meinel)
23
24+* ``WorkingTree.update`` implementations must now accept a ``revision``
25+ parameter.
26+
27 Internals
28 *********
29
30@@ -71,6 +73,12 @@
31 Testing
32 *******
33
34+* ``bzrlib.tests.permute_for_extension`` is a helper that simplifies
35+ running all tests in the current module, once against a pure python
36+ implementation, and once against an extension (pyrex/C) implementation.
37+ It can be used to dramatically simplify the implementation of
38+ ``load_tests``. (John Arbash Meinel)
39+
40 * ``bzrlib.tests.TestCase`` now subclasses ``testtools.testcase.TestCase``.
41 This permits features in testtools such as getUniqueInteger and
42 getUniqueString to be used. Because of this, testtools version 0.9.2 or
43
44=== modified file 'bzrlib/builtins.py'
45--- bzrlib/builtins.py 2009-12-21 17:24:22 +0000
46+++ bzrlib/builtins.py 2009-12-23 06:33:19 +0000
47@@ -1388,16 +1388,24 @@
48
49 If you want to discard your local changes, you can just do a
50 'bzr revert' instead of 'bzr commit' after the update.
51+
52+ If the tree's branch is bound to a master branch, it will also update
53+ the branch from the master.
54 """
55
56 _see_also = ['pull', 'working-trees', 'status-flags']
57 takes_args = ['dir?']
58+ takes_options = ['revision']
59 aliases = ['up']
60
61- def run(self, dir='.'):
62+ def run(self, dir='.', revision=None):
63+ if revision is not None and len(revision) != 1:
64+ raise errors.BzrCommandError(
65+ "bzr update --revision takes exactly one revision")
66 tree = WorkingTree.open_containing(dir)[0]
67+ branch = tree.branch
68 possible_transports = []
69- master = tree.branch.get_master_branch(
70+ master = branch.get_master_branch(
71 possible_transports=possible_transports)
72 if master is not None:
73 tree.lock_write()
74@@ -1410,20 +1418,38 @@
75 self.outf.encoding)
76 try:
77 existing_pending_merges = tree.get_parent_ids()[1:]
78- last_rev = _mod_revision.ensure_null(tree.last_revision())
79- if last_rev == _mod_revision.ensure_null(
80- tree.branch.last_revision()):
81- # may be up to date, check master too.
82- if master is None or last_rev == _mod_revision.ensure_null(
83- master.last_revision()):
84- revno = tree.branch.revision_id_to_revno(last_rev)
85- note('Tree is up to date at revision %d of branch %s'
86- % (revno, branch_location))
87- return 0
88+ if master is None:
89+ old_tip = None
90+ else:
91+ # may need to fetch data into a heavyweight checkout
92+ # XXX: this may take some time, maybe we should display a
93+ # message
94+ old_tip = branch.update(possible_transports)
95+ if revision is not None:
96+ revision_id = revision[0].as_revision_id(branch)
97+ else:
98+ revision_id = branch.last_revision()
99+ if revision_id == _mod_revision.ensure_null(tree.last_revision()):
100+ revno = branch.revision_id_to_revno(revision_id)
101+ note("Tree is up to date at revision %d of branch %s" %
102+ (revno, branch_location))
103+ return 0
104 view_info = _get_view_info_for_change_reporter(tree)
105- conflicts = tree.update(
106- delta._ChangeReporter(unversioned_filter=tree.is_ignored,
107- view_info=view_info), possible_transports=possible_transports)
108+ change_reporter = delta._ChangeReporter(
109+ unversioned_filter=tree.is_ignored,
110+ view_info=view_info)
111+ try:
112+ conflicts = tree.update(
113+ change_reporter,
114+ possible_transports=possible_transports,
115+ revision=revision_id,
116+ old_tip=old_tip)
117+ except errors.NoSuchRevision, e:
118+ raise errors.BzrCommandError(
119+ "branch has no revision %s\n"
120+ "bzr update --revision only works"
121+ " for a revision in the branch history"
122+ % (e.revision))
123 revno = tree.branch.revision_id_to_revno(
124 _mod_revision.ensure_null(tree.last_revision()))
125 note('Updated to revision %d of branch %s' %
126
127=== modified file 'bzrlib/tests/blackbox/test_update.py'
128--- bzrlib/tests/blackbox/test_update.py 2009-12-14 15:51:36 +0000
129+++ bzrlib/tests/blackbox/test_update.py 2009-12-23 06:33:19 +0000
130@@ -1,4 +1,4 @@
131-# Copyright (C) 2006 Canonical Ltd
132+# Copyright (C) 2006, 2009 Canonical Ltd
133 # -*- coding: utf-8 -*-
134 #
135 # This program is free software; you can redistribute it and/or modify
136@@ -29,6 +29,7 @@
137 urlutils,
138 workingtree,
139 )
140+from bzrlib.tests.script import ScriptRunner
141
142
143 class TestUpdate(tests.TestCaseWithTransport):
144@@ -67,11 +68,11 @@
145 def test_update_up_to_date_checkout(self):
146 self.make_branch_and_tree('branch')
147 self.run_bzr('checkout branch checkout')
148- out, err = self.run_bzr('update checkout')
149- self.assertEqual('Tree is up to date at revision 0 of branch %s\n'
150- % osutils.pathjoin(self.test_dir, 'branch'),
151- err)
152- self.assertEqual('', out)
153+ sr = ScriptRunner()
154+ sr.run_script(self, '''
155+$ bzr update checkout
156+2>Tree is up to date at revision 0 of branch .../branch
157+''')
158
159 def test_update_out_of_date_standalone_tree(self):
160 # FIXME the default format has to change for this to pass
161@@ -239,3 +240,78 @@
162 lightweight=True)
163 tree.commit('empty commit')
164 self.run_bzr('update checkout')
165+
166+ def test_update_dash_r(self):
167+ # Test that 'bzr update' works correctly when you have
168+ # an update in the master tree, and a lightweight checkout
169+ # which has merged another branch
170+ master = self.make_branch_and_tree('master')
171+ os.chdir('master')
172+ self.build_tree(['./file1'])
173+ master.add(['file1'])
174+ master.commit('one', rev_id='m1')
175+ self.build_tree(['./file2'])
176+ master.add(['file2'])
177+ master.commit('two', rev_id='m2')
178+
179+ sr = ScriptRunner()
180+ sr.run_script(self, '''
181+$ bzr update -r 1
182+2>-D file2
183+2>All changes applied successfully.
184+2>Updated to revision 1 of .../master
185+''')
186+ self.failUnlessExists('./file1')
187+ self.failIfExists('./file2')
188+ self.assertEquals(['m1'], master.get_parent_ids())
189+
190+ def test_update_dash_r_outside_history(self):
191+ # Test that 'bzr update' works correctly when you have
192+ # an update in the master tree, and a lightweight checkout
193+ # which has merged another branch
194+ master = self.make_branch_and_tree('master')
195+ self.build_tree(['master/file1'])
196+ master.add(['file1'])
197+ master.commit('one', rev_id='m1')
198+
199+ # Create a second branch, with an extra commit
200+ other = master.bzrdir.sprout('other').open_workingtree()
201+ self.build_tree(['other/file2'])
202+ other.add(['file2'])
203+ other.commit('other2', rev_id='o2')
204+
205+ os.chdir('master')
206+ self.run_bzr('merge ../other')
207+ master.commit('merge', rev_id='merge')
208+
209+ out, err = self.run_bzr('update -r revid:o2',
210+ retcode=3)
211+ self.assertEqual('', out)
212+ self.assertEqual('bzr: ERROR: branch has no revision o2\n'
213+ 'bzr update --revision only works'
214+ ' for a revision in the branch history\n',
215+ err)
216+
217+ def test_update_dash_r_in_master(self):
218+ # Test that 'bzr update' works correctly when you have
219+ # an update in the master tree,
220+ master = self.make_branch_and_tree('master')
221+ self.build_tree(['master/file1'])
222+ master.add(['file1'])
223+ master.commit('one', rev_id='m1')
224+
225+ self.run_bzr('checkout master checkout')
226+
227+ # add a revision in the master.
228+ self.build_tree(['master/file2'])
229+ master.add(['file2'])
230+ master.commit('two', rev_id='m2')
231+
232+ os.chdir('checkout')
233+ sr = ScriptRunner()
234+ sr.run_script(self, '''
235+$ bzr update -r revid:m2
236+2>+N file2
237+2>All changes applied successfully.
238+2>Updated to revision 2 of branch .../master
239+''')
240
241=== modified file 'bzrlib/tests/per_workingtree/test_workingtree.py'
242--- bzrlib/tests/per_workingtree/test_workingtree.py 2009-11-08 05:28:57 +0000
243+++ bzrlib/tests/per_workingtree/test_workingtree.py 2009-12-23 06:33:19 +0000
244@@ -590,6 +590,20 @@
245 self.assertEqual(master_tree.branch.revision_history(),
246 tree.branch.revision_history())
247
248+ def test_update_takes_revision_parameter(self):
249+ wt = self.make_branch_and_tree('wt')
250+ self.build_tree_contents([('wt/a', 'old content')])
251+ wt.add(['a'])
252+ rev1 = wt.commit('first master commit')
253+ self.build_tree_contents([('wt/a', 'new content')])
254+ rev2 = wt.commit('second master commit')
255+ # https://bugs.edge.launchpad.net/bzr/+bug/45719/comments/20
256+ # when adding 'update -r' we should make sure all wt formats support
257+ # it
258+ conflicts = wt.update(revision=rev1)
259+ self.assertFileEqual('old content', 'wt/a')
260+ self.assertEqual([rev1], wt.get_parent_ids())
261+
262 def test_merge_modified_detects_corruption(self):
263 # FIXME: This doesn't really test that it works; also this is not
264 # implementation-independent. mbp 20070226
265
266=== modified file 'bzrlib/workingtree.py'
267--- bzrlib/workingtree.py 2009-12-02 18:15:55 +0000
268+++ bzrlib/workingtree.py 2009-12-23 06:33:19 +0000
269@@ -2191,7 +2191,10 @@
270 """
271 raise NotImplementedError(self.unlock)
272
273- def update(self, change_reporter=None, possible_transports=None):
274+ _marker = object()
275+
276+ def update(self, change_reporter=None, possible_transports=None,
277+ revision=None, old_tip=_marker):
278 """Update a working tree along its branch.
279
280 This will update the branch if its bound too, which means we have
281@@ -2215,10 +2218,16 @@
282 - Merge current state -> basis tree of the master w.r.t. the old tree
283 basis.
284 - Do a 'normal' merge of the old branch basis if it is relevant.
285+
286+ :param revision: The target revision to update to. Must be in the
287+ revision history.
288+ :param old_tip: If branch.update() has already been run, the value it
289+ returned (old tip of the branch or None). _marker is used
290+ otherwise.
291 """
292 if self.branch.get_bound_location() is not None:
293 self.lock_write()
294- update_branch = True
295+ update_branch = (old_tip is self._marker)
296 else:
297 self.lock_tree_write()
298 update_branch = False
299@@ -2226,13 +2235,14 @@
300 if update_branch:
301 old_tip = self.branch.update(possible_transports)
302 else:
303- old_tip = None
304- return self._update_tree(old_tip, change_reporter)
305+ if old_tip is self._marker:
306+ old_tip = None
307+ return self._update_tree(old_tip, change_reporter, revision)
308 finally:
309 self.unlock()
310
311 @needs_tree_write_lock
312- def _update_tree(self, old_tip=None, change_reporter=None):
313+ def _update_tree(self, old_tip=None, change_reporter=None, revision=None):
314 """Update a tree to the master branch.
315
316 :param old_tip: if supplied, the previous tip revision the branch,
317@@ -2253,12 +2263,17 @@
318 last_rev = self.get_parent_ids()[0]
319 except IndexError:
320 last_rev = _mod_revision.NULL_REVISION
321- if last_rev != _mod_revision.ensure_null(self.branch.last_revision()):
322- # merge tree state up to new branch tip.
323+ if revision is None:
324+ revision = self.branch.last_revision()
325+ else:
326+ if revision not in self.branch.revision_history():
327+ raise errors.NoSuchRevision(self.branch, revision)
328+ if last_rev != _mod_revision.ensure_null(revision):
329+ # merge tree state up to specified revision.
330 basis = self.basis_tree()
331 basis.lock_read()
332 try:
333- to_tree = self.branch.basis_tree()
334+ to_tree = self.branch.repository.revision_tree(revision)
335 if basis.inventory.root is None:
336 self.set_root_id(to_tree.get_root_id())
337 self.flush()
338@@ -2268,11 +2283,12 @@
339 basis,
340 this_tree=self,
341 change_reporter=change_reporter)
342+ self.set_last_revision(revision)
343 finally:
344 basis.unlock()
345 # TODO - dedup parents list with things merged by pull ?
346 # reuse the tree we've updated to to set the basis:
347- parent_trees = [(self.branch.last_revision(), to_tree)]
348+ parent_trees = [(revision, to_tree)]
349 merges = self.get_parent_ids()[1:]
350 # Ideally we ask the tree for the trees here, that way the working
351 # tree can decide whether to give us the entire tree or give us a
352@@ -2308,8 +2324,7 @@
353 # should be able to remove this extra flush.
354 self.flush()
355 graph = self.branch.repository.get_graph()
356- base_rev_id = graph.find_unique_lca(self.branch.last_revision(),
357- old_tip)
358+ base_rev_id = graph.find_unique_lca(revision, old_tip)
359 base_tree = self.branch.repository.revision_tree(base_rev_id)
360 other_tree = self.branch.repository.revision_tree(old_tip)
361 result += merge.merge_inner(