Rev 2812: Review feedback. in http://people.ubuntu.com/~robertc/baz2.0/knits
Robert Collins
robertc at robertcollins.net
Wed Sep 12 05:22:02 BST 2007
At http://people.ubuntu.com/~robertc/baz2.0/knits
------------------------------------------------------------
revno: 2812
revision-id: robertc at robertcollins.net-20070912042151-o2k78pnf1hdwd2xt
parent: robertc at robertcollins.net-20070911041823-pn9kn8l5zmmptnsr
committer: Robert Collins <robertc at robertcollins.net>
branch nick: knits
timestamp: Wed 2007-09-12 14:21:51 +1000
message:
Review feedback.
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/knit.py knit.py-20051212171256-f056ac8f0fbe1bd9
bzrlib/reconcile.py reweave_inventory.py-20051108164726-1e5e0934febac06e
bzrlib/repofmt/knitrepo.py knitrepo.py-20070206081537-pyy4a00xdas0j4pf-1
bzrlib/repository.py rev_storage.py-20051111201905-119e9401e46257e3
bzrlib/tests/test_versionedfile.py test_versionedfile.py-20060222045249-db45c9ed14a1c2e5
bzrlib/versionedfile.py versionedfile.py-20060222045106-5039c71ee3b65490
bzrlib/weave.py knit.py-20050627021749-759c29984154256b
=== modified file 'NEWS'
--- a/NEWS 2007-09-11 04:18:23 +0000
+++ b/NEWS 2007-09-12 04:21:51 +0000
@@ -199,10 +199,9 @@
allows the avoidance of double-sha1 calculations during commit.
(Robert Collins)
- * The ``VersionedFile`` interface no longer protects against misuse when
- lines that are not lines, or are not strings are supplied. This saves
- nearly 30% of the minimum cost to store a version of a file.
- (Robert Collins)
+ * The ``VersionedFile`` interface now allows content checks to be bypassed
+ by supplying check_content=False. This saves nearly 30% of the minimum
+ cost to store a version of a file. (Robert Collins)
* ``Transport.should_cache`` has been removed. It was not called in the
previous release. (Martin Pool)
=== modified file 'bzrlib/knit.py'
--- a/bzrlib/knit.py 2007-09-10 03:53:04 +0000
+++ b/bzrlib/knit.py 2007-09-12 04:21:51 +0000
@@ -831,21 +831,21 @@
self._index.check_versions_present(version_ids)
def _add_lines_with_ghosts(self, version_id, parents, lines, parent_texts,
- nostore_sha, random_id):
+ nostore_sha, random_id, check_content):
"""See VersionedFile.add_lines_with_ghosts()."""
- self._check_add(version_id, lines, random_id)
+ self._check_add(version_id, lines, random_id, check_content)
return self._add(version_id, lines, parents, self.delta,
parent_texts, None, nostore_sha)
def _add_lines(self, version_id, parents, lines, parent_texts,
- left_matching_blocks, nostore_sha, random_id):
+ left_matching_blocks, nostore_sha, random_id, check_content):
"""See VersionedFile.add_lines."""
- self._check_add(version_id, lines, random_id)
+ self._check_add(version_id, lines, random_id, check_content)
self._check_versions_present(parents)
return self._add(version_id, lines[:], parents, self.delta,
parent_texts, left_matching_blocks, nostore_sha)
- def _check_add(self, version_id, lines, random_id):
+ def _check_add(self, version_id, lines, random_id, check_content):
"""check that version_id and lines are safe to add."""
if contains_whitespace(version_id):
raise InvalidRevisionId(version_id, self.filename)
@@ -856,6 +856,9 @@
# blanket that we can disable.
if not random_id and self.has_version(version_id):
raise RevisionAlreadyPresent(version_id, self.filename)
+ if check_content:
+ self._check_lines_not_unicode(lines)
+ self._check_lines_are_lines(lines)
def _add(self, version_id, lines, parents, delta, parent_texts,
left_matching_blocks, nostore_sha):
=== modified file 'bzrlib/reconcile.py'
--- a/bzrlib/reconcile.py 2007-09-11 04:05:11 +0000
+++ b/bzrlib/reconcile.py 2007-09-12 04:21:51 +0000
@@ -171,7 +171,7 @@
# reconcile.
new_inventory_vf._check_write_ok()
Weave._add_lines(new_inventory_vf, rev_id, parents,
- self.inventory.get_lines(rev_id), None, None, None, False)
+ self.inventory.get_lines(rev_id), None, None, None, False, True)
else:
new_inventory_vf.add_lines(rev_id, parents, self.inventory.get_lines(rev_id))
=== modified file 'bzrlib/repofmt/knitrepo.py'
--- a/bzrlib/repofmt/knitrepo.py 2007-08-30 08:27:29 +0000
+++ b/bzrlib/repofmt/knitrepo.py 2007-09-12 04:21:51 +0000
@@ -74,8 +74,9 @@
# This class isn't deprecated
pass
- def _inventory_add_lines(self, inv_vf, revid, parents, lines):
- inv_vf.add_lines_with_ghosts(revid, parents, lines)
+ def _inventory_add_lines(self, inv_vf, revid, parents, lines, check_content):
+ inv_vf.add_lines_with_ghosts(revid, parents, lines,
+ check_content=check_content)
@needs_read_lock
def _all_revision_ids(self):
=== modified file 'bzrlib/repository.py'
--- a/bzrlib/repository.py 2007-09-11 04:18:23 +0000
+++ b/bzrlib/repository.py 2007-09-12 04:21:51 +0000
@@ -125,16 +125,17 @@
inv_vf = self.control_weaves.get_weave('inventory',
self.get_transaction())
self._inventory_add_lines(inv_vf, revision_id, parents,
- osutils.split_lines(inv_text))
+ osutils.split_lines(inv_text), check_content=False)
return inv_sha1
- def _inventory_add_lines(self, inv_vf, revision_id, parents, lines):
+ def _inventory_add_lines(self, inv_vf, revision_id, parents, lines,
+ check_content=True):
final_parents = []
for parent in parents:
if parent in inv_vf:
final_parents.append(parent)
-
- inv_vf.add_lines(revision_id, final_parents, lines)
+ inv_vf.add_lines(revision_id, final_parents, lines,
+ check_content=check_content)
@needs_write_lock
def add_revision(self, revision_id, rev, inv=None, config=None):
@@ -2303,9 +2304,14 @@
# Don't change this to add_lines - add_lines_with_ghosts is cheaper
# than add_lines, and allows committing when a parent is ghosted for
# some reason.
+ # Note: as we read the content directly from the tree, we know its not
+ # been turned into unicode or badly split - but a broken tree
+ # implementation could give us bad output from readlines() so this is
+ # not a guarantee of safety. What would be better is always checking
+ # the content during test suite execution. RBC 20070912
result = versionedfile.add_lines_with_ghosts(
self._new_revision_id, parents, new_lines,
- random_id=self.random_revid)[0:2]
+ random_id=self.random_revid, check_content=False)[0:2]
versionedfile.clear_cache()
return result
=== modified file 'bzrlib/tests/test_versionedfile.py'
--- a/bzrlib/tests/test_versionedfile.py 2007-09-10 03:37:19 +0000
+++ b/bzrlib/tests/test_versionedfile.py 2007-09-12 04:21:51 +0000
@@ -115,6 +115,16 @@
f = self.reopen_file()
verify_file(f)
+ def test_add_unicode_content(self):
+ # unicode content is not permitted in versioned files.
+ # versioned files version sequences of bytes only.
+ vf = self.get_file()
+ self.assertRaises(errors.BzrBadParameterUnicode,
+ vf.add_lines, 'a', [], ['a\n', u'b\n', 'c\n'])
+ self.assertRaises(
+ (errors.BzrBadParameterUnicode, NotImplementedError),
+ vf.add_lines_with_ghosts, 'a', [], ['a\n', u'b\n', 'c\n'])
+
def test_add_follows_left_matching_blocks(self):
"""If we change left_matching_blocks, delta changes
@@ -132,6 +142,21 @@
left_matching_blocks=[(0, 2, 1), (1, 3, 0)])
self.assertEqual(['a\n', 'a\n', 'a\n'], vf.get_lines('3'))
+ def test_inline_newline_throws(self):
+ # \r characters are not permitted in lines being added
+ vf = self.get_file()
+ self.assertRaises(errors.BzrBadParameterContainsNewline,
+ vf.add_lines, 'a', [], ['a\n\n'])
+ self.assertRaises(
+ (errors.BzrBadParameterContainsNewline, NotImplementedError),
+ vf.add_lines_with_ghosts, 'a', [], ['a\n\n'])
+ # but inline CR's are allowed
+ vf.add_lines('a', [], ['a\r\n'])
+ try:
+ vf.add_lines_with_ghosts('b', [], ['a\r\n'])
+ except NotImplementedError:
+ pass
+
def test_add_reserved(self):
vf = self.get_file()
self.assertRaises(errors.ReservedId,
=== modified file 'bzrlib/versionedfile.py'
--- a/bzrlib/versionedfile.py 2007-09-10 03:53:04 +0000
+++ b/bzrlib/versionedfile.py 2007-09-12 04:21:51 +0000
@@ -78,7 +78,8 @@
raise NotImplementedError(self.has_version)
def add_lines(self, version_id, parents, lines, parent_texts=None,
- left_matching_blocks=None, nostore_sha=None, random_id=False):
+ left_matching_blocks=None, nostore_sha=None, random_id=False,
+ check_content=True):
"""Add a single text on top of the versioned file.
Must raise RevisionAlreadyPresent if the new version is
@@ -93,8 +94,8 @@
terminated \n. If the lines list does meet this constraint the add
routine may error or may succeed - but you will be unable to read
the data back accurately. (Checking the lines have been split
- correctly is expensive and extermely unlikely to catch bugs so it
- is not done at runtime.)
+ correctly is expensive and extremely unlikely to catch bugs so it
+ is not done at runtime unless check_content is True.)
:param parent_texts: An optional dictionary containing the opaque
representations of some or all of the parents of version_id to
allow delta optimisations. VERY IMPORTANT: the texts must be those
@@ -110,6 +111,8 @@
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_lines calls in the parent_texts dictionary.
@@ -118,15 +121,16 @@
parents = [osutils.safe_revision_id(v) for v in parents]
self._check_write_ok()
return self._add_lines(version_id, parents, lines, parent_texts,
- left_matching_blocks, nostore_sha, random_id)
+ left_matching_blocks, nostore_sha, random_id, check_content)
def _add_lines(self, version_id, parents, lines, parent_texts,
- left_matching_blocks, nostore_sha, random_id):
+ left_matching_blocks, nostore_sha, random_id, check_content):
"""Helper to do the class specific add_lines."""
raise NotImplementedError(self.add_lines)
def add_lines_with_ghosts(self, version_id, parents, lines,
- parent_texts=None, nostore_sha=None, random_id=False):
+ parent_texts=None, nostore_sha=None, random_id=False,
+ check_content=True):
"""Add lines to the versioned file, allowing ghosts to be present.
This takes the same parameters as add_lines and returns the same.
@@ -135,10 +139,10 @@
parents = [osutils.safe_revision_id(v) for v in parents]
self._check_write_ok()
return self._add_lines_with_ghosts(version_id, parents, lines,
- parent_texts, nostore_sha, random_id)
+ parent_texts, nostore_sha, random_id, check_content)
def _add_lines_with_ghosts(self, version_id, parents, lines, parent_texts,
- nostore_sha, random_id):
+ nostore_sha, random_id, check_content):
"""Helper to do class specific add_lines_with_ghosts."""
raise NotImplementedError(self.add_lines_with_ghosts)
=== modified file 'bzrlib/weave.py'
--- a/bzrlib/weave.py 2007-09-11 04:05:11 +0000
+++ b/bzrlib/weave.py 2007-09-12 04:21:51 +0000
@@ -261,7 +261,7 @@
return idx
def _add_lines(self, version_id, parents, lines, parent_texts,
- left_matching_blocks, nostore_sha, random_id):
+ left_matching_blocks, nostore_sha, random_id, check_content):
"""See VersionedFile.add_lines."""
idx = self._add(version_id, lines, map(self._lookup, parents),
nostore_sha=nostore_sha)
@@ -870,11 +870,12 @@
self._save()
def _add_lines(self, version_id, parents, lines, parent_texts,
- left_matching_blocks, nostore_sha, random_id):
+ left_matching_blocks, nostore_sha, random_id, check_content):
"""Add a version and save the weave."""
self.check_not_reserved_id(version_id)
result = super(WeaveFile, self)._add_lines(version_id, parents, lines,
- parent_texts, left_matching_blocks, nostore_sha, random_id)
+ parent_texts, left_matching_blocks, nostore_sha, random_id,
+ check_content)
self._save()
return result
More information about the bazaar-commits
mailing list