[merge][#288751] avoid creating text deltas spanning repository stacks

Martin Pool mbp at canonical.com
Thu Nov 20 04:03:45 GMT 2008

With this additional patch it does pass my interactive tests.

I'm going to merge this it bzr.dev as a partial fix for this bug, but
I'd like to understand more about why this is needed, and whether we
should be fixing this in some other place.  I filed
<https://bugs.launchpad.net/bzr/+bug/300177> to track the issue that
the behaviour of the stream here should be better-defined.

For a multi-parent diff, we would want to be checking that all the
parents are present, but I don't think they'd be occurring in this
format.  That would seem to mean that the content record if it's not
just a fulltext should make it clear what dependencies it has.

As I understand it, what John's saying about the parents is that it's
possible to have texts which aren't compressed against their first
graph parent, and this won't be properly represented in the record
stream.  If that's handled cleanly up to being put in the record
stream, then we could also address that by passing the compression
parents separately.

=== modified file 'bzrlib/knit.py'
--- bzrlib/knit.py      2008-11-19 07:47:46 +0000
+++ bzrlib/knit.py      2008-11-20 03:38:47 +0000
@@ -1324,6 +1324,11 @@
         # can't generate annotations from new deltas until their basis parent
         # is present anyway, so we get away with not needing an index that
         # includes the new keys.
+        #
+        # See <http://launchpad.net/bugs/300177> about ordering of compression
+        # parents in the records - to be conservative, we insist that all
+        # parents must be present to avoid expanding to a fulltext.
+        #
         # key = basis_parent, value = index entry to add
         buffered_index_entries = {}
         for record in stream:
@@ -1331,11 +1336,11 @@
             # Raise an error when a record is missing.
             if record.storage_kind == 'absent':
                 raise RevisionNotPresent([record.key], self)
-            elif ((record.storage_kind in knit_types)
+            elif ((record.storage_kind in knit_types)
                   and (not parents
+                       or not self._index.missing_keys(parents)
                        or not self._fallback_vfs
-                       or self._index.has_key(parents[0])
-                       or not self.has_key(parents[0]))):
+                       or self.missing_keys(parents))):
                 # we can insert the knit record literally if either it has no
                 # compression parent OR we already have its basis in this kvf
                 # OR the basis is not present even in the fallbacks.  In the
@@ -1383,7 +1388,7 @@
                     # They're required to be physically in this
                     # KnitVersionedFiles, not in a fallback.
-                    if not self.has_key(parents[0]):
+                    if self.missing_keys(parents):
                         pending = buffered_index_entries.setdefault(
                             parents[0], [])

Martin <http://launchpad.net/~mbp/>

More information about the bazaar mailing list