Merge lp:~jelmer/launchpad/no-revhistory-3 into lp:launchpad

[Jelmer Vernooij]

Remove use of Repository.get_ancestry(), which is deprecated in bzr 2.5.

Remove use of Repository.get_ancestry(), which is deprecated in bzr 2.5.

Instead, this imports the get_ancestry() code into Launchpad, albeit a little bit simplified.

This is simpler than my previous attempt at getting rid of Repository.get_ancestry(), which tried to improve performance of the branch scanner at the same time by no longer having it look at the full ancestry but only those bits that were necessary.

[Graham Binns (gmb)]

Couple of nitpicks, nothing major:

[1]

104 + def test_missing_revision(self):

109 + def test_some(self):

122 + def test_children(self):

These all need leading comments or docstrings explaining the expected
behaviour for which they're testing, e.g.:

 def test_foo_bars(self):
    """When called, foo() returns bar."""
    ...

[2]

109 + def test_some(self):
110 + branch = self.make_branch('test')
111 + wt = branch.bzrdir.create_workingtree()
112 + wt.commit('msg a', rev_id='A')
113 + wt.commit('msg b', rev_id='B')
114 + wt.commit('msg c', rev_id='C')
115 + self.assertEqual(
116 + set(['A']), get_ancestry(branch.repository, 'A'))
117 + self.assertEqual(
118 + set(['A', 'B']), get_ancestry(branch.repository, 'B'))
119 + self.assertEqual(
120 + set(['A', 'B', 'C']), get_ancestry(branch.repository, 'C'))

s/wt/tree (or something meaningful). Same thing in test_children().

review: Approve (code)

=== modified file 'lib/lp/code/bzr.py'
--- lib/lp/code/bzr.py	2012-06-26 14:49:27 +0000
+++ lib/lp/code/bzr.py	2012-06-26 14:49:31 +0000
@@ -10,6 +10,8 @@
     'ControlFormat',
     'CURRENT_BRANCH_FORMATS',
     'CURRENT_REPOSITORY_FORMATS',
+    'branch_revision_history',
+    'get_ancestry',
     'get_branch_formats',
     'RepositoryFormat',
     ]
@@ -31,6 +33,7 @@
 from bzrlib.bzrdir import BzrDirMetaFormat1
 from bzrlib.errors import (
     NotStacked,
+    NoSuchRevision,
     UnstackableBranchFormat,
     )
 from bzrlib.plugins.loom.branch import (
@@ -57,6 +60,7 @@
     RepositoryFormatKnitPack5,
     )
 from bzrlib.revision import (
+    is_null,
     NULL_REVISION,
     )
 from bzrlib.repofmt.knitrepo import (
@@ -64,6 +68,7 @@
     RepositoryFormatKnit3,
     RepositoryFormatKnit4,
     )
