Rev 3478: Split things into a registry of annotation policies, and start in http://bzr.arbash-meinel.com/branches/bzr/1.6-dev/annotation

John Arbash Meinel john at arbash-meinel.com
Thu Jun 5 23:48:04 BST 2008


At http://bzr.arbash-meinel.com/branches/bzr/1.6-dev/annotation

------------------------------------------------------------
revno: 3478
revision-id: john at arbash-meinel.com-20080605224745-wnih42sf8rqwbmdc
parent: john at arbash-meinel.com-20080605172317-h2u23f1ldi6h2rev
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: annotation
timestamp: Thu 2008-06-05 17:47:45 -0500
message:
  Split things into a registry of annotation policies, and start
  refactoring the tests to handle different policies.
-------------- next part --------------
=== modified file 'bzrlib/annotate.py'
--- a/bzrlib/annotate.py	2008-06-05 17:23:17 +0000
+++ b/bzrlib/annotate.py	2008-06-05 22:47:45 +0000
@@ -32,11 +32,15 @@
     errors,
     osutils,
     patiencediff,
+    registry,
     tsort,
     )
 from bzrlib.config import extract_email_address
 
 
+annotation_policy_registry = registry.Registry()
+
+
 class AnnotationPolicy(object):
     """This class represents a specific method for performing annotations.
 
@@ -44,8 +48,16 @@
     both sides claim different last-modified revisions.)
     """
 
-    def __init__(self):
+    def __init__(self, heads_provider):
+        """Create a new AnnotationPolicy
+
+        :param heads_provider: An object which provids a .heads() call to
+            resolve if any revision ids are children of others.
+            Should also provide a 'cache(keys, head)' function.
+            Can be None.
+        """
         self._sequence_matcher = patiencediff.PatienceSequenceMatcher
+        self._heads_provider = heads_provider
 
     def _get_matching_blocks(self, old, new):
         matcher = self._sequence_matcher(None, old, new)
@@ -230,64 +242,79 @@
             last_child_idx = child_idx + match_len
 
     def _resolve_ambiguous(self, revision_id, left, right):
