[MERGE][#217701][1.4] Force get_revision_file to set delta=False, and teach KnitVersionedFile.insert_data_stream to convert line-deltas to fulltexts when necessary.

[John Arbash Meinel]

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

Andrew Bennetts wrote:
| Andrew Bennetts wrote:
|> This short patch fixes <https://bugs.launchpad.net/bzr/+bug/217701>.
|>
|> The problem appears to be that get_revision_file doesn't reliably return a
|> KnitVersionedFile with delta=False, even though the revisions.kndx/.knit is
|> meant to be non-delta.  So this change:
|>
|>  - forces get_revision_file on a KnitRevisionStore to set delta=False, and
|>  - teaches KnitVersionedFile.insert_data_stream how to convert a line-delta
|>    record to a fulltext when inserting into a non-delta knit.
|>
|> In addition to fixing the traceback reported in #217701, new revision.knits will
|> be repaired, in that they will have all fulltexts, even if the source
|> incorrectly has a line-delta.
|>
|> This change still needs tests (although the existing suite does pass).  In the
|> meantime, feedback and sanity checking is most welcome!
|
| And this time with a test.  I couldn't help make a couple of small drive-by
| improvements to the test organisation (trivially breaking one huge TestCase into
| a few more reasonably-sized TestCases) in test_knit.py, but they shouldn't
| distract much from the new test.
|
| -Andrew
|
|

+                        # We received a line-delta record for a non-delta knit.
+                        # Convert it to a fulltext.
+                        gzip_bytes = reader_callable(length)
+                        lines, sha1 = self._data._parse_record(
+                            version_id, gzip_bytes)
+                        delta = self.factory.parse_line_delta(lines,
+                                version_id)
+                        content = self.factory.make(
+                            self.get_lines(parents[0]), parents[0])
+                        content.apply_delta(delta, version_id)
+                        digest, len, content = self.add_lines(
+                            version_id, parents, content.text())
+                        if digest != sha1:
+                            raise errors.VersionedFileInvalidChecksum(version)
+                        continue

^- This really seems like it should be a helper function. Converting a
line-delta to a fulltext shouldn't require this many lines.


~     def get_revision_file(self, transaction):
~         """Get the revision versioned file object."""
- -        return self.versioned_file_store.get_weave_or_empty('revisions',
transaction)
+        vf = self.versioned_file_store.get_weave_or_empty('revisions', transaction)
+        # The revisions knit should always be non-delta, so force delta=False
+        # here.
+        vf.delta = False
+        return vf

^- I realize that 'vf.delta' should always be false. The question is how was it
not set that way.


Otherwise I'm really happy to see this:
BB:approve

And I would recommend it for 1.4 as well, since it allows knit => pack
interoperability.

John
=:->

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

iEYEARECAAYFAkgIzZgACgkQJdeBCYSNAANregCgyG9NCDDhVNb0X2XY9G44mSpF
U+IAnArG46RDBWwoOpKYJqxJPERa0c02
=4DnI
-----END PGP SIGNATURE-----