Merge lp:~spiv/bzr/hpss-cross-format-fetch-bug-427736 into lp:bzr/2.0

[Andrew Bennetts]

[Andrew Bennetts (spiv)]

This fixes "unknown object type identifier 60" in bencode when pulling some branches.

It turns out that the client-side RemoteStreamSource was using the wrong serialiser to decode revision fulltexts in a stream of a stacked branch (it decodes them to extract the parent_ids so that it knows if there are any missing revisions that it needs to ask the next fallback for). Using the wrong serialiser doesn't matter if the revision serialiser is the same on both sides, and similarly this code doesn't get involved when there is no stacking. So this was just obscure enough a case that the test suite wasn't exercising it.

The fix is a one-liner, and I've added interrepo tests for this case.

[Robert Collins (lifeless)]

 review +1

perhaps change serialiser to from_serialise to make it unambiguous?

-Rob

review: Approve

[Andrew Bennetts (spiv)]

Robert Collins wrote:
> Review: Approve
> review +1
>
> perhaps change serialiser to from_serialise to make it unambiguous?

Good idea, done.

Thanks for the swift review!

-Andrew.

=== modified file 'NEWS'
--- NEWS	2009-10-01 00:56:56 +0000
+++ NEWS	2009-10-02 03:10:22 +0000
@@ -22,6 +22,10 @@
   ``keyboard-interactive`` auth but not ``password`` auth when using
   Paramiko.   (Andrew Bennetts, #433846)
 
+* Fixed fetches from a stacked branch on a smart server that were failing
+  with some combinations of remote and local formats.  This was causing
+  "unknown object type identifier 60" errors.  (Andrew Bennetts, #427736)
+
 * Occasional IndexError on renamed files have been fixed. Operations that
   set a full inventory in the working tree will now go via the
   apply_inventory_delta code path which is simpler and easier to

=== modified file 'bzrlib/remote.py'
--- bzrlib/remote.py	2009-09-10 06:32:42 +0000
+++ bzrlib/remote.py	2009-10-02 03:10:22 +0000
@@ -1887,7 +1887,7 @@
         :param search: The overall search to satisfy with streams.
         :param sources: A list of Repository objects to query.
         """
-        self.serialiser = self.to_format._serializer
+        self.from_serialiser = self.from_repository._format._serializer
         self.seen_revs = set()
         self.referenced_revs = set()
         # If there are heads in the search, or the key count is > 0, we are not
@@ -1910,7 +1910,8 @@
     def missing_parents_rev_handler(self, substream):
         for content in substream:
             revision_bytes = content.get_bytes_as('fulltext')
-            revision = self.serialiser.read_revision_from_string(revision_bytes)
+            revision = self.from_serialiser.read_revision_from_string(
+                revision_bytes)
             self.seen_revs.add(content.key[-1])
             self.referenced_revs.update(revision.parent_ids)
             yield content

=== modified file 'bzrlib/tests/per_interrepository/test_fetch.py'
--- bzrlib/tests/per_interrepository/test_fetch.py	2009-09-09 02:14:12 +0000
+++ bzrlib/tests/per_interrepository/test_fetch.py	2009-10-02 03:10:22 +0000
@@ -134,6 +134,44 @@
             to_repo.texts.get_record_stream([('foo', revid)],
             'unordered', True).next().get_bytes_as('fulltext'))
 
+    def test_fetch_from_stacked_smart(self):
+        self.setup_smart_server_with_call_log()
+        self.test_fetch_from_stacked()
+
+    def test_fetch_from_stacked_smart_old(self):
+        self.setup_smart_server_with_call_log()
+        self.disable_verb('Repository.get_stream_1.19')
+        self.test_fetch_from_stacked()
+
+    def test_fetch_from_stacked(self):
+        """Fetch from a stacked branch succeeds."""
+        if not self.repository_format.supports_external_lookups:
+            raise TestNotApplicable("Need stacking support in the source.")
+        builder = self.make_branch_builder('full-branch')
+        builder.start_series()
+        builder.build_snapshot('first', None, [
+            ('add', ('', 'root-id', 'directory', '')),
+            ('add', ('file', 'file-id', 'file', 'content\n'))])
+        builder.build_snapshot('second', ['first'], [
+            ('modify', ('file-id', 'second content\n'))])
+        builder.build_snapshot('third', ['second'], [
+            ('modify', ('file-id', 'third content\n'))])
+        builder.finish_series()
+        branch = builder.get_branch()
+        repo = self.make_repository('stacking-base')
+        trunk = repo.bzrdir.create_branch()
+        trunk.repository.fetch(branch.repository, 'second')
+        repo = self.make_repository('stacked')
+        stacked_branch = repo.bzrdir.create_branch()
+        stacked_branch.set_stacked_on_url(trunk.base)
+        stacked_branch.repository.fetch(branch.repository, 'third')
+        target = self.make_to_repository('target')
+        target.fetch(stacked_branch.repository, 'third')
+        target.lock_read()
+        self.addCleanup(target.unlock)
+        all_revs = set(['first', 'second', 'third'])
+        self.assertEqual(all_revs, set(target.get_parent_map(all_revs)))
+
     def test_fetch_parent_inventories_at_stacking_boundary_smart(self):
         self.setup_smart_server_with_call_log()
         self.test_fetch_parent_inventories_at_stacking_boundary()