[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 andrew at canonical.com
Fri Apr 18 07:49:47 BST 2008


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

-------------- next part --------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: andrew.bennetts at canonical.com-20080418064515-\
#   wnqz648mpegu5x60
# target_branch: http://bazaar-vcs.org/bzr/bzr.dev
# testament_sha1: 0a24529f102ae354e6068ffae90df3777a48918a
# timestamp: 2008-04-18 16:46:54 +1000
# source_branch: http://people.ubuntu.com/~andrew/bzr/bug-217701
# base_revision_id: pqm at pqm.ubuntu.com-20080416172809-mq3p3rm6q3vefyyz
# 
# Begin patch
=== modified file 'bzrlib/knit.py'
--- bzrlib/knit.py	2008-04-09 23:35:55 +0000
+++ bzrlib/knit.py	2008-04-18 02:16:07 +0000
@@ -843,6 +843,23 @@
                             'on the source repository, and "bzr reconcile" '
                             'if necessary.' %
                             (version_id, parents[0]))
+                    if not self.delta:
+                        # 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
+
                 self._add_raw_records(
                     [(version_id, options, parents, length)],
                     reader_callable(length))

=== modified file 'bzrlib/store/revision/knit.py'
--- bzrlib/store/revision/knit.py	2008-04-04 00:06:58 +0000
+++ bzrlib/store/revision/knit.py	2008-04-18 06:17:29 +0000
@@ -113,7 +113,11 @@
 
     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
 
     def get_signature_file(self, transaction):
         """Get the signature text versioned file object."""

=== modified file 'bzrlib/tests/test_knit.py'
--- bzrlib/tests/test_knit.py	2008-04-09 23:35:55 +0000
+++ bzrlib/tests/test_knit.py	2008-04-18 06:45:15 +0000
@@ -1505,6 +1505,10 @@
         for plan_line, expected_line in zip(plan, AB_MERGE):
             self.assertEqual(plan_line, expected_line)
 
+
+class GetDataStreamTests(KnitTests):
+    """Tests for get_data_stream."""
+
     def test_get_stream_empty(self):
         """Get a data stream for an empty knit file."""
         k1 = self.make_test_knit()
@@ -1696,6 +1700,10 @@
             bytes = reader_callable(length)
             self.assertRecordContentEqual(k1, version_id, bytes)
 
+
+class InsertDataStreamTests(KnitTests):
+    """Tests for insert_data_stream."""
+
     def assertKnitFilesEqual(self, knit1, knit2):
         """Assert that the contents of the index and data files of two knits are
         equal.
@@ -1720,7 +1728,7 @@
             self.assertEqual(left.annotate(version),
                 right.annotate(version))
 
-    def test_insert_data_stream_empty(self):
+    def test_empty_stream(self):
         """Inserting a data stream with no records should not put any data into
         the knit.
         """
@@ -1733,7 +1741,7 @@
                          k1.transport.get_bytes(k1._index._filename),
                          "The .kndx should have nothing apart from the header.")
 
-    def test_insert_data_stream_one_record(self):
+    def test_one_record(self):
         """Inserting a data stream with one record from a knit with one record
         results in byte-identical files.
         """
@@ -1744,7 +1752,7 @@
         target.insert_data_stream(data_stream)
         self.assertKnitFilesEqual(source, target)
 
-    def test_insert_data_stream_annotated_unannotated(self):
+    def test_annotated_stream_into_unannotated_knit(self):
         """Inserting an annotated datastream to an unannotated knit works."""
         # case one - full texts.
         source = self.make_test_knit(name='source', annotate=True)
@@ -1757,7 +1765,7 @@
         target.insert_data_stream(source.get_data_stream(['text-b']))
         self.assertKnitValuesEqual(source, target)
 
-    def test_insert_data_stream_unannotated_annotated(self):
+    def test_unannotated_stream_into_annotated_knit(self):
         """Inserting an unannotated datastream to an annotated knit works."""
         # case one - full texts.
         source = self.make_test_knit(name='source', annotate=False)
@@ -1770,7 +1778,7 @@
         target.insert_data_stream(source.get_data_stream(['text-b']))
         self.assertKnitValuesEqual(source, target)
 
-    def test_insert_data_stream_records_already_present(self):
+    def test_records_already_present(self):
         """Insert a data stream where some records are alreday present in the
         target, and some not.  Only the new records are inserted.
         """
@@ -1788,7 +1796,7 @@
         # record was not added a second time.
         self.assertKnitFilesEqual(source, target)
 
-    def test_insert_data_stream_multiple_records(self):
+    def test_multiple_records(self):
         """Inserting a data stream of all records from a knit with multiple
         records results in byte-identical files.
         """
@@ -1803,7 +1811,7 @@
         
         self.assertKnitFilesEqual(source, target)
 
-    def test_insert_data_stream_ghost_parent(self):
+    def test_ghost_parent(self):
         """Insert a data stream with a record that has a ghost parent."""
         # Make a knit with a record, text-a, that has a ghost parent.
         source = self.make_test_knit(name='source')
@@ -1824,7 +1832,7 @@
             target.get_parent_map(['text-a', 'text-ghost']))
         self.assertEqual(split_lines(TEXT_1), target.get_lines('text-a'))
 
-    def test_insert_data_stream_inconsistent_version_lines(self):
+    def test_inconsistent_version_lines(self):
         """Inserting a data stream which has different content for a version_id
         than already exists in the knit will raise KnitCorrupt.
         """
@@ -1838,7 +1846,7 @@
         self.assertRaises(
             errors.KnitCorrupt, target.insert_data_stream, data_stream)
 
-    def test_insert_data_stream_inconsistent_version_parents(self):
+    def test_inconsistent_version_parents(self):
         """Inserting a data stream which has different parents for a version_id
         than already exists in the knit will raise KnitCorrupt.
         """
@@ -1853,7 +1861,7 @@
         self.assertRaises(
             errors.KnitCorrupt, target.insert_data_stream, data_stream)
 
-    def test_insert_data_stream_unknown_format(self):
+    def test_unknown_stream_format(self):
         """A data stream in a different format to the target knit cannot be
         inserted.
 
