Merge lp:~jelmer/brz/get-full into lp:brz

[Jelmer Vernooij]

Work with chunks rather than full texts, in tuned gzip and commit builder.

[Martin Packman (gz)]

Changes here are all reasonable, but won't make much difference yet I think?

review: Approve

[Jelmer Vernooij (jelmer)]

On Tue, Dec 12, 2017 at 01:09:56AM -0000, Martin Packman wrote:
> Review: Approve
>
> Changes here are all reasonable, but won't make much difference yet I think?
Yep, this is just a first step towards not dealing with full texts.

--
Jelmer Vernooij <email address hidden>
PGP Key: https://www.jelmer.uk/D729A457.asc

=== modified file 'breezy/bzr/groupcompress.py'
--- breezy/bzr/groupcompress.py	2017-11-11 15:06:09 +0000
+++ breezy/bzr/groupcompress.py	2017-12-05 00:06:55 +0000
@@ -1305,24 +1305,6 @@
                                                nostore_sha=nostore_sha))[0]
         return sha1, length, None
 
-    def _add_text(self, key, parents, text, nostore_sha=None, random_id=False):
-        """See VersionedFiles._add_text()."""
-        self._index._check_write_ok()
-        self._check_add(key, None, random_id, check_content=False)
-        if not isinstance(text, bytes):
-            raise errors.BzrBadParameterUnicode("text")
-        if parents is None:
-            # The caller might pass None if there is no graph data, but kndx
-            # indexes can't directly store that, so we give them
-            # an empty tuple instead.
-            parents = ()
-        # double handling for now. Make it work until then.
-        length = len(text)
-        record = FulltextContentFactory(key, parents, None, text)
-        sha1 = list(self._insert_record_stream([record], random_id=random_id,
-                                               nostore_sha=nostore_sha))[0]
-        return sha1, length, None
-
     def add_fallback_versioned_files(self, a_versioned_files):
         """Add a source of texts for texts not present in this knit.
 

=== modified file 'breezy/bzr/knit.py'
--- breezy/bzr/knit.py	2017-11-11 15:06:09 +0000
+++ breezy/bzr/knit.py	2017-12-05 00:06:55 +0000
@@ -995,21 +995,6 @@
             parent_texts, left_matching_blocks, nostore_sha, random_id,
             line_bytes=line_bytes)
 
-    def _add_text(self, key, parents, text, nostore_sha=None, random_id=False):
-        """See VersionedFiles._add_text()."""
-        self._index._check_write_ok()
-        self._check_add(key, None, random_id, check_content=False)
-        if not isinstance(text, bytes):
-            raise errors.BzrBadParameterUnicode("text")
-        if parents is None:
-            # The caller might pass None if there is no graph data, but kndx
-            # indexes can't directly store that, so we give them
-            # an empty tuple instead.
-            parents = ()
-        return self._add(key, None, parents,
-            None, None, nostore_sha, random_id,
-            line_bytes=text)
-
     def _add(self, key, lines, parents, parent_texts,
         left_matching_blocks, nostore_sha, random_id,
         line_bytes):

=== modified file 'breezy/bzr/versionedfile.py'
--- breezy/bzr/versionedfile.py	2017-11-12 13:53:51 +0000
+++ breezy/bzr/versionedfile.py	2017-12-05 00:06:55 +0000
@@ -976,36 +976,6 @@
         """
         raise NotImplementedError(self.add_lines)
 
