[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.

[Andrew Bennetts]

John Arbash Meinel wrote:
[...]
> +                        # 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.

Good call.  I was a bit surprised to find I had to do it manually.  I've
extracted this into a simple helper method on the same class.  If nothing else
it helps avoid clutter in an already large method.

> ~     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.

Yeah, I'm wondering that too.  I don't really want to spend time digging into
it, though.

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

Thanks!  Sent to PQM, incremental diff attached in case someone is curious.

-Andrew.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: bug-217701-updated.patch
Type: text/x-diff
Size: 2545 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20080428/3a2c779e/attachment-0002.bin