-        """We have an ambiguous line between two texts.
-        
-        Resolve this somehow.
-        """
-        raise NotImplementedError(self._resolve_ambiguous)
-        if self._heads_provider is not None:
-            heads = self._heads_provider.heads((left[0], right[0]))
-            if len(heads) == 1:
-                return (iter(heads).next(), left[1])
-            else:
-                # Both claim different origins
-                # We know that revision_id is the head for
-                # left and right, so cache it
-                self._heads_provider.cache(
-                    (revision_id, left[0]),
-                    (revision_id,))
-                self._heads_provider.cache(
-                    (revision_id, right[0]),
-                    (revision_id,))
-                return (revision_id, left[1])
-        # Without a heads provider, we just always pick 'tip'
-        return (revision_id, left[1])
-
-
-class HeadsAnnotationPolicy(AnnotationPolicy):
-    """An annotation policy that uses a heads provider."""
-
-    def __init__(self, heads_provider):
-        """Create a new HeadsAnnotationPolicy.
-
-        :param heads_provider: An object which provids a .heads() call to
-            resolve if any revision ids are children of others.
-        """
-        super(HeadsAnnotationPolicy, self).__init__()
-        self._heads_provider = heads_provider
-
-    def _resolve_ambiguous(self, revision_id, left, right):
-        """We have an ambiguous line between two texts.
-        
-        Resolve this somehow.
-        """
-        if self._heads_provider is not None:
-            heads = self._heads_provider.heads((left[0], right[0]))
-            if len(heads) == 1:
-                return (iter(heads).next(), left[1])
-            else:
-                # Both claim different origins
-                # We know that revision_id is the head for
-                # left and right, so cache it
-                self._heads_provider.cache(
-                    (revision_id, left[0]),
-                    (revision_id,))
-                self._heads_provider.cache(
-                    (revision_id, right[0]),
-                    (revision_id,))
-                return (revision_id, left[1])
-        # Without a heads provider, we just always pick 'tip'
-        return (revision_id, left[1])
+        """Resolve an ambiguous annotation between texts.
+
+        This is the thunk layer, implementations should override
+        _do_resolve_ambiguous.
+
+        :param revision_id: The revision id to assign if this line is
+            considered newly introduced.
+        :param left: (left_parent_id, text)
+        :param right: (right_parent_id, text)
+        :return: (annotation_id, text)
+        """
+        # TODO: Would it be cleaner to just pass in the revision ids, rather
+        #       than (revision_id, text)?
+        return self._do_resolve_ambiguous(revision_id, left, right)
+
+    def _do_resolve_ambiguous(self, revision_id, left, right):
+        """See AnnotationPolicy._resolve_ambiguous
+        
+        This is intended to be overriden by implementations.
+        """
+        raise NotImplementedError(self._do_resolve_ambiguous)
+
+
+class MergeNodeAnnotationPolicy(AnnotationPolicy):
+    """Assign ambiguous lines to the node merging the results."""
+
+    def _do_resolve_ambiguous(self, revision_id, left, right):
+        """We have an ambiguous line between two texts.
+        
+        Resolve this somehow.
+        """
+        if self._heads_provider is None:
+            # Without a heads provider, we just always pick 'tip'
+            return (revision_id, left[1])
+        heads = self._heads_provider.heads((left[0], right[0]))
+        if len(heads) == 1:
+            return (iter(heads).next(), left[1])
+        else:
+            # Both claim different origins
+            # We know that revision_id is the head for
+            # left and right, so cache it
+            self._heads_provider.cache(
+                (revision_id, left[0]),
+                (revision_id,))
+            self._heads_provider.cache(
+                (revision_id, right[0]),
+                (revision_id,))
+            return (revision_id, left[1])
+
+
+class RightHeadAnnotationPolicy(AnnotationPolicy):
+    """Assign ambiguous lines to the last merge parent."""
+
+    def _do_resolve_ambiguous(self, revision_id, left, right):
+        """See AnnotationPolicy._do_resolve_ambiguous"""
+        if self._heads_provider is None:
+            return right
+        heads = self._heads_provider.heads((left[0], right[0]))
+        if len(heads) == 1:
+            return (iter(heads).next(), left[1])
+        else:
+            # Both claim different origins
+            # give it to right
+            return right
+
+
+annotation_policy_registry.register('merge-node', MergeNodeAnnotationPolicy,
+    help="Assign ambiguous lines to the revision which merges them."
+         " (after checking heads())")
+annotation_policy_registry.register('right-head', RightHeadAnnotationPolicy,
+    help="Assign ambiguous lines to the merge revision."
+         " (after checking heads())")
+annotation_policy_registry.default_key = 'merge-node'
 
 
 def annotate_file(branch, rev_id, file_id, verbose=False, full=False,
@@ -426,6 +453,6 @@
         If None, then any ancestry disputes will be resolved with
         new_revision_id
     """
-    policy = HeadsAnnotationPolicy(heads_provider)
+    policy = annotation_policy_registry.get()(heads_provider)
     return policy.reannotate(parents_lines, new_lines, new_revision_id,
                             _left_matching_blocks=_left_matching_blocks)

=== modified file 'bzrlib/tests/test_annotate.py'
--- a/bzrlib/tests/test_annotate.py	2008-06-04 22:37:47 +0000
+++ b/bzrlib/tests/test_annotate.py	2008-06-05 22:47:45 +0000
@@ -264,60 +264,6 @@
         tree1.lock_read()
         return tree1
 
-    def create_duplicate_lines_tree(self):
-        tree1 = self.make_branch_and_tree('tree1')
-        base_text = ''.join(l for r, l in duplicate_base)
-        a_text = ''.join(l for r, l in duplicate_A)
-        b_text = ''.join(l for r, l in duplicate_B)
-        c_text = ''.join(l for r, l in duplicate_C)
-        d_text = ''.join(l for r, l in duplicate_D)
-        e_text = ''.join(l for r, l in duplicate_E)
-        self.build_tree_contents([('tree1/file', base_text)])
-        tree1.add(['file'], ['file-id'])
-        tree1.commit('base', rev_id='rev-base')
-        tree2 = tree1.bzrdir.sprout('tree2').open_workingtree()
-
-        self.build_tree_contents([('tree1/file', a_text),
-                                  ('tree2/file', b_text)])
-        tree1.commit('A', rev_id='rev-A')
-        tree2.commit('B', rev_id='rev-B')
-
-        tree2.merge_from_branch(tree1.branch)
-        conflicts.resolve(tree2, None) # Resolve the conflicts
-        self.build_tree_contents([('tree2/file', d_text)])
-        tree2.commit('D', rev_id='rev-D')
-
-        self.build_tree_contents([('tree1/file', c_text)])
-        tree1.commit('C', rev_id='rev-C')
-
-        tree1.merge_from_branch(tree2.branch)
-        conflicts.resolve(tree1, None) # Resolve the conflicts
-        self.build_tree_contents([('tree1/file', e_text)])
-        tree1.commit('E', rev_id='rev-E')
-        return tree1
-
-    def assertRepoAnnotate(self, expected, repo, file_id, revision_id):
-        """Assert that the revision is properly annotated."""
-        actual = list(repo.revision_tree(revision_id).annotate_iter(file_id))
-        if actual != expected:
-            # Create an easier to understand diff when the lines don't actually
-            # match
-            self.assertEqualDiff(''.join('\t'.join(l) for l in expected),
-                                 ''.join('\t'.join(l) for l in actual))
-
-    def test_annotate_duplicate_lines(self):
-        # XXX: Should this be a repository_implementations test?
-        tree1 = self.create_duplicate_lines_tree()
-        repo = tree1.branch.repository
-        repo.lock_read()
-        self.addCleanup(repo.unlock)
-        self.assertRepoAnnotate(duplicate_base, repo, 'file-id', 'rev-base')
-        self.assertRepoAnnotate(duplicate_A, repo, 'file-id', 'rev-A')
-        self.assertRepoAnnotate(duplicate_B, repo, 'file-id', 'rev-B')
-        self.assertRepoAnnotate(duplicate_C, repo, 'file-id', 'rev-C')
-        self.assertRepoAnnotate(duplicate_D, repo, 'file-id', 'rev-D')
-        self.assertRepoAnnotate(duplicate_E, repo, 'file-id', 'rev-E')
-
     def test_annotate_shows_dotted_revnos(self):
         tree1, tree2 = self.create_merged_trees()
 
@@ -522,3 +468,60 @@
         blocks = [(0, 1, 1), (1, 2, 0)]
         self.annotateEqual([('rev2', 'a\n'), ('rev1', 'a\n')], [parent],
                            new_text, 'rev2', blocks)
+
+
+class TestAnnotationPolicy(tests.TestCaseWithTransport):
+
+    def create_duplicate_lines_tree(self):
+        tree1 = self.make_branch_and_tree('tree1')
+        base_text = ''.join(l for r, l in duplicate_base)
+        a_text = ''.join(l for r, l in duplicate_A)
+        b_text = ''.join(l for r, l in duplicate_B)
+        c_text = ''.join(l for r, l in duplicate_C)
+        d_text = ''.join(l for r, l in duplicate_D)
+        e_text = ''.join(l for r, l in duplicate_E)
+        self.build_tree_contents([('tree1/file', base_text)])
+        tree1.add(['file'], ['file-id'])
+        tree1.commit('base', rev_id='rev-base')
+        tree2 = tree1.bzrdir.sprout('tree2').open_workingtree()
+
+        self.build_tree_contents([('tree1/file', a_text),
+                                  ('tree2/file', b_text)])
+        tree1.commit('A', rev_id='rev-A')
+        tree2.commit('B', rev_id='rev-B')
+
+        tree2.merge_from_branch(tree1.branch)
+        conflicts.resolve(tree2, None) # Resolve the conflicts
+        self.build_tree_contents([('tree2/file', d_text)])
+        tree2.commit('D', rev_id='rev-D')
+
+        self.build_tree_contents([('tree1/file', c_text)])
+        tree1.commit('C', rev_id='rev-C')
+
+        tree1.merge_from_branch(tree2.branch)
+        conflicts.resolve(tree1, None) # Resolve the conflicts
+        self.build_tree_contents([('tree1/file', e_text)])
+        tree1.commit('E', rev_id='rev-E')
+        return tree1
+
+    def assertRepoAnnotate(self, expected, repo, file_id, revision_id):
+        """Assert that the revision is properly annotated."""
+        actual = list(repo.revision_tree(revision_id).annotate_iter(file_id))
+        if actual != expected:
+            # Create an easier to understand diff when the lines don't actually
+            # match
+            self.assertEqualDiff(''.join('\t'.join(l) for l in expected),
+                                 ''.join('\t'.join(l) for l in actual))
+
+    def test_annotate_duplicate_lines(self):
+        # XXX: Should this be a repository_implementations test?
+        tree1 = self.create_duplicate_lines_tree()
+        repo = tree1.branch.repository
+        repo.lock_read()
+        self.addCleanup(repo.unlock)
+        self.assertRepoAnnotate(duplicate_base, repo, 'file-id', 'rev-base')
+        self.assertRepoAnnotate(duplicate_A, repo, 'file-id', 'rev-A')
+        self.assertRepoAnnotate(duplicate_B, repo, 'file-id', 'rev-B')
+        self.assertRepoAnnotate(duplicate_C, repo, 'file-id', 'rev-C')
+        self.assertRepoAnnotate(duplicate_D, repo, 'file-id', 'rev-D')
+        self.assertRepoAnnotate(duplicate_E, repo, 'file-id', 'rev-E')



More information about the bazaar-commits mailing list