@@ -1867,7 +1875,7 @@
             errors.KnitDataStreamUnknown,
             target.insert_data_stream, data_stream)
 
-    def test_insert_data_stream_bug_208418(self):
+    def test_bug_208418(self):
         """You can insert a stream with an incompatible format, even when:
           * the stream has a line-delta record,
           * whose parent is in the target, also stored as a line-delta
@@ -1897,10 +1905,26 @@
         target.insert_data_stream(data_stream)
         # No errors should have been raised.
 
-
-    #  * test that a stream of "already present version, then new version"
-    #    inserts correctly.
-
+    def test_line_delta_record_into_non_delta_knit(self):
+        # Make a data stream with a line-delta record
+        source = self.make_test_knit(name='source', delta=True)
+        base_lines = split_lines(TEXT_1)
+        source.add_lines('version-1', [], base_lines)
+        source.add_lines('version-2', ['version-1'], base_lines + ['a\n'])
+        # The second record should be a delta.
+        self.assertEqual('line-delta', source._index.get_method('version-2'))
+        data_stream = source.get_data_stream(['version-1', 'version-2'])
+
+        # Insert the stream into a non-delta knit.
+        target = self.make_test_knit(name='target', delta=False)
+        target.insert_data_stream(data_stream)
+        
+        # Both versions are fulltexts in the target
+        self.assertEqual('fulltext', target._index.get_method('version-1'))
+        self.assertEqual('fulltext', target._index.get_method('version-2'))
+
+
+class DataStreamTests(KnitTests):
 
     def assertMadeStreamKnit(self, source_knit, versions, target_knit):
         """Assert that a knit made from a stream is as expected."""

# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWbgpCPUADlx/gGQURER57///
f////r////BgFC7fDCnd3H2ZUdKGgaNVojLaMXcrwCPb3Pdu10eu93dsNvauLWxGkz3LZqhurYSS
gI0AyABkmBqekmCm9DUxqanoYoNqNABglECMmE0RTxMlPUPUfqTymgNMgNANADQDQ0DjQ0DRpkaa
NMgMTBAADQGgNMgMCZAkRBAiZGpqfqJ+pGyaTaR6gDI0xABoAaaAARSSnlPEnqnqbU9NTU/0p6ZM
qemCehpA00NQ9Qw0aAANAikIACaBNCNNMp6FT9U9iU/UyeqaGn5UeoG1PSAeiH6opiUgIGBFUm85
MAsWPI4M6LNK4ZlBpF6SuSMNWEanGqiIueby6oWnc/Ah/uS/wXmMH0kPEs1qFrMhHKg3jV/Y66i+
VmvkYMXgssnRcgxRnqJ27cp9Hk38bifd8/77qv9vOIbxYELRgfiPkkkzOMHpu507TtOqLbl48JDx
zVC7SQGcIuWZgSgyzjjErrWkpXwW1Nw1TqrlO+ofyphgn9Na4J6cu/d6fCyxPIREzUH03jHn3jZT
yF5gZgQMEMQEQJBCRAm/PoIXELKosBcsBkTybopUncSctYIHkTeYd3d0dHdlaxLlhVt3aLUdZJLj
w/jkxxdu2ySclisxZKtyhSOvW8+Y5MeIntR3uO39i90jnHkKY+IUMX1gqC8ogovD2T19Zz9nz0pT
g8opAtWBNZ/2+7IbCcyJG+udsIfWUzjfA/w8BxHC7PHdhTAqjnS4a51xDFmDpkK4Eq99d6gYHLa5
lawF2NjY2Ng6ljJowcJXckkUwHQrFhbGL1BasgB+nA6Ccq09Ym7rkNPZTJsmnSw7xYNNiOp9feKk
LZsKxuXSAyqwO6mDOxWLyXtV653zuy8nz5axztQljlOcyVryz4n3nyJHegmeeeeea+I2DaQiv6/6
ICIrHtHHgbBvn39EFa+nb9vCsXqHLbU5elj/S7Uyx6WFAazXOEVK1tKlsfadP59/qFM40NmE7crR
Sbepn3Vtwv6qQx7Of++/4uTwYZrI2TJss9lOtUH5ASF9og0EGwOouBuqwHcscIExASICC0QTaHCj
gyDvPYrgTC98zBFRIfaWBmfcPcNwXlAKgYRu46OhZZ69FtO0b7rsc5WdOMzOycicn5CCeA/UWCQU
oFX0KJ30MSJ6hoBg9wq93uCX2+L4xeHTk0hREKIgiIIiCIjT2tcR5RJc3F6PgnmXA7HNz03JA2wB
XMJjOXnmMhoVA90DQgRTvO4+8eehIOlhl8xsgWiQ4EwYXSWViRQKNqNv31GV5aSgQTzGYXqwBbWS
ocsZjYrk2BazByayQgUCsuPaZeXGmd5GCuBcONehWWK11giWBivX1o1FmgXXmpAuCY6/uw933Fxu
FD/TQYUcWHE0NhUUwMB6JvrfP8+DYHA6g3IzCoqHiMU9x2iNdJDBC5wZyjGYZ0sDrlIPrJH/4eOF
vLyEzacK+c8dCsu0hkQcjiPMm/ICYuQGADkyJlIQqZdO3J1rOyz0lEEgesCWLpdqN0asFRichby8
rMiZJCd44gStai0K8yprwZbK3BcN+Rj9aBWBaAWzQmsGJBeW1Fd/pgFDh86WnEgXI+FgzXKDaA1M
/42rbeoVDMW4MyNDAwvINS4ur6jqKGl5iYjd2K9B2OO2y+p0amcdYjR5AxbAwkEhZEYlQOEDmj13
MzaEK1YAsuGMDGlxkWnQqOfPEkcDiZI4FkEzYwM9iopE8yVC/HxJldmJmbdnGQWFZv4bwu4cy3Yg
HCrhWYhwJFpvUe75eG2ZaBqQbjAkXGRM+o0oD6C1XTgrwvjjKweEsqydZMqGQmxcSrMy8/mLgvvF
irkbiRaVmauJM0NxadZBmjyJmJabplbuRsvJy6xmb5m4twub8AUY5LG8TQHeHzbDMeamo4aVLjgs
lDUeTH3mpCP8egehHsDFGYXnI4ZEsr6pUly6ydUiKeAwW1kayKiRzm2i9O7FSgdcAoVs3ELeRPMy
wOEGlFcyDQgrRgg5nMxkXFxoSFrAoappJvKWNxe41oYVJAaCDTTMniGbT10lxC/p5ugbF5Uanp7T
Y/SwmQP4I79S4uNrPFWm94lDcD+8OPaVZo+WNupuRUieyLMYttChYCwmMXtOoJkK+CXtpSgHnvuA
nwOGxoGZHmrLx070XAeEjGxBBI92R5xQe0xMKjjT+aj3PyNmq3jgZT2IOenHhXuNMsqi+sg7m8yz
vMchqm5DcTDkT6zzK/sA3WcJa9Bg22znvr4zdx+weqCg0uAYVgNSwkzCAeRzUIMC6jiTOhM4q3kk
cCkMTy9YyCCVFAXcT6wKW2YBaAwwYUiUeXKamOOQYNuRJ7RimRx41bB8uJFETzSdcpIgYGcCpmXG
qMuQF+CaamRdu0id+KCP7nhCmTx7TCcWrRSQ4iKlaIDIKbDdQYtUBjxmwz6xtLYXAvcthQMWJVGa
HXJgkxzINI378DXYpSspasd9Bx6KlyzAsIIw035liZrwNRxgMNSpgS1GSKyLCw/zz2nd16br2q6M
qqiUs2bkrcRfNWQIkIkoC7LEooDNZhmYmZgWHoYbSYocUEg3d566y68kGtuMZUNSDKUq41CTQGf6
vzHbvsLjiwWkio21x0Mz7o0KYfw2MTAsOJorkj75gWesH1A9hmZ43HIbkKoiJ057biqoNAZWk4Ki
ONnWJg7qIPbidxxJw2Qw63jA5BVpRxQ1SNa4neE3ZnQPferCVSCMHl1y1UzIEx8R2hkYjTEPa3bK
QhAaZCyNKlDAujzU2aaDgUuQKBMxxkiJb6EXCg/aP0GXUPIJQMKEBiDuV3iM0gi4L1lGe0HZHnBq
GBQ9wwDPeM1deuJ9IOU74mZjGxPUCcDDgJ8e4aY02m02NkUFEKFCYxDTTNfWGNrvC6BL2heMUisM
gahgZDMMhBM49gt74/nU9XAOjkx+/8YeTug8x4Pi8kx1hMOL6urgI67TYhpBwiODUDP1BfALBn6N
Iu9n4xZzZfPjnzdTpRDFVFiFvFyCnzQbTf45yYX0Ewv+MlYT/uBNkE3PALxycV6wsV2TCK9c/Fmw
aOEIsYrkQKYQfMT4g9MPFWSvc4ubILLUw+PSLXi86JRVRB4VAX8WSC/+bKGz4P8Hzf/82NUc3XQe
MVAte777YuDcMXJxAp8YJhMK+57XRQGH4R9z4gjNO4VgQdXL17vTsFzWsopNdYQcjTR4Z/DZ0d+0
V0BT1xY31esY59XyrAuy1xSe8vrtXHIVN9vnjFhj3UahOAun00cb3zx9/ILyelFFhft3LtWbB168
/X1bN15697vDsuTV9uO3Q3WNl9ToG+cPdDNxlk4Z3Q+Nre606uSKoQ6oiEoTLqlXRIVFJshBLZKg
tMBk4c3xoXdaiz3yWaY22NBWylq2JQxsBxoaYmCofUHNkknBCEbzL3ukIGQe4eUMYGI9x8sD4yx4
3sgQwPB+FisRaERxQvcNO/67U8JnAKURaLU6TEh6Zydi6Ux4gHYzNlxEQ+UzgVz0OBedDElMvmdL
isq9iPTMHwBeP08X8H0iCe7xx+PuhnyAHSgMwWgvOL3FbpRB0g1MogkGpkIlNTIRINTIRINTKjqM
M29BAjHsb1vA141Bu3RkRkDbtfUOZuPcuC6b6GBqDtRGKt8iY9tcXKLRl0vHKbB/UWPVCnIB0qht
VFFUyuDrOCkZmYGJBxmtUMR4G8kyOGztM6O5sObzPxi1H+rBwrN5W03mJuOZ3iCfELN5gbzY8Z+8
9/EfxFx8wu5wej5L6QiWpcrimLw6PP08eAeLWAwmculQskGAZFxkVFpKZgXGaMg8u0xcS60/Gx8L
ymJ2QXdPcP66eiwvm6IJh6hBNa+YL8apVMTnGa0JgJXu7nKNZHUOuMDV5nw6HM+wBSa1Wkv+SImB
9C99au7hV8SJwXSp6JexjUPHx9m2+lLeoqNDu8OwmV6lh2FZafaXFZ3Fo1lWw/CwwMTgr5HmMthH
9+41sBzLFjaAgz8WQ9x0271NUUsXIENp6AdRz9hT51N+J5F52TMHE7zgcwp+RIbW9grDb/oHyPKg
6Ea7OPuJ4dpGJTjgTdglDF6wiEjXjG86e3eqmqttwG2B5oA+PSHX7/hDyUwMb6ZbAtYGVGxtttvK
V8u3vYe9N7nNc/pO8h58hVF2nM5nbuTEj47OsrrVLPXCxIz09prmHWrTdbsU+wN9RI3jyGDQA3A+
zWH9QwDCYXdfbjsxDwFJIV4JWeeMhIR0FolyiCHolODowm0ClOWyZ0GTxwcszWQlQSJUUmHMCng/
HVRtFP5Dh/jH0HgIJ7d53DdulMkBB/hqITBGZj3yGXS26xD8BhYhmuARvAel0ARB6gccwe6oiEAk
OhKBm1kP9cLUF9AFLBRDRtQCocS88RDeDkEJSF9R7zxPWMxSNgwwqwaSfQUiEHzTaKZPAaTRpK5T
crD181VhvPsiED2SDDAmKbbjhhU2OIkzLqkYlwBdhaLsIzI/IDN4gIA3YzmaSM0A0Y3zk3jBi2ks
8oTJK0FQTBzCajBY8fw9YeM30wkSQXx4We9CaiOrgowqwNxHqG9fAjxXPPgdkeIfZk9+1POCCDIa
Bwl01dzwQiqMNtF2BqQAyWVCg6SaCAfpHo9fUIJKB84fEYECmIYA9f/nxAKtw6oH5whYDwC8vUYb
EDOD7Kgo/Oz/IfeNo1jiNUIEQqvuJcQfgjVwPb8kVtJtIFdP2gDgsQnYtdukA877Q2oXsaqknvhX
1pc+/t6u+p5773aPf0FgdCuVR7URCQ/j0sz0ykMiQQQbF+lERKYxC9ebLArBN7kFtkB2s/anlSFz
2K7572tpEPPoY1WnzNQCgs0/aOQpYAVU22s2PUG4awkFwZNMSEEgtHSicIHf0bg0OPmk6Hrg1CzV
hApnyDxDiG115DiKBvLYrUPKLwAeAwkBmOIs1CsYNX9TFMAvNxVnKrpAnH8JKu9TrC+o7y0fs4jq
XQK9pQaCCTCgwQEKeY4gKS0HGysQSY2HyFfmwOP6y0BSoNE5yEEliFnPYDEMyz9xJmXoMJJJCEQI
RVF8hOpPsIH7PlilUcrlAcriNomM3B8JAiCKgrN/igDoIckhAIgVl7R6TaCCQFF+lyDuSvFRlUBI
5zBKshxqBJX6AsHZceIxPctnqCQX73bZltsAzbRbROatohaYIhItALyCzPVjYFAXsIDaNHqKudbK
vIKQEwoihDlB574Dvh65jc0w5g/no58iO4S4VeyPmga+yrxuFHYAyGKHFKBh/6QNcw9f0SoBerdH
PZpQjAxcWiBd3v2AIDmSLpAokXbJ5Py7YqG0FYB4DAhQY+h3AKZ/pgOufuqHzhAu32+s9hwt4APT
5+OWGZJBkMIMQD+XkTBPiGA1P2vf4igYz2/QwCTBWTKOGNNvXQrcKCqCYmIElBRE9QNONgilqpYt
0AwZGHLDsvW17CxhvGugsd49QsgJDoOSECwMCwECEEDAQ2FQGojWZd0pEF7iDDALwhW4xSFXEaaC
iDxUznXiSTOpJ5UFkqOveUBQvHntDqLChZZKZKUoEJdyPIwB+A9Q4g25Bc6Ey3BkA0lSgQGYAyhN
wLjSbBpkD8Kgdt/CRKvILsQwBi8JhcfhyXtMCbC8zo+TDNHI+6JAEI8TgAa8aniE9wMwdfdtfG77
66yIfz4IshtIBpMT82HMaWjN5QzhhGYgkGMCOdawApIcRlQfKBisdQO7tq8dQ/QZnAO3pV8Kzo9O
uYgDN2Nh0xl0K0C5aSo6isLBcQsIs26GDFibWDog5NYH1i9hbHMTQtObKK41doZAKXgd85Fafjy5
TNPvFF3KrdzmTKBkYF9OnHaFIs4FTZ5eQXoddJAEl0WoUADMASArRsNEdg9ytg/vu6XDSZ8f6vZ6
zUg0DeFiSkrwAf7o0eOlaW1DhoPb/Ays/z9i7kinChIXBSEeoA==


More information about the bazaar mailing list