Merge lp:~vila/bzr/284038-push-strict into lp:~bzr/bzr/trunk-old

[Vincent Ladeuil]

[Vincent Ladeuil (vila)]

Still related to 'push --strict', this patch addresses the concerns raised
in the previous review by:
- adding a check that the working tree is in sync with its branch,
- mentioning the --no-strict option in the error messages so that users
  are pointed in the right direction.

[John A Meinel (jameinel)]

review: Approve

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2009-06-30 17:00:26 +0000
+++ bzrlib/builtins.py	2009-06-30 20:35:19 +0000
@@ -1117,7 +1117,14 @@
             and (strict is None or strict)): # Default to True:
             changes = tree.changes_from(tree.basis_tree())
             if changes.has_changed() or len(tree.get_parent_ids()) > 1:
-                raise errors.UncommittedChanges(tree)
+                raise errors.UncommittedChanges(
+                    tree, more='Use --no-strict to force the push.')
+            if tree.last_revision() != tree.branch.last_revision():
+                # The tree has lost sync with its branch, there is little
+                # chance that the user is aware of it but he can still force
+                # the push with --no-strict
+                raise errors.OutOfDateTree(
+                    tree, more='Use --no-strict to force the push.')
 
         # Get the stacked_on branch, if any
         if stacked_on is not None:

=== modified file 'bzrlib/errors.py'
--- bzrlib/errors.py	2009-06-30 08:06:35 +0000
+++ bzrlib/errors.py	2009-06-30 20:35:19 +0000
@@ -2093,11 +2093,16 @@
 
 class OutOfDateTree(BzrError):
 
-    _fmt = "Working tree is out of date, please run 'bzr update'."
+    _fmt = "Working tree is out of date, please run 'bzr update'.%(more)s"
 
-    def __init__(self, tree):
+    def __init__(self, tree, more=None):
+        if more is None:
+            more = ''
+        else:
+            more = ' ' + more
         BzrError.__init__(self)
         self.tree = tree
+        self.more = more
 
 
 class PublicBranchOutOfDate(BzrError):
@@ -2779,13 +2784,17 @@
 class UncommittedChanges(BzrError):
 
     _fmt = ('Working tree "%(display_url)s" has uncommitted changes'
-            ' (See bzr status).')
+            ' (See bzr status).%(more)s')
 
-    def __init__(self, tree):
+    def __init__(self, tree, more=None):
+        if more is None:
+            more = ''
+        else:
+            more = ' ' + more
         import bzrlib.urlutils as urlutils
         display_url = urlutils.unescape_for_display(
             tree.bzrdir.root_transport.base, 'ascii')
-        BzrError.__init__(self, tree=tree, display_url=display_url)
+        BzrError.__init__(self, tree=tree, display_url=display_url, more=more)
 
 
 class MissingTemplateVariable(BzrError):

=== modified file 'bzrlib/tests/blackbox/test_push.py'
--- bzrlib/tests/blackbox/test_push.py	2009-06-30 09:19:06 +0000
+++ bzrlib/tests/blackbox/test_push.py	2009-06-30 20:35:19 +0000
@@ -47,8 +47,10 @@
     changes_scenarios = [
         ('uncommitted',
          dict(_changes_type= '_uncommitted_changes')),
-        ('pending_merges',
+        ('pending-merges',
          dict(_changes_type= '_pending_merges')),
+        ('out-of-sync-trees',
+         dict(_changes_type= '_out_of_sync_trees')),
         ]
     tests.multiply_tests(changes_tests, changes_scenarios, result)
     # No parametrization for the remaining tests
@@ -588,7 +590,7 @@
         self.assertEqual('', out)
 
 
-class TestPushStrict(tests.TestCaseWithTransport):
+class TestPushStrictMixin(object):
 
     def make_local_branch_and_tree(self):
         self.tree = self.make_branch_and_tree('local')
@@ -604,17 +606,21 @@
         conf = self.tree.branch.get_config()
         conf.set_user_option('push_strict', value)
 
+    _default_command = ['push', '../to']
+    _default_wd = 'local'
+    _default_errors = ['Working tree ".*/local/" has uncommitted '
+                       'changes \(See bzr status\)\.',]
+    _default_pushed_revid = 'modified'
+
     def assertPushFails(self, args):
-        self.run_bzr_error(['Working tree ".*/local/"'
-                            ' has uncommitted changes \(See bzr status\)\.',],
-                           ['push', '../to'] + args,
-                           working_dir='local', retcode=3)
+        self.run_bzr_error(self._default_errors, self._default_command + args,
+                           working_dir=self._default_wd, retcode=3)
 
     def assertPushSucceeds(self, args, pushed_revid=None):
-        self.run_bzr(['push', '../to'] + args,
-                     working_dir='local')
+        self.run_bzr(self._default_command + args,
+                     working_dir=self._default_wd)
         if pushed_revid is None:
-            pushed_revid = 'modified'
+            pushed_revid = self._default_pushed_revid
         tree_to = workingtree.WorkingTree.open('to')
         repo_to = tree_to.branch.repository
         self.assertTrue(repo_to.has_revision(pushed_revid))
@@ -622,7 +628,8 @@
 
 
 
-class TestPushStrictWithoutChanges(TestPushStrict):
+class TestPushStrictWithoutChanges(tests.TestCaseWithTransport,
+                                   TestPushStrictMixin):
 
     def setUp(self):
         super(TestPushStrictWithoutChanges, self).setUp()
@@ -646,7 +653,8 @@
         self.assertPushSucceeds([])
 
 
-class TestPushStrictWithChanges(TestPushStrict):
+class TestPushStrictWithChanges(tests.TestCaseWithTransport,
+                                TestPushStrictMixin):
 
     _changes_type = None # Set by load_tests
 
@@ -671,6 +679,18 @@
         self.tree.merge_from_branch(other_tree.branch)
         self.tree.revert(filenames=['other-file'], backups=False)
 
+    def _out_of_sync_trees(self):
+        self.make_local_branch_and_tree()
+        self.run_bzr(['checkout', '--lightweight', 'local', 'checkout'])
+        # Make a change and commit it
+        self.build_tree_contents([('local/file', 'modified in local')])
+        self.tree.commit('modify file', rev_id='modified-in-local')
+        # Exercise commands from the checkout directory
+        self._default_wd = 'checkout'
+        self._default_errors = ["Working tree is out of date, please run"
+                                " 'bzr update'\.",]
+        self._default_pushed_revid = 'modified-in-local'
+
     def test_push_default(self):
         self.assertPushFails([])