Merge lp:~spiv/bzr/remove-dead-chk-code into lp:~bzr/bzr/trunk-old

[Andrew Bennetts]

[Andrew Bennetts (spiv)]

There's a large code-path in StreamSource._get_inventory_stream that is totally
unused in current bzrlib. Unsurprisingly, it's also broken: there's no
_find_revision_outside_set method anymore. So this branch deletes it (and
replaces it with an assertion).

For good measure it also fixes a trivial bug in RemoteRepositoryFormat to make
sure that the .supports_chk attribute is accurate.

-Andrew.

[John A Meinel (jameinel)]

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

Andrew Bennetts wrote:
> Andrew Bennetts has proposed merging lp:~spiv/bzr/remove-dead-chk-code into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
>
> There's a large code-path in StreamSource._get_inventory_stream that is totally
> unused in current bzrlib. Unsurprisingly, it's also broken: there's no
> _find_revision_outside_set method anymore. So this branch deletes it (and
> replaces it with an assertion).
>
> For good measure it also fixes a trivial bug in RemoteRepositoryFormat to make
> sure that the .supports_chk attribute is accurate.
>
> -Andrew.
>
>
>

Actually, it isn't totally unused. It is used to copy data from a --dev7
format branch to a --2a format. Which happens to be broken as you say. :)

And I'm not positive, but I think this:

> +++ bzrlib/repository.py 2009-07-10 04:49:56 +0000
> @@ -4226,12 +4226,9 @@
>
> def _get_inventory_stream(self, revision_ids):
> from_format = self.from_repository._format
> - if (from_format.supports_chks and self.to_format.supports_chks
> - and (from_format._serializer == self.to_format._serializer)):
> - # Both sides support chks, and they use the same serializer, so it
> - # is safe to transmit the chk pages and inventory pages across
> - # as-is.
> - return self._get_chk_inventory_stream(revision_ids)
> + if (from_format.supports_chks and self.to_format.supports_chks):
> + raise AssertionError(
> + "This StreamSource should not be used for chk->chk fetches.")
> elif (not from_format.supports_chks):
> # Source repository doesn't support chks. So we can transmit the
> # inventories 'as-is' and either they are just accepted on the

^- actually should be "format.supports_chks or to_format.supports_chks".

Perhaps not, though.

  review approve
  merge approve

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

iEYEARECAAYFAkpXeiEACgkQJdeBCYSNAAN73ACfSAzYNc+52gN0ElTHVQs8llOE
pS0AniDM6pkVcfd36YJB9lqORPf+pmvm
=ytfn
-----END PGP SIGNATURE-----

review: Approve

[John A Meinel (jameinel)]

Didn't this land with a bunch of the other refactorings? I'm thinking we should mark this as rejected/superseded, etc.

=== modified file 'bzrlib/remote.py'
=== modified file 'bzrlib/repository.py'
--- bzrlib/repository.py	2009-08-30 23:51:10 +0000
+++ bzrlib/repository.py	2009-08-31 04:37:17 +0000
@@ -4545,6 +4545,7 @@
 
     def _get_inventory_stream(self, revision_ids, missing=False):
         from_format = self.from_repository._format
+<<<<<<< TREE
         if (from_format.supports_chks and self.to_format.supports_chks and
             from_format.network_name() == self.to_format.network_name()):
             raise AssertionError(
@@ -4561,6 +4562,16 @@
             # Essentially the same format.
             return self._get_simple_inventory_stream(revision_ids,
                     missing=missing)
+=======
+        if (from_format.supports_chks and self.to_format.supports_chks):
+            raise AssertionError(
+                "This StreamSource should not be used for chk->chk fetches.")
+        elif (not from_format.supports_chks):
+            # Source repository doesn't support chks. So we can transmit the
+            # inventories 'as-is' and either they are just accepted on the
+            # target, or the Sink will properly convert it.
+            return self._get_simple_inventory_stream(revision_ids)
+>>>>>>> MERGE-SOURCE
         else:
             # Any time we switch serializations, we want to use an
             # inventory-delta based approach.
@@ -4577,6 +4588,7 @@
             delta_closure = not self.delta_on_metadata()
         yield ('inventories', from_weave.get_record_stream(
             [(rev_id,) for rev_id in revision_ids],
+<<<<<<< TREE
             self.inventory_fetch_order(), delta_closure))
 
     def _get_convertable_inventory_stream(self, revision_ids,
@@ -4599,6 +4611,19 @@
             effectively streams a complete inventory. Used for stuff like
             filling in missing parents, etc.
         """
+=======
+            self.inventory_fetch_order(),
+            not self.delta_on_metadata()))
+
+    def _get_convertable_inventory_stream(self, revision_ids):
+        # XXX: One of source or target is using chks, and they don't have
+        #      compatible serializations. The StreamSink code expects to be
+        #      able to convert on the target, so we need to put
+        #      bytes-on-the-wire that can be converted
+        yield ('inventories', self._stream_invs_as_fulltexts(revision_ids))
+
+    def _stream_invs_as_fulltexts(self, revision_ids):
+>>>>>>> MERGE-SOURCE
         from_repo = self.from_repository
         revision_keys = [(rev_id,) for rev_id in revision_ids]
         parent_map = from_repo.inventories.get_parent_map(revision_keys)