+from bzrlib.tsort import topo_sort
 from lazr.enum import (
     DBEnumeratedType,
     DBItem,
@@ -341,3 +346,34 @@
         return ret
     finally:
         branch.unlock()
+
+
+def get_ancestry(repository, revision_id):
+    """Return a list of revision-ids integrated by a revision.
+
+    The first element of the list is always None, indicating the origin
+    revision.  This might change when we have history horizons, or
+    perhaps we should have a new API.
+
+    This is topologically sorted.
+    """
+    if is_null(revision_id):
+        return set()
+    if not repository.has_revision(revision_id):
+        raise NoSuchRevision(repository, revision_id)
+    repository.lock_read()
+    try:
+        graph = repository.get_graph()
+        keys = set()
+        search = graph._make_breadth_first_searcher([revision_id])
+        while True:
+            try:
+                found, ghosts = search.next_with_ghosts()
+            except StopIteration:
+                break
+            keys.update(found)
+        if NULL_REVISION in keys:
+            keys.remove(NULL_REVISION)
+    finally:
+        repository.unlock()
+    return keys

=== modified file 'lib/lp/code/tests/test_bzr.py'
--- lib/lp/code/tests/test_bzr.py	2012-06-26 14:49:27 +0000
+++ lib/lp/code/tests/test_bzr.py	2012-06-26 14:49:31 +0000
@@ -5,6 +5,10 @@
 
 __metaclass__ = type
 
+from bzrlib.errors import (
+    NoSuchRevision,
+    )
+from bzrlib.revision import NULL_REVISION
 from bzrlib.tests import (
     TestCaseInTempDir,
     TestCaseWithTransport,
@@ -14,6 +18,7 @@
     BranchFormat,
     branch_revision_history,
     ControlFormat,
+    get_ancestry,
     get_branch_formats,
     RepositoryFormat,
     )
@@ -84,8 +89,51 @@
 
     def test_some_commits(self):
         branch = self.make_branch('test')
-        wt = branch.bzrdir.create_workingtree()
-        wt.commit('acommit', rev_id='A')
-        wt.commit('bcommit', rev_id='B')
-        wt.commit('ccommit', rev_id='C')
-        self.assertEquals(['A', 'B', 'C'], branch_revision_history(wt.branch))
+        tree = branch.bzrdir.create_workingtree()
+        tree.commit('acommit', rev_id='A')
+        tree.commit('bcommit', rev_id='B')
+        tree.commit('ccommit', rev_id='C')
+        self.assertEquals(
+            ['A', 'B', 'C'], branch_revision_history(tree.branch))
+
+
+class TestGetAncestry(TestCaseWithTransport):
+    """Tests for lp.code.bzr.get_ancestry."""
+
+    def test_missing_revision(self):
+        # If the revision does not exist, NoSuchRevision should be raised.
+        branch = self.make_branch('test')
+        self.assertRaises(
+            NoSuchRevision, get_ancestry, branch.repository, 'orphan')
+
+    def test_some(self):
+        # Verify ancestors are included.
+        branch = self.make_branch('test')
+        tree = branch.bzrdir.create_workingtree()
+        tree.commit('msg a', rev_id='A')
+        tree.commit('msg b', rev_id='B')
+        tree.commit('msg c', rev_id='C')
+        self.assertEqual(
+            set(['A']), get_ancestry(branch.repository, 'A'))
+        self.assertEqual(
+            set(['A', 'B']), get_ancestry(branch.repository, 'B'))
+        self.assertEqual(
+            set(['A', 'B', 'C']), get_ancestry(branch.repository, 'C'))
+
+    def test_children(self):
+        # Verify non-mainline children are included.
+        branch = self.make_branch('test')
+        tree = branch.bzrdir.create_workingtree()
+        tree.commit('msg a', rev_id='A')
+        branch.generate_revision_history(NULL_REVISION)
+        tree.set_parent_ids([])
+        tree.commit('msg b', rev_id='B')
+        branch.generate_revision_history('A')
+        tree.set_parent_ids(['A', 'B'])
+        tree.commit('msg c', rev_id='C')
+        self.assertEqual(
+            set(['A']), get_ancestry(branch.repository, 'A'))
+        self.assertEqual(
+            set(['B']), get_ancestry(branch.repository, 'B'))
+        self.assertEqual(
+            set(['A', 'B', 'C']), get_ancestry(branch.repository, 'C'))

=== modified file 'lib/lp/codehosting/scanner/bzrsync.py'
--- lib/lp/codehosting/scanner/bzrsync.py	2012-06-26 14:49:27 +0000
+++ lib/lp/codehosting/scanner/bzrsync.py	2012-06-26 14:49:31 +0000
@@ -24,7 +24,10 @@
 from zope.component import getUtility
 from zope.event import notify
 
-from lp.code.bzr import branch_revision_history
+from lp.code.bzr import (
+    branch_revision_history,
+    get_ancestry,
+    )
 from lp.code.interfaces.branchjob import IRosettaUploadJobSource
 from lp.code.interfaces.revision import IRevisionSet
 from lp.code.model.branchrevision import BranchRevision
@@ -155,8 +158,7 @@
         bzr_last = bzr_branch.last_revision()
         db_last = self.db_branch.last_scanned_id
         if db_last is None:
-            added_ancestry = set(bzr_branch.repository.get_ancestry(bzr_last))
-            added_ancestry.discard(None)
+            added_ancestry = get_ancestry(bzr_branch.repository, bzr_last)
             removed_ancestry = set()
         else:
             graph = self._getRevisionGraph(bzr_branch, db_last)