Bazaar GTK+ Frontends

Merge lp:~sinzui/bzr-gtk/gpush-do-push into lp:bzr-gtk

  1. gpush-do-push
  2. Merge into gtk3
Proposed by Curtis Hovey
Status: Merged
Approved by: Jelmer Vernooij
Approved revision: 799
Merged at revision: 788
Proposed branch: lp:~sinzui/bzr-gtk/gpush-do-push
Merge into: lp:bzr-gtk
Diff against target: 209 lines (+146/-31)
2 files modified
push.py (+51/-31)
tests/test_push.py (+95/-0)
To merge this branch: bzr merge lp:~sinzui/bzr-gtk/gpush-do-push
Reviewer Review Type Date Requested Status
Jelmer Vernooij (community) Approve
Review via email: mp+96928@code.launchpad.net

Description of the change

Update gpush.do_push to use fast, non-deprecated calls.

do_push() call branch.revision_history() and and PullResult.__init__
which are deprecated. Working with revision_history slows down the push.

--------------------------------------------------------------------

RULES

    * Add tests for the current do_push().
    * Base a new do_push on bzrlib.push which uses create_clone_on_transport
      or push_branch to do the task and it is much faster.
      * Do not call deprecated methods.
    * Remove the old do_push() with the new do_push() when the tests pass for
      both.
    * Provide a better method to show messages about the push to the user.

QA

    BZR_PLUGINS_AT=gtk@./ bzr gpush

IMPLEMENTATION

The diff is a little surprising since I had the new and old do_push
method running in parallel for a time. I struggled writing tests that
setup the three main condition of the old do_push(). The methods
push_branch() and create_clone_on_transport() handle some of the
try-except and if-else rules of the old do_push method. I had to add a
method the create a PushResult to ensure consistent data was available
to make a message. The message function makes a message for the UI based
on the old do_push message and the messages that were created for the
text UI when using `bzr push`.

To post a comment you must log in.
lp:~sinzui/bzr-gtk/gpush-do-push updated
799. By Curtis Hovey

Merged trunk.

