Merge lp:~spiv/bzr/set-tags-bytes-bug-2.0 into lp:bzr/2.0

[Andrew Bennetts]

[Andrew Bennetts (spiv)]

Fix for bug 418931. The fix itself is a one-liner, but the patch adds some missing tests (including one that would have caught this bug).

This fix should be merged to both 2.0 and bzr.dev.

[Vincent Ladeuil (vila)]

review: Approve

[Vincent Ladeuil (vila)]

> Fix for bug 418931. The fix itself is a one-liner, but the patch adds some
> missing tests (including one that would have caught this bug).
>
> This fix should be merged to both 2.0 and bzr.dev.

Isn't it a 1.18 regression bug (see duplicates) and as such worth merging for 1.18.1 too ?

=== modified file 'NEWS'
--- NEWS	2009-08-31 00:25:33 +0000
+++ NEWS	2009-08-31 02:35:25 +0000
@@ -30,6 +30,10 @@
 * The main table of contents now provides links to the new Migration Docs
   and Plugins Guide. (Ian Clatworthy)
 
+* Fix assertion error about "_remember_remote_is_before" when pushing to
+  older smart servers.
+  (Andrew Bennetts, #418931)
+
 
 bzr 2.0rc1
 ##########

=== modified file 'bzrlib/remote.py'
--- bzrlib/remote.py	2009-08-27 05:22:14 +0000
+++ bzrlib/remote.py	2009-08-31 02:35:25 +0000
@@ -2267,6 +2267,7 @@
         medium = self._client._medium
         if medium._is_remote_before((1, 18)):
             self._vfs_set_tags_bytes(bytes)
+            return
         try:
             args = (
                 self._remote_path(), self._lock_token, self._repo_lock_token)

=== modified file 'bzrlib/tests/test_remote.py'
--- bzrlib/tests/test_remote.py	2009-08-27 05:22:14 +0000
+++ bzrlib/tests/test_remote.py	2009-08-31 02:35:25 +0000
@@ -280,6 +280,12 @@
         self.expecting_body = True
         return result[1], FakeProtocol(result[2], self)
 
+    def call_with_body_bytes(self, method, args, body):
+        self._check_call(method, args)
+        self._calls.append(('call_with_body_bytes', method, args, body))
+        result = self._get_next_response()
+        return result[1], FakeProtocol(result[2], self)
+
     def call_with_body_bytes_expecting_body(self, method, args, body):
         self._check_call(method, args)
         self._calls.append(('call_with_body_bytes_expecting_body', method,
@@ -851,6 +857,16 @@
 
 class RemoteBranchTestCase(RemoteBzrDirTestCase):
 
+    def lock_remote_branch(self, branch):
+        """Trick a RemoteBranch into thinking it is locked."""
+        branch._lock_mode = 'w'
+        branch._lock_count = 2
+        branch._lock_token = 'branch token'
+        branch._repo_lock_token = 'repo token'
+        branch.repository._lock_mode = 'w'
+        branch.repository._lock_count = 2
+        branch.repository._lock_token = 'repo token'
+
     def make_remote_branch(self, transport, client):
         """Make a RemoteBranch using 'client' as its _SmartClient.
 
@@ -995,6 +1011,54 @@
         self.assertEqual({}, result)
 
 
+class TestBranchSetTagsBytes(RemoteBranchTestCase):
+
+    def test_trivial(self):
+        transport = MemoryTransport()
+        client = FakeClient(transport.base)
+        client.add_expected_call(
+            'Branch.get_stacked_on_url', ('quack/',),
+            'error', ('NotStacked',))
+        client.add_expected_call(
+            'Branch.set_tags_bytes', ('quack/', 'branch token', 'repo token'),
+            'success', ('',))
+        transport.mkdir('quack')
+        transport = transport.clone('quack')
+        branch = self.make_remote_branch(transport, client)
+        self.lock_remote_branch(branch)
+        branch._set_tags_bytes('tags bytes')
+        self.assertFinished(client)
+        self.assertEqual('tags bytes', client._calls[-1][-1])
+
+    def test_backwards_compatible(self):
+        transport = MemoryTransport()
+        client = FakeClient(transport.base)
+        client.add_expected_call(
+            'Branch.get_stacked_on_url', ('quack/',),
+            'error', ('NotStacked',))
+        client.add_expected_call(
+            'Branch.set_tags_bytes', ('quack/', 'branch token', 'repo token'),
+            'unknown', ('Branch.set_tags_bytes',))
+        transport.mkdir('quack')
+        transport = transport.clone('quack')
+        branch = self.make_remote_branch(transport, client)
+        self.lock_remote_branch(branch)
+        class StubRealBranch(object):
+            def __init__(self):
+                self.calls = []
+            def _set_tags_bytes(self, bytes):
+                self.calls.append(('set_tags_bytes', bytes))
+        real_branch = StubRealBranch()
+        branch._real_branch = real_branch
+        branch._set_tags_bytes('tags bytes')
+        # Call a second time, to exercise the 'remote version already inferred'
+        # code path.
+        branch._set_tags_bytes('tags bytes')
+        self.assertFinished(client)
+        self.assertEqual(
+            [('set_tags_bytes', 'tags bytes')] * 2, real_branch.calls)
+
+
 class TestBranchLastRevisionInfo(RemoteBranchTestCase):
 
     def test_empty_branch(self):
@@ -1342,16 +1406,6 @@
             errors.NoSuchRevision, branch.set_last_revision_info, 123, 'revid')
         branch.unlock()
 
-    def lock_remote_branch(self, branch):
-        """Trick a RemoteBranch into thinking it is locked."""
-        branch._lock_mode = 'w'
-        branch._lock_count = 2
-        branch._lock_token = 'branch token'
-        branch._repo_lock_token = 'repo token'
-        branch.repository._lock_mode = 'w'
-        branch.repository._lock_count = 2
-        branch.repository._lock_token = 'repo token'
-
     def test_backwards_compatibility(self):
         """If the server does not support the Branch.set_last_revision_info
         verb (which is new in 1.4), then the client falls back to VFS methods.