-    def _add_text(self, key, parents, text, nostore_sha=None, random_id=False):
-        """Add a text to the store.
-
-        This is a private function for use by VersionedFileCommitBuilder.
-
-        :param key: The key tuple of the text to add. If the last element is
-            None, a CHK string will be generated during the addition.
-        :param parents: The parents key tuples of the text to add.
-        :param text: A string containing the text to be committed.
-        :param nostore_sha: Raise ExistingContent and do not add the lines to
-            the versioned file if the digest of the lines matches this.
-        :param random_id: If True a random id has been selected rather than
-            an id determined by some deterministic process such as a converter
-            from a foreign VCS. When True the backend may choose not to check
-            for uniqueness of the resulting key within the versioned file, so
-            this should only be done when the result is expected to be unique
-            anyway.
-        :param check_content: If True, the lines supplied are verified to be
-            bytestrings that are correctly formed lines.
-        :return: The text sha1, the number of bytes in the text, and an opaque
-                 representation of the inserted version which can be provided
-                 back to future _add_text calls in the parent_texts dictionary.
-        """
-        # The default implementation just thunks over to .add_lines(),
-        # inefficient, but it works.
-        return self.add_lines(key, parents, osutils.split_lines(text),
-                              nostore_sha=nostore_sha,
-                              random_id=random_id,
-                              check_content=True)
-
     def add_mpdiffs(self, records):
         """Add mpdiffs to this VersionedFile.
 

=== modified file 'breezy/bzr/vf_repository.py'
--- breezy/bzr/vf_repository.py	2017-11-19 16:30:53 +0000
+++ breezy/bzr/vf_repository.py	2017-12-05 00:06:55 +0000
@@ -76,6 +76,7 @@
     )
 
 from ..sixish import (
+    BytesIO,
     range,
     viewitems,
     viewvalues,
@@ -486,12 +487,8 @@
                         nostore_sha = None
                     file_obj, stat_value = tree.get_file_with_stat(change[1][1], file_id)
                     try:
-                        text = file_obj.read()
-                    finally:
-                        file_obj.close()
-                    try:
-                        entry.text_sha1, entry.text_size = self._add_text_to_weave(
-                            file_id, text, heads, nostore_sha)
+                        entry.text_sha1, entry.text_size = self._add_lines_to_weave(
+                            file_id, file_obj.readlines(), heads, nostore_sha)
                         yield file_id, change[1][1], (entry.text_sha1, stat_value)
                     except errors.ExistingContent:
                         # No content change against a carry_over parent
@@ -499,6 +496,8 @@
                         carried_over = True
                         entry.text_size = parent_entry.text_size
                         entry.text_sha1 = parent_entry.text_sha1
+                    finally:
+                        file_obj.close()
                 elif kind == 'symlink':
                     # Wants a path hint?
                     entry.symlink_target = tree.get_symlink_target(change[1][1], file_id)
@@ -506,7 +505,7 @@
                         parent_entry.symlink_target == entry.symlink_target):
                         carried_over = True
                     else:
-                        self._add_text_to_weave(change[0], '', heads, None)
+                        self._add_lines_to_weave(change[0], [], heads, None)
                 elif kind == 'directory':
                     if carry_over_possible:
                         carried_over = True
@@ -514,7 +513,7 @@
                         # Nothing to set on the entry.
                         # XXX: split into the Root and nonRoot versions.
                         if change[1][1] != '' or self.repository.supports_rich_root():
-                            self._add_text_to_weave(change[0], '', heads, None)
+                            self._add_lines_to_weave(change[0], [], heads, None)
                 elif kind == 'tree-reference':
                     if not self.repository._format.supports_tree_reference:
                         # This isn't quite sane as an error, but we shouldn't
@@ -529,7 +528,7 @@
                         parent_entry.reference_revision == reference_revision):
                         carried_over = True
                     else:
-                        self._add_text_to_weave(change[0], '', heads, None)
+                        self._add_lines_to_weave(change[0], [], heads, None)
                 else:
                     raise AssertionError('unknown kind %r' % kind)
                 if not carried_over:
@@ -555,10 +554,10 @@
             self._require_root_change(tree)
         self.basis_delta_revision = basis_revision_id
 
-    def _add_text_to_weave(self, file_id, new_text, parents, nostore_sha):
+    def _add_lines_to_weave(self, file_id, lines, parents, nostore_sha):
         parent_keys = tuple([(file_id, parent) for parent in parents])
-        return self.repository.texts._add_text(
-            (file_id, self._new_revision_id), parent_keys, new_text,
+        return self.repository.texts.add_lines(
+            (file_id, self._new_revision_id), parent_keys, lines,
             nostore_sha=nostore_sha, random_id=self.random_revid)[0:2]
 
 

=== modified file 'breezy/plugins/weave_fmt/repository.py'
--- breezy/plugins/weave_fmt/repository.py	2017-11-20 22:51:10 +0000
+++ breezy/plugins/weave_fmt/repository.py	2017-12-05 00:06:55 +0000
@@ -608,11 +608,13 @@
             raise errors.ReadOnlyError(self)
         if '/' in key[-1]:
             raise ValueError('bad idea to put / in %r' % (key,))
-        text = ''.join(lines)
+        chunks = lines
         if self._compressed:
-            text = tuned_gzip.bytes_to_gzip(text)
+            chunks = tuned_gzip.chunks_to_gzip(chunks)
         path = self._map(key)
-        self._transport.put_bytes_non_atomic(path, text, create_parent_dir=True)
+        self._transport.put_file_non_atomic(
+                path, BytesIO(b''.join(chunks)),
+                create_parent_dir=True)
 
     def insert_record_stream(self, stream):
         adapters = {}

=== modified file 'breezy/tests/test_tuned_gzip.py'
--- breezy/tests/test_tuned_gzip.py	2017-05-22 00:56:52 +0000
+++ breezy/tests/test_tuned_gzip.py	2017-12-05 00:06:55 +0000
@@ -32,9 +32,7 @@
     def assertToGzip(self, chunks):
         raw_bytes = b''.join(chunks)
         gzfromchunks = tuned_gzip.chunks_to_gzip(chunks)
-        gzfrombytes = tuned_gzip.bytes_to_gzip(raw_bytes)
-        self.assertEqual(gzfrombytes, gzfromchunks)
-        decoded = gzip.GzipFile(fileobj=BytesIO(gzfromchunks)).read()
+        decoded = gzip.GzipFile(fileobj=BytesIO(b''.join(gzfromchunks))).read()
         lraw, ldecoded = len(raw_bytes), len(decoded)
         self.assertEqual(lraw, ldecoded,
                          'Expecting data length %d, got %d' % (lraw, ldecoded))

=== modified file 'breezy/tuned_gzip.py'
--- breezy/tuned_gzip.py	2017-05-22 00:56:52 +0000
+++ breezy/tuned_gzip.py	2017-12-05 00:06:55 +0000
@@ -22,7 +22,7 @@
 import struct
 import zlib
 
-__all__ = ["bytes_to_gzip", "chunks_to_gzip"]
+__all__ = ["chunks_to_gzip"]
 
 
 def U32(i):
@@ -40,14 +40,6 @@
     return i & 0xFFFFFFFF
 
 
-def bytes_to_gzip(bytes, factory=zlib.compressobj,
-    level=zlib.Z_DEFAULT_COMPRESSION, method=zlib.DEFLATED,
-    width=-zlib.MAX_WBITS, mem=zlib.DEF_MEM_LEVEL,
-    crc32=zlib.crc32):
-    """Create a gzip file containing bytes and return its content."""
-    return chunks_to_gzip([bytes])
-
-
 def chunks_to_gzip(chunks, factory=zlib.compressobj,
     level=zlib.Z_DEFAULT_COMPRESSION, method=zlib.DEFLATED,
     width=-zlib.MAX_WBITS, mem=zlib.DEF_MEM_LEVEL,