Revision history for this message
Jelmer Vernooij (jelmer) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'push.py'
--- push.py 2012-03-05 14:01:51 +0000
+++ push.py 2012-03-11 21:44:18 +0000
@@ -24,7 +24,8 @@
24 errors,24 errors,
25 ui,25 ui,
26 )26 )
27from bzrlib.bzrdir import BzrDir27from bzrlib.controldir import ControlDir
28from bzrlib.push import PushResult
28from bzrlib.transport import get_transport29from bzrlib.transport import get_transport
2930
30from bzrlib.plugins.gtk.dialog import (31from bzrlib.plugins.gtk.dialog import (
@@ -140,43 +141,62 @@
140 :param overwrite: overwrite target location if it diverged141 :param overwrite: overwrite target location if it diverged
141 :return: number of revisions pushed142 :return: number of revisions pushed
142 """143 """
143 transport = get_transport(location)144 revision_id = None
144 location_url = transport.base145 to_transport = get_transport(location)
145
146 try:146 try:
147 dir_to = BzrDir.open(location_url)147 dir_to = ControlDir.open_from_transport(to_transport)
148 br_to = dir_to.open_branch()
149 except errors.NotBranchError:148 except errors.NotBranchError:
150 # create a branch.149 dir_to = None
151 transport = transport.clone('..')150
151 if dir_to is None:
152 try:152 try:
153 relurl = transport.relpath(location_url)153 br_to = br_from.create_clone_on_transport(
154 transport.mkdir(relurl)154 to_transport, revision_id=revision_id)
155 except errors.NoSuchFile:155 except errors.NoSuchFile:
156 response = question_dialog(156 response = question_dialog(
157 _i18n('Non existing parent directory'),157 _i18n('Non existing parent directory'),
158 _i18n("The parent directory (%s)\ndoesn't exist. Create?") %158 _i18n("The parent directory (%s)\ndoesn't exist. Create?") %
159 location)159 location)
160 if response == Gtk.ResponseType.OK:160 if response == Gtk.ResponseType.OK:
161 transport.create_prefix()161 br_to = br_from.create_clone_on_transport(
162 to_transport, revision_id=revision_id, create_prefix=True)
162 else:163 else:
163 return164 return _i18n("Push aborted.")
164 dir_to = br_from.bzrdir.clone(location_url,165 push_result = create_push_result(br_from, br_to)
165 revision_id=br_from.last_revision())
166 br_to = dir_to.open_branch()
167 count = len(br_to.revision_history())
168 else:166 else:
169 br_to.revision_history()167 push_result = dir_to.push_branch(br_from, revision_id, overwrite)
170 try:168 message = create_push_message(br_from, push_result)
171 tree_to = dir_to.open_workingtree()169 return message
172 except errors.NotLocalUrl:170
173 # FIXME - what to do here? how should we warn the user?171
174 count = br_to.pull(br_from, overwrite)172def create_push_message(br_from, push_result):
175 except errors.NoWorkingTree:173 """Return a mesage explaining what happened during the push."""
176 count = br_to.pull(br_from, overwrite)174 messages = []
177 else:175 rev_count = br_from.revno() - push_result.old_revno
178 count = tree_to.pull(br_from, overwrite)176 messages.append(_i18n("%d revision(s) pushed.") % rev_count)
179177 if push_result.stacked_on is not None:
180 # The count var is either an int or a PushResult. PushResult is being178 messages.append(_i18n("Stacked on %s.") % push_result.stacked_on)
181 # coerced into an int, but the method is deprecated.179 if push_result.workingtree_updated is False:
182 return _i18n("%d revision(s) pushed.") % int(count)180 messages.append(_i18n(
181 "\nThe working tree was not updated:"
182 "\nSee 'bzr help working-trees' for more information."))
183 return '\n'.join(messages)
184
185
186def create_push_result(br_from, br_to):
187 """Return a PushResult like one created by ControlDir.push_branch()."""
188 push_result = PushResult()
189 push_result.source_branch = br_from
190 push_result.target_branch = br_to
191 push_result.branch_push_result = None
192 push_result.master_branch = None
193 push_result.old_revno = 0
194 push_result.old_revid = br_to.last_revision()
195 push_result.workingtree_updated = None # Not applicable to this case.
196 try:
197 push_result.stacked_on = br_to.get_stacked_on_url()
198 except (errors.UnstackableBranchFormat,
199 errors.UnstackableRepositoryFormat,
200 errors.NotStacked):
201 push_result.stacked_on = None
202 return push_result
183203
=== modified file 'tests/test_push.py'
--- tests/test_push.py 2012-03-04 01:54:22 +0000
+++ tests/test_push.py 2012-03-11 21:44:18 +0000
@@ -133,3 +133,98 @@
133 (branch, 'lp:~user/fnord/test'), push.do_push.args)133 (branch, 'lp:~user/fnord/test'), push.do_push.args)
134 self.assertEqual(134 self.assertEqual(
135 {'overwrite': True}, push.do_push.kwargs)135 {'overwrite': True}, push.do_push.kwargs)
136
137
138class DoPushTestCase(tests.TestCaseWithTransport):
139
140 def setup_ui(self):
141 set_ui_factory()
142 progress_panel = ProgressPanel()
143 ui.ui_factory.set_progress_bar_widget(progress_panel)
144 MockMethod.bind(self, progress_panel.pb, 'tick')
145 MockMethod.bind(self, progress_panel.pb, 'update')
146 MockMethod.bind(self, progress_panel.pb, 'finished')
147 return progress_panel
148
149 def make_from_branch(self):
150 from_tree = self.make_branch_and_tree('this')
151 self.build_tree(['this/a', 'this/b'])
152 from_tree.add(['a', 'b'])
153 from_tree.commit("msg")
154 return from_tree.branch
155
156 def test_do_push_without_dir(self):
157 progress_panel = self.setup_ui()
158 from_branch = self.make_from_branch()
159 message = push.do_push(from_branch, 'that', False)
160 self.assertEqual('1 revision(s) pushed.', message)
161 self.assertEqual(True, progress_panel.pb.update.called)
162 self.assertEqual(True, progress_panel.pb.finished.called)
163
164 def test_do_push_without_parent_dir(self):
165 progress_panel = self.setup_ui()
166 from_branch = self.make_from_branch()
167 MockMethod.bind(self, push, 'question_dialog', Gtk.ResponseType.OK)
168 message = push.do_push(from_branch, 'that/there', False)
169 self.assertEqual('1 revision(s) pushed.', message)
170 self.assertEqual(True, push.question_dialog.called)
171 self.assertEqual(True, progress_panel.pb.update.called)
172 self.assertEqual(True, progress_panel.pb.finished.called)
173
174 def test_do_push_without_parent_dir_aborted(self):
175 self.setup_ui()
176 from_branch = self.make_from_branch()
177 MockMethod.bind(self, push, 'question_dialog', Gtk.ResponseType.CANCEL)
178 message = push.do_push(from_branch, 'that/there', False)
179 self.assertEqual('Push aborted.', message)
180
181 def test_do_push_with_dir(self):
182 progress_panel = self.setup_ui()
183 self.make_branch_and_tree('that')
184 from_branch = self.make_from_branch()
185 message = push.do_push(from_branch, 'that', False)
186 self.assertEqual('1 revision(s) pushed.', message)
187 self.assertEqual(True, progress_panel.pb.update.called)
188 self.assertEqual(True, progress_panel.pb.finished.called)
189
190 def test_create_push_result_unstacked(self):
191 from_branch = object()
192 to_branch = self.make_branch_and_tree('that').branch
193 push_result = push.create_push_result(from_branch, to_branch)
194 self.assertIs(from_branch, push_result.source_branch)
195 self.assertIs(to_branch, push_result.target_branch)
196 self.assertIs(None, push_result.branch_push_result)
197 self.assertIs(None, push_result.master_branch)
198 self.assertEqual(0, push_result.old_revno)
199 self.assertEqual(to_branch.last_revision(), push_result.old_revid)
200 self.assertIs(None, push_result.workingtree_updated)
201 self.assertIs(None, push_result.stacked_on)
202
203 def test_create_push_result_stacked(self):
204 from_branch = object()
205 to_branch = self.make_branch_and_tree('that').branch
206 MockMethod.bind(self, to_branch, 'get_stacked_on_url', 'lp:project')
207 push_result = push.create_push_result(from_branch, to_branch)
208 self.assertEqual('lp:project', push_result.stacked_on)
209
210 def test_create_push_message_stacked_on(self):
211 from_branch = object()
212 to_branch = self.make_branch_and_tree('that').branch
213 MockMethod.bind(self, to_branch, 'get_stacked_on_url', 'lp:project')
214 push_result = push.create_push_result(from_branch, to_branch)
215 from_branch = self.make_from_branch()
216 message = push.create_push_message(from_branch, push_result)
217 self.assertEqual(
218 '1 revision(s) pushed.\nStacked on lp:project.', message)
219
220 def test_create_push_message_workingtree_updated_false(self):
221 from_branch = object()
222 to_branch = self.make_branch_and_tree('that').branch
223 push_result = push.create_push_result(from_branch, to_branch)
224 push_result.workingtree_updated = False
225 from_branch = self.make_from_branch()
226 message = push.create_push_message(from_branch, push_result)
227 self.assertEqual(
228 "1 revision(s) pushed.\n\nThe working tree was not updated:\n"
229 "See 'bzr help working-trees' for more information.",
230 message)

Subscribers

People subscribed via source and target branches

to all changes: