Rev 4568: (jam) Bug #393366, in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Fri Jul 24 21:46:42 BST 2009
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 4568 [merge]
revision-id: pqm at pqm.ubuntu.com-20090724204637-5uutgmi7rx1u8xzp
parent: pqm at pqm.ubuntu.com-20090724181237-s69vjke1os2p6xmr
parent: john at arbash-meinel.com-20090724192225-uk2jubcygmtgq05e
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Fri 2009-07-24 21:46:37 +0100
message:
(jam) Bug #393366,
request the same order from fallbacks that we request from self.
(makes annotate play nicer with stacking)
added:
bzrlib/tests/per_repository_reference/test_get_record_stream.py test_get_record_stre-20090715175116-rot0ksup7ifmtki0-1
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/knit.py knit.py-20051212171256-f056ac8f0fbe1bd9
bzrlib/tests/per_repository_reference/__init__.py __init__.py-20080220025549-nnm2s80it1lvcwnc-2
bzrlib/tests/test_knit.py test_knit.py-20051212171302-95d4c00dd5f11f2b
=== modified file 'NEWS'
--- a/NEWS 2009-07-24 03:15:56 +0000
+++ b/NEWS 2009-07-24 18:26:21 +0000
@@ -26,6 +26,10 @@
Bug Fixes
*********
+* Annotating on a stacked branch will now succeed in simple scenarios.
+ There are still some complex scenarios where it will fail (bug #399884)
+ (John Arbash Meinel, #393366)
+
* Authenticating against an ssh server now uses ``auth_none`` to determine
if password authentication is even supported. This fixes a bug where
users would be prompted for a launchpad password, even though launchpad
=== modified file 'bzrlib/knit.py'
--- a/bzrlib/knit.py 2009-07-08 23:10:47 +0000
+++ b/bzrlib/knit.py 2009-07-15 17:51:40 +0000
@@ -1489,7 +1489,8 @@
non_local_keys,
positions):
generator = _VFContentMapGenerator(self, keys, non_local_keys,
- global_map)
+ global_map,
+ ordering=ordering)
for record in generator.get_record_stream():
yield record
else:
@@ -1993,6 +1994,9 @@
class _ContentMapGenerator(object):
"""Generate texts or expose raw deltas for a set of texts."""
+ def __init__(self, ordering='unordered'):
+ self._ordering = ordering
+
def _get_content(self, key):
"""Get the content object for key."""
# Note that _get_content is only called when the _ContentMapGenerator
@@ -2032,7 +2036,7 @@
# Loop over fallback repositories asking them for texts - ignore
# any missing from a particular fallback.
for record in source.get_record_stream(missing_keys,
- 'unordered', True):
+ self._ordering, True):
if record.storage_kind == 'absent':
# Not in thie particular stream, may be in one of the
# other fallback vfs objects.
@@ -2170,7 +2174,7 @@
"""Content map generator reading from a VersionedFiles object."""
def __init__(self, versioned_files, keys, nonlocal_keys=None,
- global_map=None, raw_record_map=None):
+ global_map=None, raw_record_map=None, ordering='unordered'):
"""Create a _ContentMapGenerator.
:param versioned_files: The versioned files that the texts are being
@@ -2184,6 +2188,7 @@
:param raw_record_map: A unparsed raw record map to use for answering
contents.
"""
+ _ContentMapGenerator.__init__(self, ordering=ordering)
# The vf to source data from
self.vf = versioned_files
# The keys desired
=== modified file 'bzrlib/tests/per_repository_reference/__init__.py'
--- a/bzrlib/tests/per_repository_reference/__init__.py 2009-06-15 06:47:14 +0000
+++ b/bzrlib/tests/per_repository_reference/__init__.py 2009-07-15 17:51:40 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2008 Canonical Ltd
+# Copyright (C) 2008, 2009 Canonical Ltd
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -98,6 +98,7 @@
'bzrlib.tests.per_repository_reference.test_check',
'bzrlib.tests.per_repository_reference.test_default_stacking',
'bzrlib.tests.per_repository_reference.test_fetch',
+ 'bzrlib.tests.per_repository_reference.test_get_record_stream',
'bzrlib.tests.per_repository_reference.test_get_rev_id_for_revno',
'bzrlib.tests.per_repository_reference.test_initialize',
'bzrlib.tests.per_repository_reference.test_unlock',
=== added file 'bzrlib/tests/per_repository_reference/test_get_record_stream.py'
--- a/bzrlib/tests/per_repository_reference/test_get_record_stream.py 1970-01-01 00:00:00 +0000
+++ b/bzrlib/tests/per_repository_reference/test_get_record_stream.py 2009-07-15 18:22:38 +0000
@@ -0,0 +1,201 @@
+# Copyright (C) 2008, 2009 Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+"""Tests that get_record_stream() behaves itself properly when stacked."""
+
+from bzrlib import (
+ errors,
+ knit,
+ )
+from bzrlib.tests.per_repository_reference import (
+ TestCaseWithExternalReferenceRepository,
+ )
+
+
+class TestGetRecordStream(TestCaseWithExternalReferenceRepository):
+
+ def setUp(self):
+ super(TestGetRecordStream, self).setUp()
+ builder = self.make_branch_builder('all')
+ builder.start_series()
+ # Graph of revisions:
+ #
+ # A
+ # |\
+ # B C
+ # |/|
+ # D E
+ # |\|
+ # F G
+ # These can be split up among the different repos as desired
+ #
+
+ builder.build_snapshot('A', None, [
+ ('add', ('', 'root-id', 'directory', None)),
+ ('add', ('file', 'f-id', 'file', 'initial content\n')),
+ ])
+ builder.build_snapshot('B', ['A'], [
+ ('modify', ('f-id', 'initial content\n'
+ 'and B content\n')),
+ ])
+ builder.build_snapshot('C', ['A'], [
+ ('modify', ('f-id', 'initial content\n'
+ 'and C content\n')),
+ ])
+ builder.build_snapshot('D', ['B', 'C'], [
+ ('modify', ('f-id', 'initial content\n'
+ 'and B content\n'
+ 'and C content\n')),
+ ])
+ builder.build_snapshot('E', ['C'], [
+ ('modify', ('f-id', 'initial content\n'
+ 'and C content\n'
+ 'and E content\n')),
+ ])
+ builder.build_snapshot('F', ['D'], [
+ ('modify', ('f-id', 'initial content\n'
+ 'and B content\n'
+ 'and C content\n'
+ 'and F content\n')),
+ ])
+ builder.build_snapshot('G', ['E', 'D'], [
+ ('modify', ('f-id', 'initial content\n'
+ 'and B content\n'
+ 'and C content\n'
+ 'and E content\n')),
+ ])
+ builder.finish_series()
+ self.all_repo = builder.get_branch().repository
+ self.all_repo.lock_read()
+ self.addCleanup(self.all_repo.unlock)
+ self.base_repo = self.make_repository('base')
+ self.stacked_repo = self.make_referring('referring', 'base')
+
+ def make_simple_split(self):
+ """Set up the repositories so that everything is in base except F"""
+ self.base_repo.fetch(self.all_repo, revision_id='G')
+ self.stacked_repo.fetch(self.all_repo, revision_id='F')
+
+ def make_complex_split(self):
+ """intermix the revisions so that base holds left stacked holds right.
+
+ base will hold
+ A B D F (and C because it is a parent of D)
+ referring will hold
+ C E G (only)
+ """
+ self.base_repo.fetch(self.all_repo, revision_id='B')
+ self.stacked_repo.fetch(self.all_repo, revision_id='C')
+ self.base_repo.fetch(self.all_repo, revision_id='F')
+ self.stacked_repo.fetch(self.all_repo, revision_id='G')
+
+ def test_unordered_fetch_simple_split(self):
+ self.make_simple_split()
+ keys = [('f-id', r) for r in 'ABCDF']
+ self.stacked_repo.lock_read()
+ self.addCleanup(self.stacked_repo.unlock)
+ stream = self.stacked_repo.texts.get_record_stream(
+ keys, 'unordered', False)
+ record_keys = set()
+ for record in stream:
+ if record.storage_kind == 'absent':
+ raise ValueError('absent record: %s' % (record.key,))
+ record_keys.add(record.key)
+ # everything should be present, we don't care about the order
+ self.assertEqual(keys, sorted(record_keys))
+
+ def test_unordered_fetch_complex_split(self):
+ self.make_complex_split()
+ keys = [('f-id', r) for r in 'ABCDEG']
+ self.stacked_repo.lock_read()
+ self.addCleanup(self.stacked_repo.unlock)
+ stream = self.stacked_repo.texts.get_record_stream(
+ keys, 'unordered', False)
+ record_keys = set()
+ for record in stream:
+ if record.storage_kind == 'absent':
+ raise ValueError('absent record: %s' % (record.key,))
+ record_keys.add(record.key)
+ # everything should be present, we don't care about the order
+ self.assertEqual(keys, sorted(record_keys))
+
+ def test_ordered_no_closure(self):
+ self.make_complex_split()
+ # Topological ordering allows B & C and D & E to be returned with
+ # either one first, so the required ordering is:
+ # [A (B C) (D E) G]
+ keys = [('f-id', r) for r in 'ABCDEG']
+ alt_1 = [('f-id', r) for r in 'ACBDEG']
+ alt_2 = [('f-id', r) for r in 'ABCEDG']
+ alt_3 = [('f-id', r) for r in 'ACBEDG']
+ self.stacked_repo.lock_read()
+ self.addCleanup(self.stacked_repo.unlock)
+ stream = self.stacked_repo.texts.get_record_stream(
+ keys, 'topological', False)
+ record_keys = []
+ for record in stream:
+ if record.storage_kind == 'absent':
+ raise ValueError('absent record: %s' % (record.key,))
+ record_keys.append(record.key)
+ self.assertTrue(record_keys in (keys, alt_1, alt_2, alt_3))
+
+ def test_ordered_fulltext_simple(self):
+ self.make_simple_split()
+ # This is a common case in asking to annotate a file that exists on a
+ # stacked branch.
+ # See https://bugs.launchpad.net/bzr/+bug/393366
+ # Topological ordering allows B & C and D & E to be returned with
+ # either one first, so the required ordering is:
+ # [A (B C) D F]
+ keys = [('f-id', r) for r in 'ABCDF']
+ alt_1 = [('f-id', r) for r in 'ACBDF']
+ self.stacked_repo.lock_read()
+ self.addCleanup(self.stacked_repo.unlock)
+ stream = self.stacked_repo.texts.get_record_stream(
+ keys, 'topological', True)
+ record_keys = []
+ for record in stream:
+ if record.storage_kind == 'absent':
+ raise ValueError('absent record: %s' % (record.key,))
+ record_keys.append(record.key)
+ self.assertTrue(record_keys in (keys, alt_1))
+
+ def test_ordered_fulltext_complex(self):
+ self.make_complex_split()
+ # Topological ordering allows B & C and D & E to be returned with
+ # either one first, so the required ordering is:
+ # [A (B C) (D E) G]
+ keys = [('f-id', r) for r in 'ABCDEG']
+ alt_1 = [('f-id', r) for r in 'ACBDEG']
+ alt_2 = [('f-id', r) for r in 'ABCEDG']
+ alt_3 = [('f-id', r) for r in 'ACBEDG']
+ self.stacked_repo.lock_read()
+ self.addCleanup(self.stacked_repo.unlock)
+ stream = self.stacked_repo.texts.get_record_stream(
+ keys, 'topological', True)
+ record_keys = []
+ for record in stream:
+ if record.storage_kind == 'absent':
+ raise ValueError('absent record: %s' % (record.key,))
+ record_keys.append(record.key)
+ # Note that currently --2a format repositories do this correctly, but
+ # KnitPack format repositories do not.
+ if isinstance(self.stacked_repo.texts, knit.KnitVersionedFiles):
+ # See https://bugs.launchpad.net/bzr/+bug/399884
+ self.expectFailure('KVF does not weave fulltexts from fallback'
+ ' repositories to preserve perfect order',
+ self.assertTrue, record_keys in (keys, alt_1, alt_2, alt_3))
+ self.assertTrue(record_keys in (keys, alt_1, alt_2, alt_3))
=== modified file 'bzrlib/tests/test_knit.py'
--- a/bzrlib/tests/test_knit.py 2009-07-06 19:37:07 +0000
+++ b/bzrlib/tests/test_knit.py 2009-07-24 19:22:25 +0000
@@ -2230,7 +2230,7 @@
# self.assertEqual([("annotate", key_basis)], basis.calls)
self.assertEqual([('get_parent_map', set([key_basis])),
('get_parent_map', set([key_basis])),
- ('get_record_stream', [key_basis], 'unordered', True)],
+ ('get_record_stream', [key_basis], 'topological', True)],
basis.calls)
def test_check(self):
@@ -2342,9 +2342,9 @@
# ask which fallbacks have which parents.
self.assertEqual([
("get_parent_map", set([key_basis, key_basis_2, key_missing])),
- # unordered is asked for by the underlying worker as it still
- # buffers everything while answering - which is a problem!
- ("get_record_stream", [key_basis_2, key_basis], 'unordered', True)],
+ # topological is requested from the fallback, because that is what
+ # was requested at the top level.
+ ("get_record_stream", [key_basis_2, key_basis], 'topological', True)],
calls)
def test_get_record_stream_unordered_deltas(self):
@@ -2571,7 +2571,7 @@
last_call = basis.calls[-1]
self.assertEqual('get_record_stream', last_call[0])
self.assertEqual(set([key_left, key_right]), set(last_call[1]))
- self.assertEqual('unordered', last_call[2])
+ self.assertEqual('topological', last_call[2])
self.assertEqual(True, last_call[3])
More information about the bazaar-commits
mailing list